feat(report): add bounded obligation execution accounting - #93
feat(report): add bounded obligation execution accounting#93solloek369-arch wants to merge 4 commits into
Conversation
lywinged
left a comment
There was a problem hiding this comment.
The emission boundary from #83 holds on this head, and it holds because the code enforces it rather than because the body says so. One row needs a decision before I would call the shape settled, and there is one claim in the body my run did not reproduce.
What I ran at e72a101 over e93f085: the full suite goes from 542 to 562 passing with the same two version tests red on both because the package is not installed in my environment, so nothing that existed changes status. On the public path, run() and the accounted _run_levels agree level for level on the fixture record and on records carrying non-ASCII strings, a float, an integer outside the JCS safe range and a NaN, and the JSON report's verdict, levels and findings are identical between base and head on all four canonicalization-boundary vectors from trace-spec. The verify command's own code is untouched. The runner beneath it now passes every checker through the accounting hook, which is a no-op without a capture, and verify output is byte-identical between base and head on the same five records at all three levels.
The counterfactual that matters is removing one _observe wrapper, the one on TR-SCA's digest_valid branch. verify output does not change at all. report refuses with TR-SCA returned an unearned or duplicate TR-SCA-002 Finding, and nine of the twenty accounting tests go red. So the boundary is load-bearing: a checker that stops witnessing its branch takes the accounted report down with it instead of shipping a row that says less than it did.
The row to decide is Level 0, TR-SCA-002. It reads applicable, not_attempted, level0_scheduler_nonexecution, with no finding and no reason. It is true on both documents the registry cites: the schema at c111c2f requires build_provenance unconditionally, and the runner's level table, as docs/levels.md also states, runs TR-SCA from Level 1. The obligation is covered by a checker, so the #83 constraint as written is met. The row is where the same question comes back one level down: it is the only row in the pilot that can tell a reader an obligation applied and the suite did not attempt it while carrying nothing to say why. The TR-POL-003 no_resolver row says applied and not attempted too, but its SKIP message names the missing resolver; this row has no finding, and the reason, the level schedule, is not on it. The comment on #83 that set the constraint already put it at row level: out of scope "with a reason is a perfectly good answer for a row". The issue's own draft carried state_reason and an UNDETERMINED applicability, and the implementation dropped both. Either put the reason on that row or project it not_applicable at Level 0 with the same reason attached. Which of the two is a shape call for the maintainers, and I would settle it before the pilot merges.
On the registry pins: all thirteen JSON pointers resolve at trace-spec c111c2f, and the values match the suite's packaged schemas/trace-claim.json. The packaged copy is trace-spec at c429dfc from 22 August, four schema commits behind the revision the registry names, and the test hashes the packaged copy. The two agree at every cited pointer today and nothing in the tree ties them together. A wrong pointer also ships: with enum changed to enums in the TR-APR sources, report still emits, and the only thing that goes red is the literal list in the test. The test already computes a hash of each locator's value; carrying that hash in the registry would make the registry self-checking rather than test-checked, which is closer to what #83 meant by re-checkable.
Main has moved under this branch: #94 added LIMITATIONS.md, and the branch still merges clean onto 37c79aa. That file is the prose form of what this PR does in JSON, a statement of what the suite did not establish, with the reason attached to each boundary. Its TR-ANC-002 entry is the model: the check "does not establish that the root is one a public log actually published", and establishing that the tree is real "is not in scope here". That declaration also settles the URI half of #92 and leaves the receipt half where it was. It is the standard the Level 0 row above should meet, and the pilot's own limits belong in that file now, in the words the issue already used: three rows, an unsigned self-report, and accounting_complete meaning the bounded set is accounted rather than TRACE covered. A report that carries an accounting object while the limitations page is silent on it is the mismatch that file exists to prevent.
The claim I could not reproduce is mypy. Under the repository's own strict = true with mypy 1.19.1, accounting.py adds three diagnostics the base does not have: two where capture.current_level, typed int | None, is packed into a tuple[int, str] cell, and one missing return in _role_projection, which returns None for any role outside the six it names. The base tree carries 28 of its own, so this is not a clean tree either way, but the body says none are new. Ruff holds: 55 on base, 53 on head.
Tool-assisted: the runs and this write-up.
e72a101 to
f1ad4d4
Compare
|
Thank you — I used this review as the boundary for a fresh current-main replay and updated the branch. The Level 0 The strict Mypy claim has been re-earned under 1.19.1 with dependencies present: base and head have the same three diagnostics. A cumulative pre-review pass additionally closed nested-run capture leakage, mutable-policy and renderer-snapshot divergence, missing-level/row relation gaps, and Python bool/float aliases for identity-bearing integer fields. Current public result: |
lywinged
left a comment
There was a problem hiding this comment.
Approved on this head, f1ad4d4 over b2a6490.
The three things that stood between the previous head and a merge are closed, and I checked each against the code rather than the comment. The Level 0 TR-SCA-002 row carries state_reason, "TR-SCA is not scheduled at attempted Level 0", and LIMITATIONS.md states the seam it sits on, the schema requiring build_provenance unconditionally while the runner schedules TR-SCA from Level 1. Every source locator now carries value_sha256; I recomputed the thirteen JSON-pointer digests from schema/trace-claim.json at c111c2f over RFC 8785 bytes and all thirteen match, and the new exact-text locator for TR-POL-003 occurs once in spec/trace-v0.2.md at that commit with a matching digest. mypy under the repository's strict config with 1.19.1: 28 on base, 28 on head, no difference.
The boundary is still load-bearing. Dropping the _observe wrapper on TR-SCA's digest_valid branch leaves verify byte-identical and makes report refuse. A pointer typo now fails at run time, "unbound normative source locator", rather than in a test. The suite goes from 542 to 618 passing with the same two uninstalled-package version tests red on both sides, which is the 620 in the comment.
One thing to read as declared rather than derived: the value digests are a literal table in accounting.py. A wrong pointer cannot ship; a wrong digest behind a right pointer can, since I flipped one hex character and report still emitted, with one test red. LIMITATIONS.md says the suite does not fetch or authenticate trace-spec at report time, so this is stated rather than hidden, and a reader holding the pinned bytes can check it.
The public-path parity checks from the first review are now tests in the branch, across the four canonicalization vectors the suite ships, the three attempted widths and the JSON boundary values, which is the right place for them.
What the maintainers still own is size. The PR went from 1,944 to 3,444 inserted lines and accounting.py from 645 to 1,099, in response to a review that asked for a reason string, a digest column and a limitations paragraph. Whether a three-row pilot should carry that much bookkeeping is the #83 shape question, and it is sharper now than it was.
Tool-assisted: the runs and this write-up.
|
@lywinged The size is fair, sorry! It grew past what a three-row pilot needs. I can bring a minimal variant: the per-obligation accounting and accounting_complete, without the registry/digest/locator layer, as a separate PR so the two can be compared? |
No second PR is needed from my side. The approval on |
Implements the bounded #83 pilot for
TR-APR-001,TR-POL-003, andTR-SCA-002using checker-owned executable metadata.for each declared pilot obligation and identity-bearing machine integers
have exact integer types;
The
report/1identifier, existing fields, verdict computation, and CLI exitbehavior are unchanged. JSON gains one additive, version-tagged
obligation_accountingfield. The repository does not currently ship a formalJSON Schema for this report shape, and compatibility with unknown consumers
requiring the exact historical top-level key set was not tested.
Checks on current base
b2a64909e0cd0fc8937c747f36efd293508c0056:620 passed, 5 xpassed;and installed CLI smokes: pass;
unchanged relative to the base.
The scope is limited to these three obligations. It does not claim complete
TRACE coverage, implementation minimality, independent execution proof, or
post-emission authenticity.
The review question remains whether this bounded accounting shape and additive
report/1emission boundary fit the direction agreed in #83.