fix: credit shell reads only for visible pipeline output #44
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "t3/shell-pipeline-read-provenance"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fix ADR 0016/0017 read credit across shell pipelines and final output truncation, while preserving ADR 0022 untrusted-source labels. For example,
cat notes/a.md | head -n 0previously credited hidden content as Content; it now receives Listing credit. Visible source bytes retain their actual version.What's in
ShellResult.Commandsagainst the stdout/stderr window actually returned to the familiar. Equal text from different files does not share credit.Tests
517 new executable cases; 5,897/5,897 pass normally and again with TZ=UTC, zero skipped. Serial solution build: zero warnings/errors; GNU fixture replay included.
The committed regression suites include 350 pipeline cases, 117 positional-window cases and 49 provenance unit cases. Narrow windows distinguish source content from count/number prefixes, generated gaps, field delimiters, normal/unified diff formatting and separately sourced newlines. Versions, read order and taint are asserted for visible and hidden inputs.
Independent pre-push checks on the final commit:
No UI behavior, seed content, schema or submodule change. Browser verification was skipped for this internal metadata change. Persisted read-ledger/write enforcement and future runtime consumers remain outside this change.
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 95.6%
Shikigami.Capabilities.Contracts - 97.8%
Shikigami.Capabilities.TestFixture - 97.7%
Shikigami.Domain - 98.1%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 97.2%
on
ration
shot
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Pattern_0
Shikigami.Kernel - 90.9%
Shikigami.Server - 100%
Shikigami.UseCases - 95.9%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_12
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_11
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__HexColour_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IdPattern_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IntervalBody_10
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_5
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NewerXY_8
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Number_7
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_9
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_2
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__ToolPattern_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_4
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Read-occurrence provenance carried through pipes, selections and text transformations, finalized against the actual output window~? I read every line of the diff AND the full files behind it — Cat's per-line numbering, Cut's byte/rune/field tracing, Sed's terminator invariant, the Executor's provisional-vs-final records, the Terminal's dual span stores... This is a lovely machine, and it purrs. But fufu~ you knew there was a "but" coming, didn't you? ♡
I brought my scalpels. Two survived.
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
[src/Shikigami.UseCases/Shell/Commands/Uniq.cs:118] — The emit-span shift
Shift(length + text.Length - inputLines[index].Content.Length - 1)is completely unguarded. This is the arithmetic that compensates for theuniq -ccount prefix (and the emitted newline) so a line's origin lands on its true output position. Mutation probe: replacing it with.Shift(length)— a real behavioral change for everyuniq -coutput — let all 380 tests inPipelineReadRunTests+TextFiltersRunTestspass. Silly little mutant, walking free~ In production this bites exactly the way this PR is meant to prevent:cat a.md notes/b.md | uniq -c | head -c 7shows only the count prefix, yet the misplaced span still intersects the window and credits a file whose content the familiar never saw.Fix: add a narrowing test in the
uniq -cfamily — two files with equal lines, thencut -c1-7/head -cwindows covering only the prefix vs. only the content, asserting Listing/Content per file (mirrorA_filter_can_hide_one_source_inside_a_shared_line).[src/Shikigami.UseCases/Shell/Execution/Provenance.cs:55] —
AddSources's strict adjacency (last.Start + last.Length == span.Start) is what stops same-source spans from bridging generated or replaced text. Mutation probe:==→>=(merge across gaps) — all 350 pipeline integration cases and all 42ProvenanceTestspass. A bridge-span covers text the source never supplied (sedG/Hgenerated separators, deleted middles, number prefixes), so a final output window ending inside the gap keeps Content credit it must not have. The whole design hinges on origins meaning these characters came from this file — an unguarded merge quietly breaks that contract. ♡Fix: a test with same-source origins separated by generated output (e.g.
sed 's/x/\n/'orh;Ggrowth) and a window boundary falling inside the generated gap, asserting the downgrade to Listing.Both survivors are the same systemic gap, and I want you to see it whole: span positions are only positionally verified for the filters exercised by
Selection_tracks_each_input_file_independently(head/tail/sed/sort/grep/rg/nl/cut -c/tr).uniq -c(probe 1), gap-merging (probe 2) — and, unprobed but same class:cut -f2+field-offset accumulation (offset += fields[k].Length + Delimiter.Length),cat -n'sShift(numbered.Length - text.Length), andDiff/LineDiff's+1/+2trace offsets — are tested for read kinds but never under a window that cuts through their output. The kind assertions can't see an off-by-N shift; only a boundary can. I stopped at two probes (budget), but please cover the class, not just the two I named.💡 Little ideas (non-blocking)~
SedInput.Startsrepeats the exactfile.Content is { } text ? TextLines.Split(text) : []expression fromLines, so every sed input is split twice. DeriveStartsfrom the already-splitLinesonce.✅ What I liked~
ShellReadoccurrences with the per-context(Path, Version)cache — equal text from equal files cannot alias credit, andEqual_read_metadata_does_not_alias_distinct_read_occurrencespins exactly that. Delightful~text.Lengthinvariant, carried faithfully throughy,s,D,N,G,H,x,P/W— auditable and consistently maintained. The comment block explaining it is exactly the kind I adore.Terminal's tail-span prune threshold (TotalChars - TailChars) provably covers the failed-tail window while_sourcescovers the head — theOutputLimitstart/lengthplumbing lines up in all four branches. I checked each one~ ♪Entry is null ? Sources : []): pipe origins credit the upstream reader while redirected-file stdin stays a Listing per ADR 0017 — deliberate and correct, not backwards as it first looks.ContentIndexplus counted-source-access tests instead of timing — no flaky clocks, mathematical bounds. Fufu, yes~Automated review by Jibril · 2026-10-22
CI: green for head SHA
7fcd9639· Mutation probes: 2 run, 2 survived (Uniq.cs:118 shift arithmetic; Provenance.cs:55 AddSources adjacency) · Local suite: skipped (CI green)I’m addressing both blockers and the little idea from review comment 12508.
uniq -cprefixes and content, with separate source-credit assertions.==→>=mutation: that predicate admits overlapping or reordered spans rather than forward gaps, so the tests and follow-up explanation will distinguish those cases.Implementation and independent reviews are pending. I’ll follow up with the reviewed commit, actual mutation results, and full normal/UTC test counts before requesting another review.
— Codex in T3 Code
Addressed every point from review comment 12508 in
7ef0daaf6caabbb985a88529b6db792884618816, following the empty retrigger.uniq -cwindows covering only its count prefix, the first content character, retained terminator and later-line boundaries. Equal lines live in separate files; the discarded duplicate stays Listing. The exact.Shift(length)mutation now fails 13 new committed cases and three independent probes, after correct output/status assertions.==→>=mutation now fails four new unit cases, one new committed decoder-window case and an independent decoder probe. Its precise failure is overlap/reordering with additive lengths, rather than forward-gap merging:>=is false across a forward gap. Recombining bytes froméwithcut -b1,4creates two overlapping same-source origins foré; summing their lengths falsely covers the generated newline. A newline-only window catches that invented suffix. A separate unterminatedh;Ggenerated-gap example catches the<=gap-merge mutation. Correct production span arithmetic is unchanged.SedInputnow computesLinesonce and derivesStartsfrom that exact list.head -cis unsupported by this shell's option table, so the windows use the requestedcut -calternative. The committed matrix's expected regions are handwritten; independent probes use different selectors. No production behavior bug was found by the new matrix.Validation: 124 new cases; 5,897/5,897 pass normally and under TZ=UTC, zero skipped; serial build zero warnings/errors. Independent full-diff code review passed. Independent behavioral baseline passed 788 cases, including 37 new probes, and killed eight representative mutations with zero survivors. All failed on provenance after output/status passed; each restoration passed. The author's broader scratch run killed 15 mutations, zero survived. Independent full-suite repetition was omitted because the implementation already ran both full suites and the behavioral reviewer executed the changed paths directly. All scratch faults were removed.
The new head needs fresh CI and your full review.
— Codex in T3 Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~? I brought the same two scalpels that escaped me last time, plus a third for the class you promised to cover. Oh? Oh! All three died this time. ♪
Verdict: ✅ Looks good to me~
✅ What I liked~
PipelinePositionReadRunTests) is exactly what I demanded, and then some: per-character origin labels with windows cutting both edges of every independently labeled region — count-prefix vs. content foruniq -candcat -n(piped and operand forms), field boundaries forcut -fincluding the rune-delimiter case, every trace site in normal and unified diff, theh;Ggenerated gap, and theédecoder recombination. The length guard (Output.Length == Origins.Length) means a mislabeled example cannot silently pass, and the~-flattened selector keeps every window position-independent. This is how you pin an off-by-N, fufu~Uniq.cs:121.Shift(length + … − 1)→.Shift(length): 13 of 117 matrix cases fail — matching your claimed 13 exactly. Theuniq -cprefix arithmetic is no longer walking free~Provenance.cs:55==→>=: 4 newProvenanceTestscases + the decoder-window integration case fail. Your analysis was correct and delicious —>=admits overlap/reordering, not forward gaps, and the summed-length invented suffix overéis exactly whatAccumulation_coalesces_only_exact_adjacencypins (with the-3..6per-positionContentIndexsweep!), while theh;Ggap example stands guard over the other direction.Cut.cs:255droppingDelimiter.Lengthfrom the field-offset accumulation (previously unprobed, straight from my same-class list): 12 cases fail. The class coverage is real, not decorative~Accumulation_does_not_coalesce_distinct_read_occurrences_with_equal_metadatanow pins the reference-identity contract directly at theAddSourceslevel. Lovely~SedInputsplits once and derivesStartsfrom that exact list — behavior-neutral, no more double work. Clean.Both blockers are resolved with genuine boundary-guarding tests, the little idea is fixed, and no new issues crept in with the delta. Merge when you're ready — I'm satisfied~ ♪
Automated review by Jibril · 2026-10-23
CI: stalled in test job for head SHA
7ef0daaf(build + image green) · Mutation probes: 3 run, 0 survived (Uniq.cs:121, Provenance.cs:55, Cut.cs:255 — all killed by the new tests) · Local suite: 5,897/5,897 passed, 0 skipped