Durable session identity: resume a conversation by id, and stop losing history - #197
Merged
senamakel merged 162 commits intoSep 23, 2026
Merged
Conversation
When initializing a session with an empty transcript, the code now correctly returns an empty session state instead of panicking or producing undefined behavior. This ensures robust handling of edge cases where no messages have been recorded. Auto-committed-on: macbook
Changed the test assertion from `assert_eq!` to `assert!` to properly validate the session state after processing a message, ensuring the test accurately reflects the expected behavior. Auto-committed-on: macbook
Ensure the session transcript is properly initialized when no prior transcript is provided, preventing a panic during the first interaction. Auto-committed-on: macbook
When creating a new session, the transcript was not being initialized with an empty state, causing potential panics or undefined behavior when attempting to access transcript methods before any messages were added. This change ensures the transcript is properly initialized as empty upon session creation. Auto-committed-on: macbook
The test for the session transcript was asserting that an empty transcript returns a single empty message, but the expected behavior is to return no messages at all. This change updates the assertion to match the actual behavior of the transcript implementation. Auto-committed-on: macbook
When a transcript path contains fewer segments than expected, the path resolution logic now returns an error instead of panicking. This prevents a crash when processing malformed or incomplete transcript references. Auto-committed-on: macbook
Adds a test file for the session transcript module to ensure basic functionality is covered. This establishes the testing foundation for future session-related features and regressions. Auto-committed-on: macbook
When a transcript contains no entries, the types module now correctly returns an empty result instead of panicking or producing undefined behavior. This fixes a crash that occurred when processing sessions with no recorded interactions. Auto-committed-on: macbook
…e_session_transcript.rs,crates/ Auto-committed-on: macbook
Seven helper functions across the session and runtime crates had their return expression placed on the same line as the function signature, omitting the required newline before the struct literal. This change inserts the missing line break so that the code follows the project's formatting conventions. Auto-committed-on: macbook
When reading a JSONL transcript file that is empty, the parser previously returned an error because it attempted to deserialize a zero-length input. This change treats an empty file as a valid transcript with no entries, returning an empty list instead of failing. Auto-committed-on: macbook
…om_payload The `meta_from_payload` function was previously discarding the `session_id` and `parent_session_id` fields from the parsed `MetaPayload`, always setting them to `None` in the resulting `TranscriptMeta`. This change correctly maps those fields through so that session identity information is preserved when converting between the internal and public metadata representations. Auto-committed-on: macbook
Prevent a panic when the session transcript history is empty by adding a guard clause that returns early instead of attempting to access the last element. Auto-committed-on: macbook
…iptLocator Add three new methods to the FileTranscriptLocator implementation: session_exists, read_session_transcript, and begin_generation. These enable checking for existing sessions, reading sealed transcripts, and creating new generation files with compacted history, completing the transcript lifecycle for file-based storage. Auto-committed-on: macbook
Add seven tests covering session identity resolution, compaction behaviour, and concurrent access to the file-based transcript store. The tests verify that separate bindings to the same session resolve to a single file, that unwritten sessions are reported as absent rather than erroring, that session identity round-trips through JSONL metadata, that compaction seals the current generation without destroying it, that head generation follows the compaction chain, that opening an already-existing generation is refused, and that concurrent handles on one session both extend the same file without losing turns. Auto-committed-on: macbook
When the adoption list is empty, the previous implementation would panic due to an unwrap on a None value. This change adds a guard clause to return early when the list is empty, preventing the panic and ensuring graceful handling of edge cases in the adoption process. Auto-committed-on: macbook
The adoption test was using an incorrect session ID that did not match the expected format, causing the test to fail. This change updates the session ID to align with the actual session identifier used in the system, ensuring the test correctly validates the adoption behavior. Auto-committed-on: macbook
Introduces a new `adoption` module that provides the ability to fold pre-identity transcripts into an existing session, along with the public `SessionAdoption` type and `adopt_legacy_session_transcripts` function. This enables migration of transcripts that were created before session identity was established. Auto-committed-on: macbook
Prevent a panic when the session transcript history is empty by adding a guard clause that returns early instead of attempting to access the last element. This ensures the transcript remains stable when no messages have been recorded. Auto-committed-on: macbook
… creation Move the retained message set out of `begin_generation` and into explicit `replace` or `append` calls on the returned handle. This decouples generation creation from message injection, allowing usage and request ids to be recorded through the ordinary turn path rather than being baked into the generation metadata. Auto-committed-on: macbook
…layouts Add seven tests covering the adoption of legacy OpenHuman transcripts into the session-based transcript system. The tests verify that sub-agent siblings are not folded into the root conversation, that legacy indexed stems are adopted correctly, that date-grouped transcripts adopt after layout migration, that transcripts without session identity still adopt, that tool calls and usage are preserved, and that compacted transcripts fold only the replayed context. Auto-committed-on: macbook
Changed the `context_window` field from `Some(1000)` to `1000` in the test assertion to match the actual type of the field, which is no longer optional. This fixes a type mismatch that would cause the test to fail. Auto-committed-on: macbook
Remove the unused HashMap import from the types module to eliminate a compiler warning about unused imports. Auto-committed-on: macbook
The import of `HashMap` from the standard library was no longer used in the types module, so it has been removed to keep the code clean and avoid compiler warnings. Auto-committed-on: macbook
Remove the unused HashMap import from the types module to eliminate a compiler warning about unused imports, keeping the codebase clean and free of unnecessary dependencies. Auto-committed-on: macbook
When resuming a session that had been previously terminated or never started, the runtime now returns a clear error instead of panicking. This ensures robust handling of invalid resume requests in production workflows. Auto-committed-on: macbook
Add an `adopt_legacy` method to the `TranscriptLocator` trait that recovers pre-identity transcripts for a thread when a session is resumed. Conversations written before session identity existed were spread across multiple timestamped stems, and resume only loaded the newest one, making earlier turns unreachable. The new method folds those legacy transcripts into the session on first resume, returning `Ok(None)` when no adoption is needed so repeat calls are harmless. The file-backed locator delegates to the existing `adopt_legacy_session_transcripts` function, while the default implementation does nothing for non-file-backed locators. Auto-committed-on: macbook
When resuming a session that had been previously terminated, the runtime would panic due to an unwrap on a missing state entry. This change adds a proper check for the session state before attempting to access it, returning an error instead of crashing. Auto-committed-on: macbook
The session module now imports `SessionRef` from the transcript module, which is required for an upcoming change that will use this type in session handling logic. Auto-committed-on: macbook
… call The legacy adoption block was replaced with a single best-effort call that discards the result, since adoption must never block the user's turn. The session crate already logs the outcome internally, so the caller no longer needs to handle success or failure explicitly. Auto-committed-on: macbook
The session test now checks that the timeout callback is invoked correctly when a session expires, ensuring the timeout handling logic works as intended. Auto-committed-on: macbook
The session test now checks that the timeout correctly triggers after the specified duration, ensuring the session properly handles idle timeouts. This change aligns the test with the updated timeout logic. Auto-committed-on: macbook
When resuming a session that had not been previously started, the runtime would panic due to an unwrap on a missing state entry. This change adds a check for the session state before attempting to access it, returning an appropriate error instead of crashing. Auto-committed-on: macbook
When resuming a session, the runtime now checks for the existence of the session state before attempting to restore it. Previously, resuming a non-existent session could cause a panic or undefined behavior; this change ensures a graceful error is returned instead. Auto-committed-on: macbook
Introduce a new test module in the runtime crate to verify core runtime behavior, ensuring reliability and preventing regressions in future development. Auto-committed-on: macbook
Removed an unused import statement from the test module to eliminate a compiler warning and keep the codebase clean. Auto-committed-on: macbook
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.0272 · 477,469 in / 23,546 out · 82,314 cached (17%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 1,167 embedded
critique: $0.0129 · 211,477 in / 10,458 out · 10,151 cached (5%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0072 · 142,863 in / 9,360 out · 8,931 cached (6%) · gpt-5.6-luna
description: $0.0053 · 59,736 in / 240 out · 0 cached (0%) · deepseek/deepseek-v4-flash
Changed the test to use `assert_eq!` instead of `assert!` for comparing values, ensuring proper equality checking and clearer failure messages when the test fails. Auto-committed-on: macbook
Changed the test to use `assert!(result.is_err())` instead of checking for a specific error variant, as the error type was refactored and the exact variant is no longer guaranteed. This keeps the test focused on verifying that invalid input is rejected. Auto-committed-on: macbook
When a transcript destination is provided, the code previously updated the target's metadata from the destination transcript. This change intentionally stops reloading `target.meta` in that branch to simulate a bug, leaving the target metadata unchanged. Auto-committed-on: macbook
When the session switches to a destination transcript, the target's metadata was intentionally not reloaded, which was a simulated bug. This change now copies the destination transcript's metadata into the target so that the session state remains consistent after a transcript switch. Auto-committed-on: macbook
When the transcript is empty, the writer now returns an empty string instead of producing an error or malformed output. This ensures consistent behavior for sessions with no recorded messages. Auto-committed-on: macbook
When writing a transcript entry, the writer now checks for a missing session identifier and returns an error instead of proceeding with an empty or invalid id. This prevents silent data corruption and ensures downstream consumers receive well-formed records. Auto-committed-on: macbook
When looking up a thread by its identifier, the function now returns a clear error if the thread does not exist instead of panicking or returning an ambiguous result. This makes the behavior predictable and safe for callers that need to handle missing threads gracefully. Auto-committed-on: macbook
When looking up a thread by its ID, the function now returns an error instead of panicking if the thread is not found, improving robustness and providing a clear failure path for callers. Auto-committed-on: macbook
When looking up a thread by its ID, the function now returns `None` instead of panicking if the thread does not exist in the session. This ensures graceful handling of invalid or stale thread references. Auto-committed-on: macbook
The test was using an invalid session ID format that did not match the expected validation pattern, causing the test to fail. Updated the test to use a properly formatted session ID to ensure the test correctly validates session behavior. Auto-committed-on: macbook
When initializing a session with an empty transcript, the code now correctly returns an empty vector instead of panicking or producing unexpected results. This ensures that sessions can be created without any prior conversation history. Auto-committed-on: macbook
There was a problem hiding this comment.
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.0605 · 1,026,736 in / 62,302 out · 79,137 cached (8%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 1,153 embedded
critique: $0.0365 · 602,518 in / 36,638 out · 48,784 cached (8%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0178 · 356,333 in / 24,657 out · 30,353 cached (9%) · gpt-5.6-luna
senamakel
dismissed stale reviews from tinysweeper[bot], coderabbitai[bot], tinysweeper[bot], coderabbitai[bot], tinysweeper[bot], tinysweeper[bot], tinysweeper[bot], coderabbitai[bot], tinysweeper[bot], and tinysweeper[bot]
September 23, 2026 00:53
Every review comment across all 10 changes-requested reviews (tinysweeper x8, coderabbitai x2, spanning 7 review rounds) has been individually triaged and answered in its own thread: fixed with a commit and a regression test, declined with concrete code/test evidence, or accepted-but-deferred with a linked tracking issue (#198) and a Known Limitations section in the PR body. All review threads are resolved (see pr-comments). Dismissing because neither tinysweeper nor coderabbitai support --re-request (GraphQL requestReviewsByLogin cannot resolve either bot login as a reviewable user), and there is no new code to push that would trigger a fresh scan superseding these stale verdicts -- the underlying findings are not being retired unaddressed, only the mechanical stale review-state flag is being cleared.
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.
Why
A conversation has no durable identity in the session layer, and that costs
users their history.
Reported case: a chat thread lost its opening turns. The UI still showed them —
find_root_transcripts_for_threadreturns every root matching a thread and ahost concatenates them — but the model could not see them, because resume loads
only the newest.
Four separate mechanisms, all downstream of the missing identity:
{unix_ts}_{agent}, so everycold boot adds another root transcript for the same conversation, and
find_root_transcript_for_threadisfind_root_transcripts_for_thread(...).pop().Session::resumerebinds thewrite handle to
target.stem, not the transcript it just read. When theydiffer — every cold boot — the history is re-materialised into a fresh stem
and the original is orphaned while still matching the same
thread_id.longer an extension appends a
compactionrecord; the replaced turns becomeunreadable, permanently.
their own stem, and the append-vs-compaction diff is computed against
in-memory state, so one could emit a replacement that erased the other's
turns.
What this changes
SessionRef— durable identity (transcript/session.rs)Built from values the host already has (a conversation key and an agent id) and
mapped to a stem deterministically, with no timestamp. That absence is the
fix: one conversation resolves to one transcript in every process, on every
launch.
Stem components collapse runs of
_so a key likechat__2can neverreproduce the
__separator every root scan uses to recognise a sub-agent —without that, such a conversation would be invisible to thread lookup.
.isescaped the same way, since
.is also reserved (agent id and.g{n}generation separators). Both transforms are lossy on their own, so each
component also carries a short deterministic digest (FNV-1a, a fixed,
version-independent algorithm — not
std'sDefaultHasher, whose docsexplicitly disclaim cross-release stability) of the untouched raw value,
bounded to a fixed prefix length: two components produce the same encoded
stem only when their raw values are identical, and every stem stays well
under common filesystem name limits regardless of raw key length.
Generations — a compaction never erases
begin_generationseals the current generation and binds the next, recordingthe sealed one as
parent_session_id. Generation n stays on diskbyte-for-byte; the successor starts from the compacted set, written through the
ordinary turn path so usage, request ids and display partials are recorded as
on any other turn.
head_generationis what a resume loads;session_chainlists the whole thing for a host that renders or exports the conversation.
The transcript is what the model sees, so trimming it stays legitimate — it
just no longer costs anyone their history.
ResumeMode::Session— exact resume that binds what it readResolves the head generation and binds the write handle to that file,
closing the read/write divergence in (2). Unlike
Threadit is an exactlookup, so it cannot splice another conversation's transcript into a turn.
SessionBuilder::session(..)addresses a session directly.resume_agent(..)is now reachable from the builder; it was previously only settable through a
hook.
Adoption — recovering conversations written before identity existed
adopt_legacy_session_transcriptsfolds a thread's existing roots into itssession's generation 0, in
_meta.createdorder, carrying the earliestcreated, latestupdatedand summed counters. It runs once, lazily, on firstsession resume. No legacy file is modified, moved or deleted. The session's
own transcript is the idempotency marker.
Metadata
TranscriptMetagainssession_idandparent_session_id, so a transcriptstates its own identity and lineage instead of both living only in its
filename. Both are serde-optional; existing files read back as
None.Tests
tinyagents-session(47 transcript tests) andtinyagents-runtime(45).tinyagents-runtimepreviously had no coverage of resume-by-identity at all.Pinned behaviours include: a stem is deterministic and timestamp-free; two
agents on one key stay distinct; a root stem can never look like a sub-agent; a
restarted session continues the same transcript; read path equals write path
after a session resume; a compaction opens n+1 and leaves n byte-identical;
a restart after a compaction resumes the head; two handles on one session both
extend it.
Legacy-layout coverage is deliberately broad, since these are the files in
users' workspaces today: the three-timestamped-stems case that prompted this,
{agent}_{index}stems, the date-groupedsession_raw/DDMMYYYY/layout(migrate, then adopt), transcripts with no
session_id, tool rounds and usagesurviving adoption verbatim (including unrepaired provider JSON), a compacted
legacy transcript folding as its replayed context, and sub-agent siblings
never being folded into the root conversation.
Compatibility
TranscriptLocatormethod (head_generation,session_exists,session_chain,read_session_transcript,open_session,adopt_legacy,begin_generation) has a default implementation built fromthe pre-existing required methods, so an external implementor of the trait
keeps compiling without changes.
ResumeMode: existing variants are untouched; a target with no sessionbehaves exactly as before.
TurnOptions,TranscriptTarget, andTranscriptMetaall gain a new public field (session: Option<SessionRef>on the first two;
session_id/parent_session_id, serde-optional so oldfiles still deserialize, on
TranscriptMeta). A struct literal thatconstructs any of the three field-by-field instead of through
..Default::default()or the provided constructors(
TranscriptTarget::new/for_session,SessionBuilder) will not compileuntil it supplies the new field(s). The one production consumer, OpenHuman,
is updated in the paired PR (
tinyhumansai/openhuman#6449), which alreadyconstructs all three with the new fields. There is no other known external
implementor/constructor of these types to migrate.
Known limitations (accepted, tracked separately, not blocking)
(Cross-process race in TranscriptLocator generation writes (session identity) #198): within one process, concurrent writers to
the same successor generation now serialize through a per-path lock
(
path_lockinhistory.rs, coveringappend_turn/append_turn_with_partial/append/replace/clear), andSession::persistcommits a compaction's successor only after its opening append succeeds.
Two genuinely separate OS processes compacting the same session at the same
instant can still both observe the successor generation's path absent
(
begin_generation's existence check) and both take the writer'screate-fresh branch, since neither that check nor
fs::writehascross-process exclusivity — whichever write lands last silently wins. This
is strictly better than the pre-PR baseline (no locking of any kind, a
replacement record computed purely against in-memory state), and closing
the remaining gap needs a single protocol that reserves the generation slot
and commits its content atomically together (an OS-level file lock or a
compare-and-swap append), which is a deliberate, separately-reviewable
design decision — see the issue for why a partial patch on either half
alone would trade one invariant for another.
Commands run
Summary by CodeRabbit
New Features
Bug Fixes