You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Collect a routine report's files by the turn that sent them - #5179
Main already brings the files a routine sent with attach_file into its report, without the images its tools returned: that is the first version of #4771, merged through #4827. Main finds the files by walking from the reply to the person's lines around it. So a report could also carry the files of an earlier turn of the run, or of a goal that ran in between, and lose the files sent before a goal queued while the turn worked. It also missed two cases: after a restart, a turn that only sent files was read from the earlier turn's reply; and a run resumed after a failure whose turn said and sent nothing re-posted the earlier reply instead of saying it recovered. This change replaces the walking with turn ownership. It is the follow-up to #4771, moved here so it can be reviewed on its own.
A line posted by attach_file records the provider turn that sent it, in a new optional field sentInTurn. It is kept apart from turnId, so the last line with a turnId is still the turn's reply. The field lives on the server's message type, not on WireMessage, because only the server reads it.
A report carries the files of the reported turn only, in order: the lines whose turnId or sentInTurn is that turn, without images marked fromTool. The person's lines, goals and other turns in the run conversation don't affect it. A reply that names no turn carries only its own files.
A report held while the chat is busy remembers that its turn said and sent nothing, so a run resumed after a failure posts the recovered line.
After a restart, the last turn is found from the lines it settled on or the files it sent, so a turn that only sent files is reported with them.
Known gaps:
A report carries only its last turn's files. Files sent in a turn before the run paused for a hand-off are not carried.
Runs that deliver to the brief (deliver: "brief") are unchanged. The brief reads a run's answer as text only and has nowhere to show files.
An automatic voice reply that is synthesized after the run's turn ends lands on the run's reply after the report was already posted, as before.
If a report is still waiting for a busy chat when the app restarts, and its turn said and sent nothing after an earlier failure, the earlier reply is posted again.
File lines from before this change name no turn and are not collected.
Summary by CodeRabbit
Bug Fixes
Attachments are now associated with the correct run and reply, including after a restart.
Run reports exclude files from earlier turns and images returned by tools.
Quiet recovery and resumed runs remain quiet and do not include attachments from earlier turns.
File-only replies are included in reports, while files sent around steered or goal messages are attributed to the appropriate turn.
Attachment messages are linked to the relevant bot reply without being treated as replies themselves.
No actionable comments were generated in the recent review.
Walkthrough
Attached-file messages now record the turn in which they were sent. Automation reports use turn identifiers to select replies and collect attachments. Tests cover recovery, quiet runs, and reports without reply text.
Changes: Turn-owned attachment reporting
Layer / File(s)
Summary
Record turn ownership for attached files server/store.ts, server/bot-attachment.ts, server/index.ts, server/automation-reports.e2e.test.ts
Message supports sentInTurn. Attachment publication sets it to the current live turn ID and includes the bot identity for room sources. The end-to-end test checks that the attachment references the terminal reply’s turn and has no turnId.
Select replies and attachments by turn server/automation-reports.ts, server/automation-reports.test.ts
Report selection uses turnId and sentInTurn to identify replies and collect attachments. Tests cover ordering, restart recovery, quiet runs, and runs without reply text.
No merge-blocking risk is established. Reports remain limited to the last turn’s files.
Pre-merge checks: 4 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title clearly and concisely describes the main change: collecting routine report files by the turn that sent them.
Description check
The description is detailed and on topic. It explains the problem, implementation, expected behavior, and known gaps. It does not include explicit template headings for verification, screenshots, or t…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Description check
Explanation
The description is detailed and on topic. It explains the problem, implementation, expected behavior, and known gaps. It does not include explicit template headings for verification, screenshots, or the checklist, but these omissions do not make the description largely incomplete.
Full details: Docstring Coverage
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 5 files. (1 skipped: 1 too large.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 1bd611db-f604-4a01-9778-f048eb0acd5c
Commits
Reviewing files that changed from the base of the PR and between 11a5b46 and 6117673.
Files selected for processing: 3
server/automation-reports.e2e.test.ts
server/index.ts
server/store.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
A routine's report took the files of every line between the person's last line and the next, so it also carried the
files of an earlier turn of the run, or of a goal that ran in the same stretch, and lost the files sent before a goal
queued while its turn worked. A line attach_file posts now records the turn that sent it (sentInTurn), and the report
carries the files of the reply's turn only: its attach_file lines, then the reply's own generated images and voice
notes, without the images its tools returned. A reply that names no turn carries only its own files.
A report held while its chat was busy kept no record of a turn that said and sent nothing, so a run resumed after a
failure re-posted the earlier turn's reply instead of saying it recovered. After a restart, a report whose last turn
only sent files was read from the earlier turn's reply. A held report now remembers that its turn said nothing, and
after a restart the last turn is found by the lines it settled on or the files it sent.
@milind-soni could you take a look at this when you have a moment? It's the follow-up to #4771, whose first version went in through #4827. It was rebased on main today and merges cleanly, and the change stays within the routine report path (6 files). main moves fast and this has already conflicted once, so it would help to get it into a train soon. Happy to adjust anything.
This branch has not been deployed
No deployments
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
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.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Main already brings the files a routine sent with
attach_fileinto its report, without the images its tools returned: that is the first version of #4771, merged through #4827. Main finds the files by walking from the reply to the person's lines around it. So a report could also carry the files of an earlier turn of the run, or of a goal that ran in between, and lose the files sent before a goal queued while the turn worked. It also missed two cases: after a restart, a turn that only sent files was read from the earlier turn's reply; and a run resumed after a failure whose turn said and sent nothing re-posted the earlier reply instead of saying it recovered. This change replaces the walking with turn ownership. It is the follow-up to #4771, moved here so it can be reviewed on its own.attach_filerecords the provider turn that sent it, in a new optional fieldsentInTurn. It is kept apart fromturnId, so the last line with aturnIdis still the turn's reply. The field lives on the server's message type, not onWireMessage, because only the server reads it.turnIdorsentInTurnis that turn, without images markedfromTool. The person's lines, goals and other turns in the run conversation don't affect it. A reply that names no turn carries only its own files.Known gaps:
deliver: "brief") are unchanged. The brief reads a run's answer as text only and has nowhere to show files.Summary by CodeRabbit