I have been building with coding agents daily for a while now. They are good. The
problem I keep running into is not that they write bad code — it ...
For further actions, you may consider blocking this person and/or reporting abuse
That framing - tests answer correctness, not state - is going to stick with me. I've definitely shipped things where tests passed but the actual diff told a completely different story.
Thanks — “tests passed but the diff told a different story” is exactly the distinction I was trying to get at.
The tests weren’t wrong; they were answering a different question from the one you had at review time. Appreciate you reading.
Strong separation of generation, verification, and acceptance. I’d add one more boundary: byte-level integrity and semantic admissibility should produce different findings. In a research pipeline, a source can be byte-identical yet stale for the current as-of date, or legitimately changed because an official filing was amended. I would freeze evidence metadata alongside the corpus—source URI, retrieval timestamp, effective date, parser version, and schema—and return separate integrity, freshness, and lineage statuses. That lets the human reviewer distinguish unauthorized mutation from expected evidence evolution. Have you considered versioning a policy manifest alongside the corpus manifest so permitted roots, freshness windows, and parser versions are reviewed rather than embedded in the verifier?
This is the same structural lesson as the authority bug, one level up.
Freshness windows and parser versions are policy, and policy hard-coded in a
verifier is invisible from inside your own tests for exactly the reason a
hard-coded authority identity is. It also lands on a boundary the repo declares
rather than hides — docs/LIMITATIONS_AND_FUTURE_SEAMS.md opens with "Hashes
prove byte identity, not semantic truth or provenance."
Concretely, the corpus module (PM-04) answers both of your cases correctly and
helps the reviewer in neither, in opposite directions. A source that is
byte-identical but stale verifies clean — correct about the bytes, silent about
admissibility. A legitimately amended filing raises
CI03_SOURCE_CHANGED—also correct about the bytes, and equally silent about whether that change was
expected. The finding vocabulary has one axis, so nothing in the output
distinguishes unauthorized mutation from expected evidence evolution.
Partial correction, since some of what you're asking for is already there:
permitted roots and exclusions are frozen into the manifest header alongside
the rule-set version, so bounded scope is reviewable as data, not embedded in
the verifier. What's entirely absent is anything time-shaped — source records
carry root, relative path, size, and sha256, with no retrieval timestamp, no
effective date, no notion of parser or source schema. (
schema_versionexists,but it versions the manifest format itself.) So the policy manifest you
describe is skeletal for scope and absent for freshness and lineage.
The boundary I'd want kept sharp is that byte integrity is intrinsic to the
frozen evidence, while freshness is contextual — "stale for the current as-of
date" is a question about something outside the evidence set. That doesn't make
it non-deterministic; it makes the context an input. The check stays
reproducible only if the as-of date is an explicit call-site parameter, never
read from the artifact — same rule as
--authority-id. And note the recursionyour proposal implies: the policy manifest then needs its own trust anchor,
also supplied out of band, or the self-authorizing artifact has just been
rebuilt one level up.
Given that input, the three-way split looks right, and part of it already fits
the existing model: freeze is no-clobber by construction — writing different
bytes to an existing manifest path is a collision finding, not an overwrite —
so an amended filing has to become a new manifest artifact rather than mutate
the old one. What's missing is that the tool reports the drift as an integrity
event with no vocabulary for calling it a lineage one.
That's a larger change than adding fields, so I'd rather think it through than
bolt a timestamp onto a source record. But integrity / freshness / lineage as
separate finding families rather than one validity bit — yes.
Curious how you handle the as-of date today, if you have a pipeline doing this:
pinned per run, or derived from the filings themselves? That choice seems to
determine most of the rest.
Pinned per run. I treat evidence time and execution time as separate fields, with an explicit as_of timestamp supplied by the run context. Filings contribute their own filing, effective, and amendment dates; they never silently set run time. The acceptance layer evaluates freshness relative to the pinned as_of, so the same evidence snapshot, policy version, and as_of reproduces the same statuses. For amendments, I preserve both artifacts and link predecessor/successor lineage: the newer filing may supersede applicability without erasing the older byte-integrity record. The trust-anchor recursion remains, so the policy manifest or signature must come from a separate approval boundary.
Pinned per run answers it, and the lineage consequence is the part I hadn't worked through cleanly. Preserving both artifacts and linking predecessor to successor means supersession can change what's applicable now without touching what bytes were validly observed then. A single "current" artifact can't express that — overwriting is the only move it has.
Your tuple also separates three things I'd been running together: evidence state, policy state, and run context.
as_ofbelongs to the third; filing, effective, and amendment dates stay intrinsic to the first.Last time I said that policy hard-coded in a verifier is invisible from inside your own tests. I went and checked my own, and the real shape is narrower than that, and worse. It isn't invisible. My manifest header carries a
rule_versionfield, written at freeze time, sitting right there in the artifact. Verify never reads it. I replaced it with a garbage identifier naming a ruleset that doesn't exist, and verification passed clean — exit 0, no findings. Theschema_versionin that same header is enforced and rejects immediately.So the two version fields sitting side by side are doing different jobs: one is a gate, the other is a record. And the recorded one is precisely the field that purports to identify the rules the artifact was frozen under. Writing that identifier into the artifact turns out not to be the same thing as making it bind. I had those conflated.
Which is what I'd want to know about yours. Is the policy version in that tuple read at evaluation time and compared against what's actually running, or recorded alongside the result? If the freshness evaluator or schema interpretation can move while the identifier stays fixed, then the same evidence, same nominal policy, and same
as_ofcan still yield a different status, and the record won't tell you why.And when the policy manifest comes from a separate approval boundary — is the approved object a bundle that pins the evaluator together with the policy text, or the policy on its own, with the evaluator as an independent input beside it?
The lineage idea may generalize past evidence, is what I keep circling back to. Superseding a policy has the same shape as amending a filing: you'd want the old decision context preserved, rather than a newer evaluator silently reinterpreting an older run.
That garbage-rule-version test is the right falsification. My tuple was meant to identify the evaluated policy, but your result shows identity alone is not binding. The evaluator should fail closed unless (1) the declared policy version resolves to an immutable ruleset digest, (2) the digest actually loaded at execution matches it, and (3) the evaluator and schema versions are recorded separately. I would add a replay test that swaps each identifier independently and expects a hard failure; otherwise identical evidence, nominal policy, and as_of can still conceal semantic drift. I'd treat the previous tuple as incomplete without those bindings.
I think “identity alone is not binding” is the right correction.
The replay test is especially useful: change the declared rule version, the loaded ruleset identity, the module/evaluator version, and the schema version independently, then require each mismatch to fail for the reason it was meant to test.
There is one recursion I’d keep explicit. If a policy version resolves to an immutable digest through a registry, that registry becomes a trust anchor too. It has to come from outside the artifact being evaluated, otherwise the hard-coded constant has only moved.
The current implementation doesn’t close that whole loop. It records module_version and rule_set_version separately, and this generation explicitly forbids schema_version in the runtime result, but it does not bind a declared rule version to the exact ruleset bytes that executed. So I agree the earlier tuple is incomplete.
I’m keeping that as a falsification result rather than turning it straight into another feature. The stronger binding needs a real consumer case before it becomes product surface.
Exit codes as a versioned public interface is the line I would put in front of people, because it is the part teams skip and then rediscover the first time a CI job branches on "nonzero" and swallows a real finding. Two things I would push on. First, bounded cuts both ways. A manifest over explicitly supplied roots is honest, and it has the same blind spot as a test suite: it covers what you remembered. I would want the manifest to record what it did not cover, so the gap lives in the artifact rather than in somebody's memory. Second, I think you concede too much to git diff. Your mechanism is a claim about a state you reviewed and accepted, not about a commit, and those two come apart precisely when the agent is the thing making commits.
The exit-code line is the one I'd defend hardest, so after your comment I went and audited mine. It came apart in a direction that makes your point rather than mine.
2carries two public meanings: an input-class finding, and an argparse usage error. One way to reach the second is to omit a required nested subcommand — while the bare top-level command treats the analogous omission differently and exits0after printing help. So an analogous omission returns0at one level of the parser and2at the next, and the2also collides with a domain finding code that's supposed to mean something specific.And I've been calling
2contractual when it is documented nowhere: not in the README, not indocs/, not incontracts/. The public documentation mentions exit codes exactly once, to say a CI job should treat nonzero as review evidence. The test suite pins2and3and never asserts4— the integrity code, which is the one everything below turns on. "Versioned and tested like any other public interface" is what I argued for; it isn't what I shipped.On bounded cutting both ways: you're right, and the manifest is about a third of the way there. It records the exclusions it was given, so instructed gaps are in the artifact. The summary carries a
special_file_skip_count— things it walked past and refused to hash — but that's a bare integer with no paths and no reasons: the shape of your objection rather than an answer to it. And the one exclusion the tool applies on its own initiative, keeping its own output out of the corpus it writes, never enters the manifest itself; when that self-exclusion is relevant because the output sits under a source root, it surfaces only at run time.The sharper version showed up when I tried to attack it. Change a byte in a recorded file: HOLD, exit 4. Now shrink the declared scope, two ways.
Add an exclusion so that an already-recorded file falls outside it: caught, because the header and the records now contradict each other.
Remove the record instead, and the smaller manifest is internally consistent. Change that file and verify passes, exit 0. With
--detect-newit comes back as a WARN, and verify has no--profileflag to make warnings blocking; passing--profile strictgets you a usage error and exit2, the first defect biting the second.Delete the record and the file together and there is nothing at all: no finding, no WARN,
--detect-newincluded.So the asymmetry isn't widening versus shrinking — both of those were reductions. It's that the checker detects a scope change which leaves the manifest contradicting itself, and cannot distinguish a legitimately small scope from a self-consistently reduced one.
Forgetting isn't an error state.
That's "it covers what you remembered" made mechanical — except the remembering stays editable afterwards, and the edit doesn't have to lie about anything, because a smaller manifest is a perfectly well-formed manifest.
Which is where recording the gap runs out. Writing the uncovered set into the artifact does move it out of somebody's memory, and it's worth doing. But the artifact remains the only party asserting what the scope was.
The checker defends the declared set; it cannot establish that the declared set is the set that should have been declared.
Change a file and rewrite its hash into the manifest and that passes too: verify treats the manifest as the thing that is true. Telling those apart needs a second statement of expected scope — one supplied at the call, or a previous manifest. I have neither, and the second absence is the larger one: nothing in the tool treats a manifest as having a predecessor. No parent reference, no record-set diff.
To be clear about what I'm not conceding: inside a manifest whose scope is right, the integrity check does what it claims. It caught every content-level tamper I threw at it, deterministically.
What fails is the idea that the artifact can establish its own scope. The tool is narrower than I described it, not empty.
None of these defects is fixed; they're logged. Changing what
2means is itself a contract change — the versioning problem I was arguing for — and I'd rather do that deliberately than quietly in a patch release.On
git diff, I concede more than you asked for.The differences I listed were file-set differences — several roots at once, deliberately untracked files, archive members, symlink identity. That's an enumeration, and enumerations lose. Your version is categorical and it's the better argument.
A commit identifies a repository state; acceptance is a decision about a state.
The two coincide only when the accepted-state reference is pinned to that exact commit or tree — which it can be, and often is. But an agent can commit its own work and make
git diff HEADclean by construction without anything having established that HEAD was accepted.So the problem isn't Git. It's treating a producer-controlled moving reference as though it were the acceptance anchor.
The uncomfortable follow-on is on my side.
My freeze end at least takes an input that is out-of-band relative to the manifest: the roots come from the invocation rather than from the artifact being created. But out-of-band relative to the artifact is not the same as independent of the producer.
If the same agent picks the roots, freezes them, and later verifies them, that input was never independent of anybody — "produced" and "accepted" collapse into one act, and nothing in the output records which one you're looking at.
My verify end doesn't even have the first property; it reads its scope back out of the artifact.
What makes that worse is that the same codebase already implements the structure I need, one module over.
For an executed mutating action, the closeout checker requires an expected authority identity supplied by the caller; if none is supplied it holds — even when the document under test contains a well-formed authorization record naming a legitimate decider.
There's a test pinning exactly that property, named for it: a recorded authority cannot replace the caller's expectation.
That is what scope is missing, implemented and tested, in the same package.
I didn't fail to think of it. I applied it on one axis and not the other.
So there are three places the expected scope could come from.
A caller-supplied declaration, stale and noisy — but at least disagreement with the artifact can fail closed instead of being silently absorbed by it.
Derive it from the repository, and you've moved the problem up a level.
Or diff against a predecessor, which buys continuity rather than acceptance unless that predecessor is one somebody accepted: "previous" is a time relation, "accepted" is an authority relation, and only the second anchors anything.
My own argument points at the first.
Given the CI failure you opened with, what I'd want to know is whether you've seen a derived expected-universe hold up in practice — because if it has, I'm wrong about where the recursion stops.
We tried this exact pattern first: second LLM pass as reviewer on our entity linker decisions, metrics looked promising on the test set. Didn't catch the real problem for six weeks. The merge errors that were systematic just agreed with each other - the romanization blindspots that tripped up pass one weren't visible to pass two at all. We had to take the LLM out of the accept/reject gate entirely and replace it with a hardcoded structural check.
Six weeks is the part I'd want to sit with. The agreement wasn't just failing to catch the error — on that failure class, it was also depriving you of the disagreements that might have made the pattern visible sooner.
The way I'd put your result is that a second pass is useful as an independent boundary only to the extent that its failure modes differ from the first on the cases that matter. "Different model" is at best a proxy for that. Independence can come from different evidence, but also from a different representation, rule set, or decision procedure. In your case the two passes shared the romanization blind spot, so their agreement carried very little independent information exactly where you needed it most.
That's the part I think is underrated. On a shared systematic failure class, a correlated reviewer doesn't fail neutrally: it produces fewer disagreements, which means fewer cases get surfaced for inspection, which means it can take longer to accumulate enough examples to name the failure. Random errors tend to create visible disagreement. Systematic shared ones can come back looking like consensus.
That also changes how I'd evaluate a reviewer. A 99% agreement rate by itself tells me almost nothing about whether the second pass is useful. What I care about is its marginal error detection: which failures does it surface that the first pass would otherwise miss, and are those disagreements cheap enough to inspect? Aggregate accuracy still matters, but agreement is not independence.
The clearest case I've had of both halves was in the toolkit itself. It shipped with an authorization check tied to a maintainer-specific identity, so the public contract did not generalize to an external maintainer. Reading the code, I then got the trigger condition wrong — confidently wrong, because the authorization check is only reached for an action that is both executed and mutating.
What settled it was a synthetic differential: governance packs held constant except for the deciding identity, exercised against explicit expected-authority inputs. The useful part wasn't another interpretation of the code; it was constructing cases where the expected distinction was known in advance, then asking the implementation to make it.
The fix exposed the other half of the problem. My first instinct was to let the input document declare its own authority. That would have replaced a hardcoded identity with a self-authorizing artifact. The boundary only became meaningful when the expected authority moved out of band, as a caller-supplied parameter. What made the review useful wasn't agreement with my diagnosis; it was disagreement with my proposed fix, for a reason that could be checked.
On the structural check you ended up with, I'd keep the claim narrow. It isn't a correctness oracle. It can only encode an invariant someone already knows how to state. Your romanization invariant was apparently expressible after the failure class had become visible; the deterministic gate didn't discover that knowledge. What it gives you after that point is reproducibility — the same bounded inputs exercise the same explicit rule, instead of asking another model to rediscover the judgment.
So the practical version, for me: don't ask whether a second pass is "independent" in the abstract. Test whether it adds signal on the failure classes you actually care about. Aggregate metrics on a held-out set can still hide a shared blind spot if that class is underrepresented, unsliced, or averaged away. Seeding known instances of a specific failure class gives you something narrower but cleaner — ground truth by construction, for that class.
The obvious limit is that you can only seed failures you've already named, which is exactly the loop your six-week story exposes. So for unknown failure classes the useful property may be less "the reviewer is usually right" than "the disagreements it creates are cheap enough to inspect that the rare useful ones survive triage."
What finally made the romanization failure expressible after six weeks — had enough examples accumulated that the invariant became obvious, or did one person notice a pattern the review process had never surfaced? Those are pretty different failure modes.
Had the same issue. I'd deployed an agent that was quietly rewriting Neo4j edges during retrieval passes, and tests passed because the fixtures were pre-baked snapshots rather than live reads. It's exactly the correctness-vs-drift gap you're naming. The bounded scope part matters a lot; in my case it crept from "just the src/ dir" to "everything except node_modules" within two sprints, at which point the manifest was catching basically nothing.
You're right that this is the part that matters, and I got it wrong on the first pass — I tested the adjacent direction and called it a reproduction. Worth correcting in public.
What I'd checked was widening the exclusion list and watching coverage shrink silently. Real gap, wrong failure. Yours is the opposite: the covered set grew. So I ran that instead.
A small synthetic reproduction:
A root = src/ 2 files
B root = project/, exclude node_modules 32 files
one real edit under src/, plus a normal rebuild:
A verify → 1 finding (the one you'd want)
B verify → 31 findings (30 of them build/cache churn)
So "catching basically nothing" is right operationally, but the mechanism isn't that detection stopped. It detects harder than ever. B is a wall of legitimate findings with the one that matters buried at a signal-to-noise ratio of 1:30. Both runs fail; only one is readable. A check that fires on routine churn quickly becomes background noise. The failure mode is different from silence, but the operational result can converge on the same thing: nobody reads it.
That's why I think widening scope has to be treated as a change to the contract rather than just an edit to a config line. Bounded scope isn't a performance knob; it's part of what gives a finding meaning. Widen it and you haven't merely loosened the check — you've asked it a different question.
For what it's worth, on what actually holds that line, in decreasing order of how much I trust it:
The mechanical version is to make widening produce an artifact rather than be an edit. Freeze here is no-clobber, so a wider scope can't overwrite the baseline in place — you end up with two manifests whose headers each state their own roots and exclusions. That's half a ratchet by accident; the half that's missing is anything that consumes the pair. On your side that doesn't require changing the verifier: keep the previous manifest and require the header diff on the PR that widens scope. The point isn't the diff, it's that the widening now has to be addressed by someone instead of succeeding silently.
The version I'd trust least is the same rule written down as a principle. I've watched that fail on my own work — four rules written for one project, three of them violated within days, and the only one that held was the one I happened to still be watching. That experience changed how I classify rules: if nothing mechanically observes the invariant, I treat the rule as documentation rather than as a control. A paragraph in a README can explain a boundary; I wouldn't count it as enforcing one.
In between, and cheap: don't ask the config whether the check still works, ask the check. Inject a known meaningful mutation and confirm it still emerges clearly above the routine churn. That doesn't prove the checker is complete — it proves something narrower and operationally useful, that a failure you care about is still visible under the scope you have actually configured.
If you want a number rather than a judgement call, the cheapest one I've found is the noise floor: run verify after routine churn with no meaningful change, and count what comes back. Same three-step creep, measured:
just src/ 2 files noise floor 0
Each step is locally reasonable, and the floor moves on the first run after it rather than two sprints later. A non-zero floor means every real finding now has to be distinguished from routine churn before the result is actionable.
Which is the other thing your comment turned up. I had assumed scope worked like the authority identity: supplied independently at the call site and not taken from the artifact being checked. That's true at freeze time. At verify time there is no caller-supplied expected-scope input: the source records come from the manifest, exclusions come from its header, and --detect-new also uses the header's roots to define what it scans. So the baseline carries its own verification scope. That's weaker than the boundary I thought I had.
Related and smaller: there's a finding named CI07_UNBOUNDED_ROOT that sounds as if it guards breadth, and it doesn't. It fires for an empty root list or for a root that isn't an existing non-symlink directory. CI07 itself accepts an existing directory regardless of how broad it is.
On the Neo4j half, PM-04 has a more basic limitation: its source types are filesystem files, symlinks and ZIP members. There's no graph-aware source, so it has no direct way to observe a live edge mutation as graph state. At most it could notice byte drift if that mutation surfaced in files that had deliberately been included in the corpus.
Was the widening one decision in your case, or accretion — a run of "just add this one directory" changes? Would a scope-diff review gate have helped, or did each incremental widening look locally reasonable enough that it would have passed review anyway?