Menu

#230 cli: make policy-history --json deterministic like other JSON commands

closed
nobody
None
2026-08-03
2026-08-02
Anonymous
No

Originally created by: AshSgDe29071999

Summary

policy-history --json used bare json.dumps(rows, default=str) while scan/doctor/log use sort_keys=True and compact separators.

  • Serialize with sort_keys=True, separators=(",", ":") to match siblings
  • Add a populated-ledger test and assert two consecutive calls are byte-identical

Fixes [#226]

Related

Tickets: #226
Tickets: #230
Tickets: #297

Discussion

  • Anonymous

    Anonymous - 2026-08-03

    Originally posted by: fu351

    Reviewed — merge-ready, nothing to change. I checked all four json.dumps call sites: main.py:242, :568 and :810 already had sort_keys=True and compact separators, and :992 was the lone outlier exactly as you described. All four agree now. Since sort_keys/separators touch only key order and whitespace, no parsed value changes for anything consuming this output — the only thing that changes is raw-byte diffing, which was the point.

    The test is the part worth singling out. Byte-identical-dump tests pass vacuously more often than they don't: one row in the fixture, or single-key dicts, and key order is trivially stable, so the test goes green whether or not the fix is present. Yours doesn't have that problem, and I confirmed it rather than assuming it — I applied your test file alone to unpatched main and watched it fail, then re-applied your fix and watched it pass. Three deliberate choices got you there:

    • 2 rows × 9 columns, inserted in non-alphabetical order, so alphabetical sorting actually reorders something and the assertion has teeth.
    • Deriving expected from json.loads(first.stdout) re-serialized canonically instead of hardcoding a byte string. That asserts "stdout is already in its own canonical form" rather than "stdout matches a string I typed once", and it won't rot the next time the fixture changes.
    • A fixture matching the real policy_changes schema column for column (src/doberman/storage/db.py:171-182) instead of inventing a row shape. An invented schema agrees with the code's assumptions rather than the database's, which is how a green test ends up hiding a real bug.

    That last one is a step above what the issue asked for.


    Three PRs in one sitting, each verified against the code rather than asserted. Thank you — that's a genuinely strong start here.

    Where to go next, if you want it:

    • #192 — define a shared JSON-output contract across the CLI commands (level-5). This is the generalization of what you just did: you fixed one command's determinism, and [#192] is about writing the contract all of them should meet and testing it once, centrally. You've now read all four call sites, which puts you further ahead on it than anyone.
    • #193 — audit and document every CLI exit code (level-5), if you'd rather stay near [#229]'s territory where you were already asserting exit-code parity. Fair warning that it has teeth: doberman scan exits 0 no matter what it finds, while its help text advertises it for CI gates.

    Both are a step up from the level-1 to level-3 range you've been working in, which I think is the right direction for you.

    There's also a Discord if you'd like a faster loop than PR comments — it's where the roadmap gets argued about and where "is this the right approach?" gets answered in minutes rather than days: https://discord.gg/Sfy5XGNqty. Entirely optional, the tracker works fine too.

     

    Related

    Tickets: #192
    Tickets: #229

  • Anonymous

    Anonymous - 2026-08-03

    Ticket changed by: fu351

    • status: open --> closed
     
  • Anonymous

    Anonymous - 2026-08-03

    Originally posted by: fu351

    All three are merged: [#228] (30c0960), [#230] (cfc45c2a) and [#229] (b57151c1). Issues [#224], [#225] and [#226] closed with them.

    You're added to CONTRIBUTORS.md in [#231], which is open now.

    One thing worth knowing, since it explains the wait on [#229]: its Windows leg took around 40 minutes, while the earlier red runs on that branch finished in about one. That isn't flakiness. The red runs died at ruff format --check, several steps before pytest ever started, so the commit that fixed the formatting was the first time the full Windows suite actually ran on your branch. Worth remembering when a green run looks suspiciously slower than the red one before it — sometimes the fast red run just means CI never got far enough to do the real work.

    The routing above still stands, and [#192] is the one I'd take.

     

    Related

    Tickets: #192
    Tickets: #224
    Tickets: #225
    Tickets: #226
    Tickets: #228
    Tickets: #229
    Tickets: #230
    Tickets: #231


Log in to post a comment.