Repository navigation
sessions: an import writes its archive's search - #774
Conversation
The importer holds each conversation's bytes, so it writes the archive's search as it files them, through the platform's search write API (the search capability's contract): one source a conversation's file, its revision the file's sha256, one section a turn (fragment_core::session_search: "Person: … / Agent: …", tool output left out and calls named, a summary its own section, turns numbered from the start so a grown conversation keeps its earlier ids, within the capability's limits). After its writes, each import reconciles search with the archive: it pages `sources`, writes each conversation whose index sha256 is not the revision search holds (from the spool, else read from the archive), and deletes each conversation's source the archive no longer has. A host without search (404), or one unreachable, is said and skipped; the import stands. fragment_fakes::search stands in for the routes (the code.storage fake's host routes, on per fragment), checking every limit the contract sets. The request shapes are the CLI's own until the capability adds its wire types to fragment_proto (debt ledger). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ve-search # Conflicts: # docs/MODEL.md
The search capability (#783) is on master, with its wire types in fragment_proto::search and its checks in fragment_core::searchable. The archive's search writer now uses them, and its own copies are gone (the debt entry with them): - `session_search::upsert_of` builds a `fragment_proto::search::Upsert` within the platform's limits: sections and text as before; title, fields and context bounded in bytes (`searchable`); no control character but a line's end and a tab in a section, and none (nor a filter's `=`) in a name or a filter. - The import runs `searchable::check` on each upsert before it sends it (one the platform would refuse is said, unsent), deletes with `search::Delete`, and pages sources by `SOURCES_PAGE_MAX`. - `fragment_fakes::search` checks an upsert as the cell does (the same type and check), and answers as the cell does. - The e2e's search section imports the sessions section's export against the real service: each conversation kept becomes a source, its person finds one by its words, and the same import again writes nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alexlwn123
left a comment
There was a problem hiding this comment.
Fail for now at ea9996f2b521640ed1a6e6f092013d25d80ea502.
Blocking:
- [P2] An older import can undo a completed import's search reconciliation.
cli/src/session/search.rs:98-99computes writes and deletions from the entries saved bywrite(), rather than the archive as it stands when reconciliation runs. Reproduced with a temporary test using the existing code.storage/search fake: A commits conversationaand pauses beforesync; B updatesa, addsb, and completes its search sync; A resumes. A reportswritten: 1, deleted: 1, refused_count: 0, restoresa's old search revision from its spool, and deletessessions/hermes-b.jsonfrom search although main and its index still hold both conversations. Both imports succeed, but the newer words and the new conversation stop being searchable until another import repairs them.
This is additional to the debt ledger's accepted same-conversation file/index race: even imports of different conversations can remove each other's search sources. Reconciliation must account for archive changes after its saved snapshot, for both upserts and deletions; absence from that snapshot is not evidence a file is gone. Please cover overlapping imports that add distinct conversations and update an existing conversation, including a writer overtaken during reconciliation. The archive-read fallback also reads mutable MAIN while assigning the saved entry's hash (write_one, lines 174-177), so preserve the bytes/revision relationship across this boundary too.
The extractor and use of the platform's shared wire types/checks otherwise fit the purpose without unnecessary platform machinery.
Validation: all four fragment-core session-search tests and the existing CLI reconciliation test pass; the temporary overlap reproduction confirmed the behavior above and was removed afterward. CI check and images check passed; e2e shards and the real-Hermes Docker check were still running when inspected. I did not run full e2e locally. The engineering-style document named by AGENTS.md was unavailable at its specified path.
… then Alex's review of #774: the reconciliation took its writes and deletes from the entries `write()` saved, so an import that paused after its commit, while another wrote after it and put search right, wrote an older copy back over a newer one and deleted the other's new conversation from search (both imports succeeding). Absence from a snapshot is no evidence a file is gone. - It reads the archive as it stands when it reconciles: main's head, its indexes, and the files changed since them (`held`, `found`), not this import's snapshot. - Each conversation's bytes are read at that head (or the spool's, when they are those), and its revision is their own sha256: the bytes and the revision are one. - Each write names the revision it replaces, as search was read holding it (`replaces`, `""` for none): another import's write since is refused here (409) and left as it is, counted `overtaken`. - It deletes nothing: the platform forgets a conversation's source when a move of main deletes its file (cell/src/searchable.rs). The fake holds writers to `replaces` as the service does. Tests (each fails before): import A writes `a` and pauses, B writes a newer `a` and a new `b` and its search, then A's search changes neither; a write naming a revision another import has written over since is left as the other wrote it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed at 3347c5a. You're right: reconciliation computed from 1. An older import could undo a completed import's search reconciliation.
The existing reconciliation test no longer expects deletions. The README's "Reconciled" paragraph and the PR body describe the new rule. |
alexlwn123
left a comment
There was a problem hiding this comment.
Fail for now at 3347c5ae175cecbde2873035f152ca0c66743667.
The earlier reproduction is fixed: reconciliation rereads the archive, no longer deletes sources from a stale snapshot, and hashes the actual bytes read at a fixed head. The two new regression tests pass. One ordering of the same concurrency boundary remains broken.
Blocking:
- [P2] A newer reconciliation can lose to an older in-flight write.
cli/src/session/search.rs:205treats every compare-and-set conflict as a completed replacement worth preserving, but a changed search revision does not establish that it represents the current archive. Reproduced with a temporary test using the existing fake andwrite_one: A commitsa, reads empty search and its archive head, then pauses before upsert; B commits a newera, reads the same empty search and its newer head; A's upsert lands first; B's upsert receives 409 and returnsOvertaken. A returnsWritten(0). Search keeps A's old hash while main and its index hold B's newer conversation, so the new words remain unsearchable until another import. This does not require the file/index race accepted in the debt ledger: the archive writes are sequential and agree.
Keep the conditional writes, but on conflict reconcile again against fresh search state and the current archive, with a bounded retry and an explicit unresolved result if contention persists. Do not simply retry the captured old payload against the newly observed revision. Cover both write orderings (older-first and newer-first), for both an initially absent source and an existing source being updated; a CAS conflict alone cannot determine which writer has the authoritative bytes.
The implementation otherwise stays appropriately small by reusing held/found and the platform's shared search types and checks.
Validation: all 16 existing CLI session tests and all four core session-search tests passed. The temporary older-first reproduction failed as described and was removed; the checkout is clean. CI check and images check passed; e2e shards and real-Hermes Docker were still running when inspected. Full e2e was not run locally. The engineering-style document named by AGENTS.md was unavailable at its specified path.
…ive-search # Conflicts: # crates/e2e/src/lanes/search.rs # docs/MODEL.md
The per-fragment search index caps its chunks and refuses a write past them (507 storage_full). An import kept sending every conversation left, each refused; now its writers stop at the first such refusal and count the conversations not sent as refused, for the next import to write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the search index a Durable Object of each fragment (#846), the dev node is no host without search: its import writes the archive's search. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@alexlwn123 here's what changed since your review. Your blocker, a reconciliation that could undo a later import's search, was fixed in 3347c5a (see my reply above). Since then:
Checks: |
alexlwn123
left a comment
There was a problem hiding this comment.
Fail for now at 67f5e5ff1c8d19df86fab6d66edd906e44b21a02.
The original stale-snapshot deletion and bytes/revision issues remain fixed. The blocker from my review of 3347c5ae175cecbde2873035f152ca0c66743667 remains unchanged.
Blocking:
- [P2] A newer reconciliation still loses to an older in-flight write.
cli/src/session/search.rs:214treats every 409 asOvertakenand finishes without reconciling again. A reads search and its older archive head; B commits the newer conversation and reads the same search revision; A's upsert lands first; B's upsert conflicts. Search retains A's old conversation despite main and its index holding B's newer bytes, so the new words remain unsearchable until another import. The current SearchIndex CAS (cell/src/search_index.rs:522-525) checks revision equality, not archive freshness, so moving search into a Durable Object does not fix this ordering.
I reproduced this on this head with the existing fake and write_one, for both an initially absent source and an existing source being updated: A returns Written(0), B returns Overtaken, and search retains A's hash instead of B's. The archive commits are sequential; this does not depend on the debt ledger's file/index race.
Keep conditional writes, but on conflict reread search and the current archive and reconcile with bounded retries, reporting an unresolved result if contention persists. Do not retry the captured old payload against a fresh revision. Cover both older-first and newer-first write orderings for absent and existing sources.
The extractor and reconciliation still reuse the platform's types/checks and archive helpers appropriately; no additional complexity concern blocks this change.
Validation: all 17 existing CLI session tests passed. A temporary reproduction confirmed both stale-search cases and was removed; the checkout is clean. CI images check passed; check and real-Hermes Docker were pending when inspected, and e2e build/shards were skipped. Full e2e was not run locally. The engineering-style document named by AGENTS.md is unavailable at its specified path.
alexlwn123
left a comment
There was a problem hiding this comment.
Fail for now at 82ffed21202deb354877e01360784ec0e9ffaab0.
The original stale-snapshot deletion and bytes/revision issues remain fixed. The blocker from my previous verdict remains: this merge leaves the importer, its fake, and the server's compare-and-set behavior unchanged.
Blocking:
- [P2] A newer reconciliation still loses to an older in-flight write.
cli/src/session/search.rs:214returnsOvertakenfor every 409, without reconciling again. A reads search and its older archive head; B commits the newer conversation and reads the same search revision; A's upsert lands first; B's conflicts. Both finish, but search retains A's old bytes and the newer words remain unsearchable until another import. This affects both absent sources and updates to existing sources. The archive commits can be sequential and agree with their indexes, so this is separate from the accepted file/index race.cell/src/search_index.rs:520-528still compares revisions only; it cannot determine which payload matches the current archive.
Keep conditional writes, but on conflict reread search and the current archive and reconcile with bounded retries, reporting unresolved contention explicitly. Do not retry the captured old payload against a fresh revision. Cover older-first and newer-first write orderings for both absent and existing sources.
The shared types, validation and archive helpers remain an appropriate implementation; this merge adds no new complexity concern to the PR's own changes.
Validation: traced the failure on this head and confirmed the relevant code is unchanged from the previously reproduced head; did not rerun that temporary reproduction. cargo test -p fragment-cli session:: -- --nocapture: 34 passed, 1 ignored. CI check, images check and real-Hermes Docker were pending when inspected; e2e build/shards were skipped. Full e2e was not run locally. The engineering-style document named by AGENTS.md remains unavailable at its specified path.
Alex's #774 review: a 409 was taken as "overtaken" and the import finished, so an older import's write landing first left search holding its older copy while main and its index held the newer one. Now each pass reads search, then the archive, and writes what differs; those overtaken are read anew and reconciled again (never retried with the payload read before), at most PASSES_MAX passes, any still contended said as `unresolved`. The code.storage fake lands another writer's moves just ahead of a search upsert (`land_before_upserts`) to test older-first and newer-first orderings, for a source search held and one it did not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@alexlwn123 the blocker you raised at 67f5e5f and 82ffed2 (a newer reconciliation losing to an older in-flight write) is fixed in fded92d. What changed (
Tests:
Checks: master merged in again (362efef, no force-push), plus a clippy alias (d271123). |
alexlwn123
left a comment
There was a problem hiding this comment.
Pass at d271123110aa1f20a3d9a6a3381996c8937888a5.
The earlier blockers are fixed. Stale snapshots no longer drive deletions, revisions match the bytes read at a fixed archive head, and a CAS conflict now triggers fresh search and archive reads rather than being treated as successful reconciliation. The retry is bounded to four passes, with remaining contention explicitly reported as unresolved. The new tests cover older-first and newer-first writes for both absent and existing sources, plus sustained contention.
This makes each import repair its archive's search through the shared platform API. The extractor, shared validation, archive helpers and small bounded reconciliation loop fit that purpose; I see no remaining blocking regression or complexity concern.
Validation: CLI session tests passed (36 passed, one fixture-generation test ignored), and all four core session-search tests passed. CI check passed; real-Hermes Docker was pending, and images check and e2e build/shards were skipped when inspected. Full e2e was not run locally. The engineering-style document named by AGENTS.md was unavailable at its specified path.
An import writes its Session Archive's search, through the platform's search write API (#783):
GET /api/f/{name}/search/sources,POST …/search/upsertandPOST …/search/delete. The importer is the writer that holds each conversation's bytes, so it writes them itself. The platform knows nothing of the archive's format, and an app can't read a conversation of 1 MiB or more.What
crates/core/src/session_search.rs, pure): builds afragment_proto::search::Upsertfor each conversation file (sessions/<key>.json), with the file's sha256 as its revision and one section per turn.t<n>; a split turn's later parts aret<n>.<k>), so a conversation that grew keeps its earlier ids and only its new turns are embedded again.{kind, origin, source}. A section's context is[source, agent], itsatis the turn's first message, and itslocationis{from, to}(message indexes, both included).fragment_core::searchable::check, the cell's own check):=is kept out of filters;left_out).cli/src/session/search.rs):sources, then reads the archive at main's head (its indexes, plus the files changed since them:held,found), not the entries this import saved. Another import may have written meanwhile.replaces,""for none). So another import's write since is refused (409), left as that import wrote it, and countedovertaken.searchable::checkbefore it's sent; one the platform would refuse is reported and never sent.search: {state: "skipped", why}, and the import still succeeds.crates/fakes/src/search.rs): the write API behind the code.storage fake's host routes, switched on per fragment (enable_search). It checks an upsert with the cell's own type and check, and answers as the cell does.Tests
session_search):=, and a title or agent name far past its bytes in CJK all still give an upsertsearchable::checkaccepts.an_import_puts_its_archives_search_right: a host without search, valid writes, a replay with no upserts, an update, a lost index repaired from the archive's files, and nothing deleted.an_import_overtaken_undoes_nothing_of_a_later_ones_search(Alex's case): A writesaand pauses; B writes a neweraand a newb, then its search; A's search then changes neither.a_write_overtaken_leaves_the_others_as_it_wrote_it: a write naming a revision another import has written over since is refused and left as the other wrote it.searchsection, against the real service: importing the sessions section's export writes 3 sources. A person finds the beans chat by "plant beans", and the same import again writes nothing.sessionssection: on a node with no search service, the import says it skipped search and still succeeds.cargo xtask checkis green.Complexity ledger
crates/core/src/session_search.rscli/src/session/search.rs, one call inrunsearch; failures are reported, never fatalcrates/fakes/src/search.rs, plus a route and two levers incodestorage.rscrates/e2e/src/lanes/search.rsThere are no changes to the wire, the schema, the archive format or the cell.
🤖 Generated with Claude Code