cli: make policy-history --json deterministic like other JSON commands
Your AI's guard dog to stop it from going rogue.
Brought to you by:
doberman
Originally created by: AshSgDe29071999
policy-history --json used bare json.dumps(rows, default=str) while scan/doctor/log use sort_keys=True and compact separators.
sort_keys=True, separators=(",", ":") to match siblingsFixes [#226]
Originally posted by: fu351
Reviewed — merge-ready, nothing to change. I checked all four
json.dumpscall sites:main.py:242,:568and:810already hadsort_keys=Trueand compact separators, and:992was the lone outlier exactly as you described. All four agree now. Sincesort_keys/separatorstouch 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
mainand watched it fail, then re-applied your fix and watched it pass. Three deliberate choices got you there:expectedfromjson.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.policy_changesschema 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:
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.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 scanexits 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:
#192Tickets:
#229Ticket changed by: fu351
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.mdin [#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:
#192Tickets:
#224Tickets:
#225Tickets:
#226Tickets:
#228Tickets:
#229Tickets:
#230Tickets:
#231