Skip to content

fix: recover transcripts after interrupted writes - #222

Merged
senamakel merged 4 commits into
tinyhumansai:mainfrom
senamakel:failed-turn-recovery
Sep 25, 2026
Merged

senamakel merged 4 commits into
tinyhumansai:mainfrom
senamakel:failed-turn-recovery

Conversation

@senamakel

@senamakel senamakel commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Summary

Recover a session after a process stops partway through a transcript write. A torn JSONL tail previously merged with the next append, making that later turn disappear from replay. A tail cut inside a multibyte character made the whole transcript unreadable.

  • Publish the first transcript file atomically so an incomplete _meta header is never visible.
  • Separate an unfinished tail before the next append, preserving its bytes for diagnosis.
  • Decode JSONL per record so readers skip only a damaged record and retain complete turns before and after it.

API Or Behavior Changes

No public API change. Model-context, display, metadata, and usage readers now tolerate a damaged non-header record, including invalid UTF-8. A damaged first header still returns an error.

Tests

  • cargo fmt --manifest-path vendor/tinyagents/Cargo.toml --package tinyagents-session --check (from OpenHuman worktree)
  • cargo test -p tinyagents-session --lib (163 passed)
  • Full workspace Clippy, build, and test matrix (CI)

The new torn-append and invalid-UTF-8 regressions failed before the fix and pass afterward.

Documentation

Updated the reader and writer contract comments in code; no external API documentation changed.

Summary by CodeRabbit

  • Bug Fixes
    • Resuming a transcript after an incomplete or invalid trailing record now preserves earlier messages and allows new turns to be added.
    • Transcript readers can continue past invalid records after the first valid entry, while still reporting errors or no results when the first entry is invalid.
    • Creating a transcript no longer overwrites a file created by another writer, and appends are separated from incomplete trailing records.

@tinysweeper

tinysweeper Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 2 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Changes requested
Priority: high
Reviewed head: b427135580af
Updated: 1790362750 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 2 Active findings 3
Tests 1 Noted findings 0
Documentation 0 Resolved findings 9
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

  • high · tests · Handle a competing first writer without failing the turn — When two concurrent sessions both create the same transcript file, the second writer now fails with an error instead of using the existing file. The prior behavior (overwriting) wa (crates/tinyagents\-session/src/transcript/writer\.rs:225)
  • high · description · Do not expect malformed JSON records to be skipped until the reader handles them — The test `a_torn_append_does_not_swallow_the_next_turn` writes a truncated JSON line (valid UTF-8 but invalid JSON) and expects `read_transcript` to succeed and skip it. However, t (\(pull request description\))
  • high · description · Handle a competing first writer without failing the turn — The current code uses `publish_transcript_if_absent` (likely backed by `create_new`) which returns `Ok(false)` if the file already exists, causing the `ensure!` to fail with an err (\(pull request description\))

Resolved this pass

  • Fix test that asserts success on file with invalid JSON line
  • Handle a competing first writer without failing the turn
  • Skip invalid JSON non-header records instead of returning an error
  • Fix test that asserts success on file with invalid JSON line
  • Handle a competing first writer without failing the turn
  • Skip invalid JSON non-header records instead of returning an error
  • Fix test that asserts success on file with invalid JSON line
  • Skip invalid JSON non-header records instead of returning an error
  • Fix test that asserts success on file with invalid JSON line

Before merge

  • Address Handle a competing first writer without failing the turn (crates/tinyagents\-session/src/transcript/writer\.rs).
  • Address Do not expect malformed JSON records to be skipped until the reader handles them (\(pull request description\)).
  • Address Handle a competing first writer without failing the turn (\(pull request description\)).

How this fits together

