Task: Generic unified-diff history renderer in ores.shell
Table of Contents
This page documents a task in the Consolidate history dialogs onto HistoryDialogBase story. It captures the goal, current status, acceptance, and any notes or results.
Goal
Re-enable the shell's history-diff command, disabled since the
currency pilot's hand-written shape was reverted, as one generic,
entity-parameterised unified-diff renderer that consumes the generic
history.v1.get response. Context lines come from unchanged fields;
+/- pairs come from the server-computed changes. All comparison
happens server-side (ores.history's dispatch_registry +
per-entity history provider); the shell renderer is pure formatting,
shared by every entity's history-diff command.
Status
| Field | Value |
|---|---|
| State | DONE |
| Parent story | Consolidate history dialogs onto HistoryDialogBase |
| Now | Nothing. |
| Waiting on | Nothing. |
| Next | Nothing. |
| Last touched | 2026-07-13 |
Acceptance
[X]ores.historyrestructured to the api/core layout used elsewhere in the codebase.[X]Genericrender_history_diff()added inores.shell, consuming the generichistory.v1.getsubject and formatting the server-computed diff as unified-diff text (context lines + -/+ pairs, or full addition from/dev/nullfor the oldest version).[X]Currency'shistorycommand merged into one entry point: flat table by default,--difffor the unified diff,--diff --version <n>to pick a version (rejected without--diff). Replaces the earlier separatehistory-diffcommand.[X]Country wired the same way: registered as a secondhistory.v1.getdispatch provider (it already had a codegen'dcountry_history_field_mapperandget_country_history) and its shellhistorycommand merged into--diff=/–version=.[X]Manually verified both against live entities with real history (add/rename, table view, diff view, diff of the initial version, and the--version-without=–diff= guard).[ ]Rolled out to the remaining entities with an existing plainhistoryshell command: tenants, accounts, change_reasons, change_reason_categories. These four do not yet have a codegen'd*_history_field_mapperor a confirmedget_*_historyservice method — that's prerequisite codegen work, not just wiring.[ ]Rolled out to the ~22 other entities that don't have any shell history command yet (out of this task's original "for every entity" scope; likely belongs to a follow-on task given the codegen prerequisite above).[X]Shell recipes updated (doc/recipes/shell/currencies/history_2.org,doc/recipes/shell/countries/history_3.org) to document--diff=/–version=; the standalone "History Diff" recipe/script for currencies was retired.
Plan
- Split
ores.historyintoapi/(protocol types,version_builder) andcore/(registrar,dispatch_registry, handler), matching the api/core layout already used byores.refdataand siblings. - Add
ores.shell::app::commands::render_history_diff(): takes the dispatch key (entity_type_of()), entity id, and an optional version; querieshistory.v1.getover the NATS session and prints the response as a unified diff. - Re-enable currency's history command (previously a disabled
history-diff) by delegating to the new generic renderer instead of the removed hand-writtencurrency_version_historyshape. - Merge it into the plain
historycommand rather than keeping a separatehistory-diff:history <id>keeps the flat table by default,--diffswitches to the unified diff,--diff --version <n>picks a version. Retired the standalone recipe/script. - Verified manually against a freshly provisioned tenant: added/edited
a test currency, checked table view, diff view, diff of the oldest
version (all-additions from
/dev/null), and the--versionwithout--diffguard. - Remaining: repeat the "delegate to
render_history_diff()"' wiring for the other 26 entities' history commands.
Notes
Test Scenarios
Manual QA scenarios (scaffolded via compass add test_scenario, run
through the QA Validation Runner panel) that verify this task. Link
new ones here as they're created; the scenario doc itself links back
via its "Verifies task" field.
| Scenario | State | Notes |
|---|---|---|
PRs
| PR | Title |
|---|---|
| #1547 | [history,shell] Generic unified-diff history renderer |
Review
| # | Comment summary | File | Decision | Notes |
|---|---|---|---|---|
| 1 | Context lines never rendered for unchanged fields, contradicting the docstring/task-doc/recipes | history_diff_renderer.cpp | Fixed | d310911c5: walk target->fields in order, unchanged as context, changed as -/+ |
| 2 | -/+ lines gated on old_value/new_value being non-empty, so clearing an optional field to blank looked identical to removal | history_diff_renderer.cpp | Fixed | d310911c5: unconditional -/+ for changed entries |
| 3 | No unit tests for render_history_diff() | history_diff_renderer.cpp | Declined (for now) | Manual verification only for this PR; noted as a follow-up once more entities roll out |
| 4 | –diff/–version parsing duplicated across entity command files | currencies_commands.cpp, countries_commands.cpp | Declined | Matches existing per-entity command pattern; tracked on the shell-codegen capture as the natural place to dedupe |
| 5 | Round 2: unconditional - line also fires for a genuinely added field, printing a spurious blank -field: line ahead of +field: value | history_diff_renderer.cpp | Fixed | 2d2f5db96: only emit - when the field existed in prev_it->fields; unreachable for currency/country's fixed schema today but correct for a future evolving-schema entity |
Result
Restructured ores.history into the api/core layout and added a
generic ores.shell::app::commands::render_history_diff() that
renders any entity's history.v1.get response as a unified diff —
context lines from unchanged fields, -/+ pairs from the
server-computed changes, or every field as an addition from
/dev/null for the oldest version.
Wired currency and country onto it: both registered as
history.v1.get dispatch providers in ores.refdata's registrar, and
both entities' shell command merged from a separate history-diff
into history <id> [--diff] [--version <n>] (--version rejected
without --diff). Manually verified end-to-end against a live
provisioned tenant for both entities: table view, diff view, diff of
the initial version, and the --version-without=–diff= guard.
Also discovered and filed two follow-ups rather than folding into this task's scope:
- A pre-existing bug where an entity's
versionresets after a soft-delete + recreate cycle, soread_all'sversion DESCordering andversion_builder's position-based renumbering can diff two unrelated "generations"' rows together (task 2A05DA93). - The remaining entities with a plain shell
historycommand (tenants, accounts, change_reasons, change_reason_categories) lack the codegen'd*_history_field_mapper=/=get_*_historythat currency and country already had; noted on the "Shell entity commands — top-level commissioning story" capture as the natural place to fold--diffrollout into once that story'shistoryfacet is templatized, rather than hand-wiring the remaining ~26 entities here.