Repository navigation
fix(bgtail): a wider lines window returns the earlier lines, not "no new lines" - #23
Merged
Merged
Conversation
…new lines" The delta bookmark recorded the log path and the search window but not how many lines the caller had already been shown, so a repeat read asking for a WIDER window was answered from the appended-lines path: `fresh` was empty, and the tool replied "(no new lines since last read — log at N lines)". That reads as "nothing more to see" while the lines just asked for had never been shown. Seen in practice: `bgtail(lines: 60)` → 5,238 chars, then `bgtail(lines: 80)` → 49 chars of "(no new lines since last read — log at 238 lines)". A request to widen the view was answered as a poll. - The bookmark gains `served`: the trailing content lines the caller has been shown. A later read is a widening request when `lines > served` and the log holds lines older than `served`. - A widening returns only `rawLines.slice(total - lines, total - served)` — the earlier lines that have never been in context, never a re-send of the ones that have. `details.widened` marks the response. - It is bounded by what exists: once `served === total` there is nothing older to hand over, so the cheap poll response stands and polling a small log with the default 40 lines stays a ~49-char reply. - Narrowing is unchanged, and appended lines still win over a widening (the delta is what a caller most needs); `served` advances with contiguous coverage. - The raw / reset / shrink / replaced / changed-window paths reset `served`, since each re-establishes a full tail. Verified: `tsc --noEmit` clean; 225 tests pass (224 existing plus a new boundary test covering widening, narrowing, full coverage, and the appended-lines precedence); and a live pi session through the real tool interface, where `lines: 3` then `lines: 6` on a 12-line log returned exactly the three earlier lines with the widening header. The tool description lives in the changed file and is updated with the behavior; README and skill prose follow separately.
…bg skill The tool now hands back only the earlier lines it has not shown when a repeat read asks for more lines than the last one, but both surfaces still described repeat reads as strictly "only lines appended since your last read". - README: the `bgtail` row in the tools table, and the "Quick peek" bullet in "Reading results without flooding context". - skills/run-bg: the read-tools row and the quick-peek step. Both state the cost property explicitly, since that is the point of opening the window this way: widening costs the difference rather than re-sending the whole window, and it still never returns lines already in context.
Contributor
CI report
Ref: |
…ink path Review found the widening claim stronger than the behaviour: "never the ones already in your context" is false whenever coverage has a gap, because `served` tracks only the contiguous trailing run. A delta read showing fewer lines than were appended (`fresh.length > lines`) sets `served = lines`, leaving the fresh lines above the requested window as an unseen gap between the old run and the new one; a reset read with a narrower `lines` shrinks it too. A later widening then slices from the shrunken value and re-sends a line the caller already has. Reproduction: 13-line log → read `lines: 5` (shows 6-10) → append 11-13 → read `lines: 2` (shows 12-13, so line-11 was never shown) → read `lines: 4` returns line-10 again, plus line-11. - The claim is now accurate in all five places that made it — the tool description, the README tools table, the README quick-peek bullet, the skill's Tail row, and its quick-peek step. The trigger is "more lines than you have already been shown" (matching the `lines > served` guard, rather than the misleading "than last time"), and the return is "the lines above your current coverage", with the possible repeat stated. No behaviour changed here; only what the docs promise. - A test pins the case the invariant must survive: after that shrinking read, a widening still returns line-11, the line nothing has shown. It deliberately does NOT assert the repeat, so the true-dedup fix tracked in #28 can land without breaking it. Verified: `tsc --noEmit` clean; 226 tests pass.
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.
What
bgtail's delta bookmark recorded the log path and the search window, but not how many lines the caller had already been shown. A repeat read asking for a wider window therefore fell into the appended-lines path —freshwas empty — and the tool answered:That replies to the wrong question. The caller had just asked to see more of the tail, and the answer reads as "nothing more to see", while the lines it asked for had never been shown.
Seen in practice against the repo's own suite:
bgtail(lines: 60)→ 5,238 chars, thenbgtail(lines: 80)→ 49 chars of(no new lines since last read — log at 238 lines).Change
served— the trailing content lines the caller has been shown. A later read is a widening whenlines > servedand the log holds lines older thanserved.rawLines.slice(total - lines, total - served): the earlier lines, and never the whole window again.details.widenedmarks the response.served === totalthere is nothing older to hand over, so the cheap poll response stands.servedadvances with contiguous coverage.served, since each re-establishes a full tail.Verification
tsc --noEmitclean.lines: 3thenlines: 6on a 12-line log returned exactly the three earlier lines with the widening header.Follow-up from review (third commit)
Verdict: incorrect, non-blocking. The widening works and no path can hide an appended line, but the claim "never the ones already in your context" is false when coverage has a gap —
servedtracks only the contiguous trailing run, so a delta read showing fewer lines than were appended (fresh.length > lines) setsserved = linesand forgets a larger earlier coverage; a reset read with a narrowerlinesshrinks it too.Reproduction (13-line log):
lines: 5→ shows 6–10; append 11–13;lines: 2→ shows 12–13 (line-11 never shown);lines: 4→ returns line-10 again, plus line-11.Landed:
lines > servedguard rather than the misleading "than last time"), and the return is "the lines above your current coverage", with the possible repeat stated;servedcannot express a gapped coverage) is filed as bgtail: a widening can repeat a line the caller already saw when coverage has a gap #28, with the reproduction and three options.