flowchart LR
  n0["read_transcript<br/>changed"]:::changed
  n1["read_transcript_jsonl<br/>changed"]:::changed
  n2["...cord_is_skipped_without_losing_valid_rows<br/>changed"]:::changed
  n3["new"]:::impacted
  n4["path"]:::impacted
  n5["meta"]:::impacted
  n6["tempdir"]:::impacted
  n7["codec"]:::impacted
  n8["...rget_does_not_corrupt_its_own_destination"]:::impacted
  n0 -->|calls| n1
  n2 -->|calls| n0
  n2 -->|tests| n0
  n2 -->|calls| n6
  n8 -->|calls| n0
  n8 -->|tests| n0
  n8 -->|calls| n3
  n8 -->|tests| n3
  n8 -->|calls| n4
  n8 -->|tests| n4
  n8 -->|calls| n5
  n8 -->|tests| n5
  n8 -->|calls| n6
  n8 -->|calls| n7
  n8 -->|tests| n7
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The reader now isolates invalid UTF-8 records while preserving the required-header invariant, and the added tests cover torn JSON and UTF-8 tails. The previously reported concerns are addressed; this change looks safe to merge. _The code index is behind this pull request (indexed at `c0ea84492e2a`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The transcript reader now isolates invalid UTF-8 records while preserving the required header invariant, and the added tests cover torn JSON and UTF-8 tails. The change looks safe to merge. _The code index is behind this pull request (indexed at `c0ea84492e2a`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

tests

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: Adds resilient JSONL reading (splits on newlines, decodes each line independently) and torn-write recovery (detects trailing partial lines and inserts a newline before appending). The initial-creation path prevents clobbering a concurrent writer's file, but if such a race occurs it fails the turn with an error instead of recovering gracefully — a regression from the previous overwrite behavior. _The code index is behind this pull request (indexed at `c0ea84492e2a`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._
  • Evidence: crates/tinyagents\-session/src/transcript/writer\.rs — Handle a competing first writer without failing the turn

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: The reader changes improve UTF-8 tolerance, but the writer still fails on concurrent creation and the new test assumes the reader skips invalid JSON lines, which it does not. These issues must be addressed before merging. (1 earlier finding(s) still open) _The code index is behind this pull request (indexed at `c0ea84492e2a`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._
  • Evidence: \(pull request description\) — Do not expect malformed JSON records to be skipped until the reader handles them
  • Evidence: \(pull request description\) — Handle a competing first writer without failing the turn

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: ladder/vectors, gpt-5.6-luna, deepseek-v4-flash
  • Spend: $0.005388
  • Tokens: 172683 input · 36912 output · 27758 cached · 634 embedding
Head State Pass summary
f4dfc027e073 changes requested 2 active finding(s), 0 resolved finding(s) (at 1790362024)
b427135580af changes requested 3 active finding(s), 9 resolved finding(s) (at 1790362750)

tinysweeper 0.1.0

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

  • Run on-demand review

This review includes 3 billable files and costs up to $0.75.

Or wait 44 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: fde0d0be-2f21-4d04-9659-cabcab25c4bf

📥 Commits

Reviewing files that changed from the base of the PR and between ec40220 and b427135.

📒 Files selected for processing (3)
  • crates/tinyagents-session/src/transcript/reader.rs
  • crates/tinyagents-session/src/transcript/test.rs
  • crates/tinyagents-session/src/transcript/writer.rs

Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: 1 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0053 · 187,310 in / 16,343 out · 12,510 cached (7%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 624 embedded
critique:    $0.0021 · 65,597 in  / 4,345 out  · 2,114 cached (3%)  · gpt-5.6-luna
security:    $0.0023 · 90,341 in  / 1,367 out  · 3,740 cached (4%)  · gpt-5.6-luna
tests:       $0.0004 · 17,552 in  / 4,644 out  · 4,608 cached (26%) · deepseek-v4-flash
description: $0.0003 · 8,892 in   / 3,531 out  · 2,048 cached (23%) · deepseek-v4-flash

Comment thread crates/tinyagents-session/src/transcript/test.rs
@tinysweeper tinysweeper Bot added the priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition. label Sep 25, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: 2 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0054 · 172,683 in / 36,912 out · 27,758 cached (16%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 634 embedded
critique:    $0.0014 · 44,374 in  / 3,287 out  · 4,228 cached (10%)  · gpt-5.6-luna
security:    $0.0021 · 67,087 in  / 5,135 out  · 5,610 cached (8%)   · gpt-5.6-luna
tests:       $0.0013 · 49,435 in  / 17,403 out · 16,896 cached (34%) · deepseek-v4-flash
description: $0.0004 · 6,529 in   / 7,157 out  · 1,024 cached (16%)  · deepseek-v4-flash

// stops mid-write. Such a file exists but cannot be resumed at all.
// Publish the complete first generation only after staging it, and
// never replace a file another writer created meanwhile.
anyhow::ensure!(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high tests confident

Handle a competing first writer without failing the turn

When two concurrent sessions both create the same transcript file, the second writer now fails with an error instead of using the existing file. The prior behavior (overwriting) was bad, but failing the turn is also unacceptable: the turn should detect the race and continue writing into the existing file. The test suite does not cover this scenario.

[RULE] race-condition-handling ·

@senamakel
senamakel merged commit 325ad44 into tinyhumansai:main Sep 25, 2026
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant