Repository navigation
Close an import batch on bytes and report what it drops - #817
Merged
Merged
Conversation
A stderr line written under the Spectre live region is erased by its next frame, so the warnings ride the progress channel and carry their session id for the routed sources' shared sink. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Summary by QodoBound transcript batches by bytes and surface dropped content
AI Description
Diagram
High-Level Assessment
Files changed (18)
|
Code Review by Qodo
1. Cursor imports skip blocked batches
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+35
to
+36
| public sealed record LineSkipped(string SessionId, string? AgentId, int LineNumber, int Bytes) | ||
| : ImportWarning(SessionId, AgentId) { |
There was a problem hiding this comment.
1. Warning types have ambiguous ownership 📘 Rule violation ⚙ Maintainability
ImportProgress.cs adds the public ImportWarning, LineSkipped, and BatchDropped top-level records alongside its existing primary type instead of placing each public type in a matching file. Because the closely related hierarchy exception requires implementations to be private or internal, later maintainers must treat one file as the owner of several independently visible types.
Agent Prompt
## Issue description
`ImportProgress.cs` now contains multiple additional public top-level warning types, so their names do not match the file that owns them.
## Issue Context
The closely related hierarchy exception only permits implementations that are private or internal. Preserve the public API by moving each public record into a file whose name matches the type.
## Fix Focus Areas
- src/Capacitor.Cli/Commands/ImportProgress.cs[28-43]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+557
to
+560
| if (bytes > TranscriptBatchBuffer.MaxBytes) { | ||
| progress?.Report(new LineSkipped(sessionId, agentId, lineNumber, bytes)); | ||
| } else { | ||
| if (!batch.Fits(bytes)) await FlushAsync(); |
There was a problem hiding this comment.
3. Quarantined cursor imports continue 🐞 Bug ≡ Correctness
SendTranscriptBatches reports an oversized line without invoking abortDelivery, and it returns without calling FlushAsync when the remaining transcript has no sendable lines. If Cursor’s quarantine marker appears after the final successful batch while the tail contains only blank or oversized lines, Cursor proceeds to child delivery and session-end instead of taking its close-and-fail path.
Agent Prompt
## Issue description
`SendTranscriptBatches` only evaluates `abortDelivery` inside `FlushAsync`. An over-budget line is now skipped without flushing, so a Cursor transcript whose remaining lines are blank or oversized can return normally after quarantine is marked.
## Issue Context
Cursor catches `TranscriptDeliveryAbortedException` to stop subsequent child delivery and close the session through its quarantine-specific failure path. Preserve that behavior even when no additional transcript batch is posted.
## Fix Focus Areas
- src/Capacitor.Cli/Commands/SessionImporter.cs[553-581]
- src/Capacitor.Cli/Harness/Cursor/CursorImportSource.cs[575-635]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+557
to
+559
| if (bytes > TranscriptBatchBuffer.MaxBytes) { | ||
| progress?.Report(new LineSkipped(sessionId, agentId, lineNumber, bytes)); | ||
| } else { |
There was a problem hiding this comment.
4. Valid large lines are discarded 🔗 Cross-repo conflict ≡ Correctness
SendTranscriptBatches unconditionally emits LineSkipped for any line over 4 MiB, even when that line would fit by itself within kcap-server's request-body limit. This triggers for records such as a 5 MiB mostly-ASCII JSON line, although the server's transcript contract has no per-line limit and processes each supplied string independently.
Agent Prompt
## Issue description
Do not discard a transcript line merely because it exceeds the normal 4 MiB batch budget; kcap-server can accept larger individual lines when the complete request remains within its body limit.
## Issue Context
The 4 MiB threshold is appropriate for closing multi-line batches, but it is not a server-defined per-line limit. Flush the current batch and attempt an oversized line as a single-line batch, reporting it as dropped only if the server refuses that request.
## Fix Focus Areas
- src/Capacitor.Cli/Commands/SessionImporter.cs[553-565]
- src/Capacitor.Cli/Commands/TranscriptBatchBuffer.cs[34-43]
- test/Capacitor.Cli.Tests.Unit/Commands/SessionImporterProgressTests.cs[198-230]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+632
to
+635
| if (loss is not null) { | ||
| if (failOnError) throw new HttpRequestException($"transcript batch lines {first}-{last} rejected: {loss}"); | ||
|
|
||
| progress?.Report(new BatchDropped(sessionId, agentId, first, last, loss)); |
There was a problem hiding this comment.
5. Cursor imports skip blocked batches 🔗 Cross-repo conflict ≡ Correctness
FlushBatchAsync treats Cursor's HTTP 409 as a lenient BatchDropped, clears the buffer, and lets SendTranscriptBatches post later ranges. When Cursor normalization or persistence blocks within a batch, kcap-server defines that response as a retry-from-gap signal, so later requests can advance the watermark beyond the refused range.
Agent Prompt
## Issue description
Honor kcap-server's Cursor frontier protocol instead of continuing after an HTTP 409 blocked response.
## Issue Context
The server holds Cursor's next line at the failed record and expects the source to retry from that gap. Make the top-level Cursor batch call strict, or otherwise stop delivery specifically on 409, so a later import resumes from the server watermark rather than posting later ranges.
## Fix Focus Areas
- src/Capacitor.Cli/Harness/Cursor/CursorImportSource.cs[568-590]
- src/Capacitor.Cli/Commands/SessionImporter.cs[624-640]
- test/Capacitor.Cli.Tests.Unit/Commands/SessionImporterProgressTests.cs[232-274]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 10, 2026
This was referenced Sep 18, 2026
This was referenced Sep 25, 2026
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.
Closes #814 — AI-2586
What & why
kcap importflushed a transcript batch every 100 lines whatever their size, and read the response status only for a strict caller, so a batch the server refused was counted as sent. A batch now also closes at 4 MiB of line content, a line over that budget is skipped rather than posted, and a lenient path reports a refused batch instead of swallowing it. Both reports travel on the import progress channel: the vendor sources get a per-run sink on the import context, so their warnings render above the live progress region instead of being erased by its next frame.Where to look
TranscriptBatchBuffercarries the two limits and why 4 MiB. A dropped batch's lines still count toward the "lines sent" figure; the warning line above it is the record. Status retries stay off for the batch POST: a strict batch's normalization failure comes back as a 500, and retrying it would stall every strict import for the whole retry budget.Verification
SessionImporterProgressTests: three 1.5 MiB lines post as 2 + 1; a line over budget is skipped withline_numbers [0,2]on the wire; a 413 yieldsBatchDroppedfor a lenient caller and a range-naming exception for a strict one.*Import*classes: 41 passed. AOT publish: 0 IL warnings.