Skip to content

Watcher: don't consume half-written transcript lines - #291

Merged
alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/ai-1243-watcher-partial-line-drop
Jul 7, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/ai-1243-watcher-partial-line-drop

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Problem

The live watcher's 1-second drain (WatchCommand.DrainNewLines) reads the transcript to EOF with StreamReader.ReadLineAsync, which returns the final line even when it is not yet newline-terminated — i.e. the agent is mid-write of it. The watcher sends that truncated prefix and advances state.LinesProcessed past it, so when the writer completes the line and appends more, the completed line is never re-read. Its truncated JSON fails to normalize server-side, so the event is permanently lost (and the gap sits below the high-water mark, where a resend can't back-fill it).

Large Read tool_result lines (long JSON, slow to flush) are the common victim — they land as orphaned tool calls that render as an eternal spinner in the Capacitor chat view.

Diagnosed from a real hosted-subagent session: two Read results present in the source .jsonl were absent from the ingested stream, with matching $lineNumber gaps at exactly those tool_result lines.

Fix

SplitNewCompleteLines(fileText, linesProcessed, holdIncompleteFinalLine = true):

  • Emits only newline-terminated lines.
  • Holds a partial final line back — excluded from the batch and the watermark — so the next drain re-reads it once complete.
  • The final drain at session end passes holdIncompleteFinalLine: false (the file is static then), so a vendor that leaves its last line unterminated is still delivered.

Blank-line skipping and line-number semantics are unchanged.

Tests

SplitNewCompleteLinesTests — 10 cases incl. the exact bug shape (tool_use complete + tool_result mid-write → held back, then delivered once the line completes). Full CLI unit suite green (2481); AOT publish clean (no IL2026/IL3050).

Linear: AI-1243. Paired with the server-side chat-rendering guard (kurrent-io/kcap-server) that stops already-orphaned calls from spinning.

🤖 Generated with Claude Code

The live watcher drain (DrainNewLines) read the transcript with ReadLineAsync,
which returns a final line that is not yet newline-terminated — i.e. the agent
is mid-write of it. It sent the truncated prefix and advanced its watermark past
it, so the completed line was never re-read (its truncated JSON fails to
normalize server-side, and the gap can't be back-filled below the high-water
mark). Large Read tool_result lines were the common victim, surfacing as
orphaned tool calls that spin forever in chat (AI-1243).

Add SplitNewCompleteLines, which emits only newline-terminated lines and holds a
partial final line back — excluded from the batch AND the watermark — until a
later drain sees it complete. The final drain at session end opts out
(holdIncompleteFinalLine: false) so a genuinely unterminated last line is still
delivered.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jul 7, 2026

Copy link
Copy Markdown

AI-1243

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Watcher: avoid consuming non-newline-terminated transcript lines

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Prevent live watcher drains from emitting half-written transcript lines.
• Hold back incomplete final line without advancing the line-number watermark.
• Add unit coverage for partial-line, blank-line, and final-drain behaviors.
Diagram

graph TD
  T[("Transcript .jsonl")] --> D["DrainNewLines"] --> S["SplitNewCompleteLines"] --> R["SecretRedactor"] --> H["SignalR Hub (SendTranscriptBatch2)"]
  W["WatchState watermark (LinesProcessed)"] --> D
  D --> W
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Incremental tail reader (byte offset + partial-line buffer)
  • ➕ Avoids File.ReadAllTextAsync on every 1s poll; scales better for large transcripts.
  • ➕ Naturally handles partial final lines by buffering trailing bytes until newline.
  • ➖ More complex state management (byte offset, encoding, CRLF handling).
  • ➖ Harder to keep line-number semantics aligned with server expectations.
2. Keep StreamReader but gate EOF line on trailing newline check
  • ➕ Minimal change from original streaming approach.
  • ➕ Less memory overhead than reading the entire file content.
  • ➖ Requires extra file probing (e.g., read last byte / track newline positions) and careful coordination with line counting.
  • ➖ Easier to get edge cases wrong (CRLF, empty file, concurrent writes).

Recommendation: The PR’s approach (compute complete lines and hold back the incomplete final line while keeping the watermark before it) is the safest correctness fix with clear tests. If transcript sizes make full-file reads too expensive, consider the incremental tail-reader alternative later.

Files changed (2) +187 / -30

Bug fix (1) +72 / -30
WatchCommand.csHold back incomplete final transcript line during live drains +72/-30

Hold back incomplete final transcript line during live drains

• Adds an isFinalDrain option to DrainNewLines and switches transcript reading to SplitNewCompleteLines, which emits only newline-terminated lines and prevents advancing LinesProcessed past a still-being-written final line. Introduces NewTranscriptLines and SplitNewCompleteLines to preserve existing blank-line/line-number semantics while avoiding dropped tool_result events.

src/Capacitor.Cli/Commands/WatchCommand.cs

Tests (1) +115 / -0
SplitNewCompleteLinesTests.csUnit tests for newline-terminated draining and watermark behavior +115/-0

Unit tests for newline-terminated draining and watermark behavior

• Adds 10 unit tests covering newline-terminated files, partial final line holdback, AI-1243 reproduction (tool_use followed by mid-write tool_result), blank-line counting, CRLF handling, and final-drain flush semantics.

test/Capacitor.Cli.Tests.Unit/SplitNewCompleteLinesTests.cs

@qodo-code-review

qodo-code-review Bot commented Jul 7, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Transcript read blocks writer ✓ Resolved 🐞 Bug ☼ Reliability
Description
WatchCommand.DrainNewLines now uses File.ReadAllTextAsync(transcriptPath), which doesn’t allow
specifying FileShare.ReadWrite like other transcript-reading code paths do. This can create
sharing/locking conflicts with a concurrently-writing agent (especially if the writer reopens the
file), causing intermittent drain failures or stalled transcript writes.
Code

src/Capacitor.Cli/Commands/WatchCommand.cs[R781-782]

+            var fileText  = await File.ReadAllTextAsync(transcriptPath, ct);
+            var drainRead = SplitNewCompleteLines(fileText, state.LinesProcessed, holdIncompleteFinalLine: !isFinalDrain);
Evidence
The new drain implementation reads the transcript using File.ReadAllTextAsync, while multiple
other transcript-reading paths in the same codebase explicitly use FileShare.ReadWrite to avoid
interfering with concurrent writers, indicating that shared read/write access is required for
transcript files.

src/Capacitor.Cli/Commands/WatchCommand.cs[761-786]
src/Capacitor.Cli/Commands/WatchCommand.cs[831-856]
src/Capacitor.Cli/Commands/WatchCommand.cs[1713-1721]
src/Capacitor.Cli/WatcherManager.cs[341-346]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DrainNewLines` switched from an explicitly shared `FileStream(..., FileShare.ReadWrite)` to `File.ReadAllTextAsync(...)`, which provides no way to retain the `FileShare.ReadWrite` behavior expected for live, concurrently-written transcript files.

## Issue Context
Other parts of the CLI intentionally open transcript files with `FileShare.ReadWrite` to coexist with a writer. The watcher’s live drain should preserve that behavior to avoid locking conflicts.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[761-787]

## Suggested change
Replace `File.ReadAllTextAsync(transcriptPath, ct)` with an explicit `FileStream` opened with `FileShare.ReadWrite` and read the contents via `StreamReader` (e.g., `ReadToEndAsync(ct)`), then pass the resulting text to `SplitNewCompleteLines`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Full transcript read each tick ✓ Resolved 🐞 Bug ➹ Performance
Description
DrainNewLines now materializes the entire transcript into a single string each drain, even though
the watcher drains once per second. For long-running sessions and large transcripts, this increases
allocation/GC pressure and makes drain cost scale with total transcript size rather than appended
lines.
Code

src/Capacitor.Cli/Commands/WatchCommand.cs[R781-786]

+            var fileText  = await File.ReadAllTextAsync(transcriptPath, ct);
+            var drainRead = SplitNewCompleteLines(fileText, state.LinesProcessed, holdIncompleteFinalLine: !isFinalDrain);

-                lineIndex++;
-            }
-
-            var linesRead = lineIndex;
+            var newLineNumbers = drainRead.LineNumbers;
+            var newLines       = drainRead.Lines.Select(SecretRedactor.RedactLine).ToList();
+            var linesRead      = drainRead.NextPosition;
Evidence
The watch loop runs DrainNewLines once per second, and the new drain implementation reads the entire
file into a string each time; this makes the per-tick cost depend on total file size.

src/Capacitor.Cli/Commands/WatchCommand.cs[295-350]
src/Capacitor.Cli/Commands/WatchCommand.cs[761-786]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The live watcher drains every second, but the new implementation reads the entire transcript into memory (`ReadAllTextAsync`) each time. This can become expensive for long sessions / large transcripts.

## Issue Context
The watch loop calls `DrainNewLines` repeatedly with a 1-second delay; the new drain path does a whole-file read each time.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[295-350]
- src/Capacitor.Cli/Commands/WatchCommand.cs[761-787]

## Suggested change
Keep the partial-line protection while avoiding whole-file reads:
- Open a `FileStream` with `FileShare.ReadWrite`.
- Determine whether the file ends with `\n` by checking the last byte/char (seek to end-1 when length>0).
- Stream line-by-line (like the previous approach) and, if the file does not end with `\n`, do not emit/advance past the final line.
Alternatively, track a byte offset watermark (instead of line index) so each drain reads only newly appended bytes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Commands/WatchCommand.cs Outdated
Comment thread src/Capacitor.Cli/Commands/WatchCommand.cs Outdated
… read (Qodo #291) [AI-1243]

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant