fix(stella-pipeline): let the verifier read an untracked file's content, not just its name - #2034
Merged
Merged
Conversation
…ent, not just its name `gather_diff` described every untracked file the turn created or modified with one marker line — the path and an added-line count — because `git diff` cannot see an unstaged file and the numstat probe was the only thing asked about it. For a task whose entire deliverable IS an untracked file, that made the verifier a reviewer of filenames: it could confirm something had been written and nothing about what. A real run graded such a change PASS and said so in as many words, that "the unseen content cannot itself justify a FAIL". Adds `DiagnosticInvocation::UntrackedPatch` beside the existing numstat — the same `git diff --no-index` probe minus `--numstat` — and appends its hunks under the marker they belong to. Two deliberate restrictions: - The patch's `diff --git`/`---`/`+++` preamble is stripped. `warrant::changed_paths` reads `+++ ` lines as the set of paths a change touched, and an untracked path already reaches it through the marker. Letting both channels name the file would move untracked-only changes off the `paths.is_empty()` branch that keeps them `Required`, quietly relaxing when a witness is owed. This change makes the verifier see more; it must not make the warrant ask for less. - Binary content stays git's `Binary files ... differ` sentence. That is the useful evidence, and it is what keeps a database sidecar's bytes out of a prompt. The verifier's system prompt asserted this content was unrenderable, which this makes false, so it moves in the same commit.
Fails on the previous behaviour with the whole defect visible in the assertion output — the verifier's diff section is one line, `+ untracked change: regex.txt (+1 lines)`, and nothing else. Asserts on the verifier's own prompt rather than the probe's return value: the latter would pass while the text was dropped anywhere between the probe and the model. The double lives in verification_honesty.rs rather than extending ScriptedRunner in pipeline/tests.rs, which is a god file sitting exactly at its 2537-line ceiling. ScriptedRunner's fallback arm already answers UntrackedPatch with empty stdout, so no existing scenario changes shape.
…s own module The `UntrackedPatch` arm pushed `agent/tools.rs` to 1504 lines, over the 1500-line limit, and that file is not grandfathered — so the arm had to land somewhere else rather than the ceiling move. `impl DiagnosticRunner for GitDiagnosticRunner` is the natural seam: it is the one place a `DiagnosticInvocation` becomes a process, and it needs nothing from `tools` but the runner's root, its baseline commit, and the two spawn helpers. Moving it takes tools.rs to 1470. `agent.rs` grows by the one line that declares the module — the irreducible case the baseline documents an escape hatch for, so the +1 lands as a visible baseline diff. Regenerating also ratchets command_deck.rs (-55) and bus.rs (-235) down to what main already earned.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's GuideExtends the verification pipeline so untracked files’ content, not just their names and line counts, are surfaced to the verifier by adding a new untracked patch diagnostic, wiring it through CLI/server runners, and updating verifier instructions and tests; also refactors Git diagnostic wiring into a new module to stay within file size limits. Sequence diagram for untracked file verification with contentsequenceDiagram
participant Pipeline as Pipeline
participant DiagnosticRunner as DiagnosticRunner
participant Git as Git
Pipeline->>DiagnosticRunner: run_diagnostic(UntrackedNumstat{path})
DiagnosticRunner->>Git: diff --no-index --numstat -- /dev/null path
Git-->>DiagnosticRunner: CmdOutcome(stdout_tail numstat)
DiagnosticRunner-->>Pipeline: CmdOutcome
Pipeline->>Pipeline: untracked_added_lines(surface, path)
Pipeline->>DiagnosticRunner: run_diagnostic(UntrackedPatch{path})
DiagnosticRunner->>Git: diff --no-index --no-color -- /dev/null path
Git-->>DiagnosticRunner: CmdOutcome(stdout_tail patch)
DiagnosticRunner-->>Pipeline: CmdOutcome
Pipeline->>Pipeline: patch_body(stdout_tail)
Pipeline->>Pipeline: append untracked_change_line and patch_body to text
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
macanderson
marked this pull request as ready for review
August 7, 2026 04:41
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
This was referenced Aug 7, 2026
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…_deck.rs (#2044) **`main` is red.** `make file-size` fails at `6a41889a`: ``` crates/stella-cli/src/command_deck.rs grew to 4567 lines, over its baseline ceiling of 4566 (+1) ``` Every open PR inherits this, so it wants merging ahead of anything else. ## Cause — retightening skew, not real growth Nobody added 55 lines to `command_deck.rs`. #2034 regenerated the baseline and ratcheted that entry **down** from 4621 to 4566 (its size at that moment). A parallel PR that grew the file by one line was written against the old 4621 entry, so its own gate was green. Both landed, and the sum is red. `ci.yml` does not run on a push to `main` (#1986), so nothing reported it — the next PR to run the gate would have looked like the culprit. That retightening was mine, in #2034. Filing this rather than leaving it for whoever hits it next. ## Fix `make file-size-update`. Regenerated, not hand-edited: the baseline is generated and gate-enforced, so it is the only copy that can stay correct, and hand-editing one entry is how the last limit died. The regeneration also picks up `deck_render.rs`'s earned ratchet-down (1531 → 1529). That carries the *same* collision risk in the other direction, and there is no way to take the fix without it. The mitigation is merging this quickly rather than editing one line by hand — please do not let it sit. ## Verified `make file-size` — `OK — 1139 files, none over 1500 lines except 30 grandfathered (none grew)`. No source file changes; baseline only. ## Summary by Sourcery Build: - Regenerate scripts/file-size-baseline.txt so the file-size check passes with the latest line counts.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…ff channels (#2041) Follow-up to #2034, which merged before this landed on the same branch. ## The problem #2034 introduced #2034 made the probe half of the diff render an untracked file's content. The authored half (`FileTouchPort::authored_diff`) already rendered the same content for any file written through the file tools — and `absorb_probe` concatenates the two halves. So after #2034, a file written with `write_file` that is also untracked — the single commonest shape of agent work — reached the verifier **twice**: once as a probe hunk, once as an authored chunk. That spends the diff budget twice to say one thing, and leaves a reader to wonder whether it is looking at one change or two. ## The fix `DiffProbe` now names the untracked paths whose content its text carries (`untracked_rendered`), and `splice_authored` drops those chunks from the authored half. Reuses `verify::strip_witness_hunks` rather than writing a second chunk parser, so the two callers cannot come to disagree about where a chunk boundary is. Which half survives is deliberate, and follows the precedence `authored`'s own module docs already state — *probe first, since on-disk state is the stronger claim where it exists*. Only a **hunk body** claims the path: git's `Binary files ... differ` sentence is not content, so the authored channel still covers a binary file, and a path whose probe failed is likewise absent from the list. This is the text-side analogue of the `max` (never a sum) that `absorb_probe` already applies to the two channels' line counts, for the same stated reason — they are two views of one change, not two changes. ## Witness `a_file_the_probe_already_rendered_is_not_repeated_by_the_authored_half` asserts the content appears exactly once (`matches(...).count() == 1`), which is the property that actually matters and which a "does it contain" assertion would not catch. `a_file_the_probe_did_not_render_is_still_spliced` pins the other side: the drop is per path, and a file only the tools saw is still spliced — that being the entire reason the authored channel exists. ## Gate `guards-fast` (all 14), `clippy -D warnings` on `stella-pipeline`/`stella-cli`/`stella-serve`, and `cargo test -p stella-pipeline` — 605 pass, up from 603 by exactly these two tests. Refs #2034 ## Summary by Sourcery Prevent untracked files written by tools from having their contents rendered twice across probe and authored diff channels in the stella pipeline. Bug Fixes: - Avoid duplicate diff output for untracked files whose contents are present in both the probe and authored channels by dropping redundant authored chunks for those paths. Tests: - Add regression tests ensuring untracked files rendered by the probe are not repeated by the authored half, while files only seen by tools are still included in the final diff.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…ved out (#2045) **`main` is red.** `cargo check` / `clippy` fails on the **non-test** build at `94d013c5`: ``` error: unused imports: `DiagnosticInvocation` and `DiagnosticRunner` --> crates/stella-cli/src/agent/tools.rs:11:46 = note: `-D unused-imports` implied by `-D warnings` error: could not compile `stella-cli` (bin "stella") due to 1 previous error ``` My breakage, from #2034. Fixing rather than leaving it. ## Cause #2034 moved `impl DiagnosticRunner for GitDiagnosticRunner` out of `agent/tools.rs` into `agent/diagnostics.rs` (tools.rs had crossed the 1500-line limit). After that move, nothing **outside `cfg(test)`** in tools.rs names those two types, so the top-level import is dead in a non-test build. ## Why the local gate did not catch it — worth knowing `make lint` runs `clippy --all-targets`, which **compiles the test cfg**. Both types are still used there (`mod tests` at 844, `mod diff_baseline_tests` at 1365), so the import reads as live and clippy is green. Only a build *without* the test targets sees the unused import — which is exactly what CI's `cargo check on the declared MSRV` job runs. **`clippy --all-targets` is not a superset of the plain build for unused-import purposes.** A symbol used only under `cfg(test)` keeps a top-level import looking alive. Anyone extracting code out of a module wants to check both. ## Fix Import the two types per test module instead of at the top, so the non-test build carries nothing it does not use. No behaviour change, no test change. ## Verified both ways - `cargo clippy -p stella-cli --bin stella -- -D warnings` — the build that was failing. Clean. - `cargo clippy -p stella-cli --all-targets -- -D warnings` — clean. - `cargo test -p stella-cli --bin stella` — 1468 pass. ## Summary by Sourcery Resolve unused import errors in stella-cli by scoping diagnostic imports to test modules. Bug Fixes: - Fix non-test stella-cli build failure caused by unused DiagnosticInvocation and DiagnosticRunner imports. Tests: - Update test modules to import diagnostic types locally so they remain available only where used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
gather_diffdescribed every untracked file a turn created or modified with a single marker line — the path and an added-line count:git diffcannot see an unstaged file, and the numstat probe was the only thing ever asked about one. For a task whose entire deliverable is an untracked file, that made the verifier a reviewer of filenames: it could confirm something had been written, and nothing at all about what.This is not hypothetical. A real run of exactly that shape (write a regex to
/app/regex.txt) returned PASS, with the verifier saying so in as many words:The verifier's own system prompt told it to expect this — it asserted the content was something "no probe could render" — so the model was behaving correctly on the evidence it was given. The evidence was the bug.
The fix
Adds
DiagnosticInvocation::UntrackedPatchbeside the existing numstat: the samegit diff --no-indexprobe minus--numstat, appended under the marker line it belongs to.Two deliberate restrictions, both load-bearing:
diff --git/---/+++preamble is stripped.warrant::changed_pathsreads+++lines as the set of paths a change touched, and an untracked path already reaches it through the marker. Letting both channels name the file would move untracked-only changes off thepaths.is_empty()branch that keeps themWitnessWarrant::Required— quietly relaxing when a witness is owed. This change makes the verifier see more; it must not make the warrant ask for less.no_header_survives_for_changed_paths_to_readpins that.Binary files … differsentence. That sentence is the useful evidence, and it is what keeps a.stella/private/*.dbsidecar's bytes out of a prompt.The verifier prompt's claim that this content was unrenderable is now false, so it moves in the same commit.
Witness
the_verifier_reads_an_untracked_files_content_not_just_its_nameasserts on the verifier's own prompt, not the probe's return value — the latter would pass while the text was dropped anywhere between probe and model.Verified the artisanal way. With the append disabled, the test fails and prints the entire diff the old code handed the verifier:
One line. That is the defect, reproduced deterministically.
Why
agent/tools.rsmovedThe new match arm pushed
crates/stella-cli/src/agent/tools.rsto 1504 lines, past the 1500-line limit, and that file is not grandfathered — so the arm had to land elsewhere rather than the ceiling move.impl DiagnosticRunner for GitDiagnosticRunneris the natural seam: the one place aDiagnosticInvocationbecomes a process, needing nothing fromtoolsbut the runner's root, its baseline commit, and two spawn helpers. tools.rs drops to 1470.agent.rsgrows by the single line that declares the module — the irreducible case the baseline documents an escape hatch for. Regenerating also ratchetscommand_deck.rs(-55) andbus.rs(-235) down to what main already earned.Gate
guards-fast(all 14),wire-schema,clippy -D warningsand the full suites forstella-pipeline(603),stella-cli(1463) andstella-serve— all green locally.Exemplar
The probe/authored split follows
pipeline/authored.rs's existing rule that both channels must be unified-diff shaped so one parser reads the joined string. This change makes the tree-state channel hold up its end of that contract — the authored channel only ever covered files written through the file tools, so anythingbashcreated stayed invisible.Refs #1701
Summary by Sourcery
Extend verification to include the content of untracked files in diffs and wire a new untracked patch diagnostic through the pipeline and CLI, ensuring verifiers can review actual changes while preserving witness and safety guarantees.
New Features:
UntrackedPatchdiagnostic to retrieve unified diffs for individual untracked files alongside existing numstat data so verifiers can see their content.Binary files … differline.Bug Fixes:
Enhancements:
agent::diagnosticsmodule and expose minimal helpers fromagent::toolsto keep files within size limits and clarify responsibilities.Tests: