fix(stella-pipeline): do not render an untracked file through both diff channels - #2041
Merged
Conversation
…ff channels Rendering untracked content in the probe half made the authored half's copy of the same file redundant, and the two are concatenated — so a file written through `write_file` reached the verifier twice, spending the diff budget twice to say one thing and leaving a reader to wonder whether it was looking at one change or two. `DiffProbe` now names the untracked paths whose content its text carries, and `splice_authored` drops those chunks from the authored half via the existing `strip_witness_hunks` — one definition of "a chunk for this path", so the two callers cannot disagree about a chunk boundary. The probe's copy is the one kept, by the precedence `authored`'s module docs already state: on-disk state is the stronger claim about what survived. 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. This is the text-side analogue of the `max` (never a sum) that `absorb_probe` already applies to the two channels' line counts, and it is there for the same reason: two views of one change, not two changes.
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
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's GuideEnsures untracked files rendered by the diff probe are not redundantly rendered again in the authored diff, by tracking which untracked paths had their content emitted by the probe and dropping those chunks from the authored half while reusing shared chunk-parsing logic, plus tests to pin the new behavior. Sequence diagram for combining probe and authored diffs without duplicate untracked contentsequenceDiagram
participant Pipeline
participant DiffProbe
participant AuthoredChange
participant authored as authored_module
participant verify as verify_module
Pipeline->>DiffProbe: gather_diff()
DiffProbe-->>Pipeline: DiffProbe{text, available, untracked_rendered}
Pipeline->>AuthoredChange: collect_authored_changes()
AuthoredChange-->>Pipeline: AuthoredChange{text, ...}
Pipeline->>authored: splice_authored(probe.text, &authored, &probe.untracked_rendered)
activate authored
authored->>verify: strip_witness_hunks(&authored.text, already_rendered)
verify-->>authored: StripResult{diff}
authored-->>Pipeline: combined_diff_text
deactivate authored
Pipeline->>Pipeline: verification_honest_diff(combined_diff_text)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
… double `stella-pipeline`'s `patch_body` makes two assertions about what this argv prints: everything above the first `@@` is strippable preamble, and a binary file yields `Binary files ... differ` and no hunk. Every test of those rules lives over there and runs against a scripted string, so the double and git could drift and only a live run would notice. These two run the real `GitDiagnosticRunner` against a real temp repo. They cost no model call and no network, which is the point — the gap was reachable without either.
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
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.
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 — andabsorb_probeconcatenates the two halves.So after #2034, a file written with
write_filethat 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
DiffProbenow names the untracked paths whose content its text carries (untracked_rendered), andsplice_authoreddrops those chunks from the authored half.Reuses
verify::strip_witness_hunksrather 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'sBinary files ... differsentence 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) thatabsorb_probealready 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_halfasserts 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_splicedpins 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 warningsonstella-pipeline/stella-cli/stella-serve, andcargo 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:
Tests: