Skip to content

fix(core): accept-time publication marker; PUT renames announce moves; real-DB runner tests - #1668

Merged
phernandez merged 6 commits into
mainfrom
accepted-write-publication-marker
Oct 7, 2026
Merged

phernandez merged 6 commits into
mainfrom
accepted-write-publication-marker

Conversation

@phernandez

@phernandez phernandez commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Summary

Closes #1661.

Publication marker in the accept transaction

  • The accept transaction writes the pending relation_search_refresh marker for the accepted generation, through RelationRepository.record_pending_relation_publication (the single writer, also used by the publisher's begin). An accepted note is never visible without it.
  • begin reuses a marker already pending for its generation, so a successful publication leaves exactly one unit of refresh work. A failed publication leaves it pending, which keeps change detection's repair path.
  • Why: Basic Memory Cloud wrote its own marker after the request so its Wiki report would wait. That marker landed after publication cleanup, stayed pending, and masked the entity checksum, so every app-written note was fully re-indexed when its storage notification arrived. Cloud drops that fence once this ships.

PUT renames announce the move

  • A PUT whose destination differs from the note's path recorded a move on its project change, but its materialization carried no previous path. The materialization now takes previous_file_path from the project change in attach_accepted_project_note_change, so a PUT rename and an explicit move carry the same evidence. The planners no longer accept an independent second value.

Tests

  • The accepted-note runner tests drove every mutation with a stand-in session and fake repositories. They now run the production mutation dependencies against a real database (test-int/test_accepted_note_mutation_runner_db.py): create, update, edit, move, delete, base-checksum conflicts, relay supersession, materialization states, directory casing, graph policy, pending markers, inbound-link cleanup, PUT-rename move evidence. Races are injected at the note lock and say so. Only pure mapping tests remain as unit tests. Net −2,100 lines.
  • test-int/test_note_write_outcomes.py: a published write leaves nothing pending; the accepted generation is pending before post-commit publication starts.
  • Two SQLite engine tests now pass an explicit SQLite config instead of reading the developer's home config.
  • Each new guard was checked by breaking the fix and watching the test fail.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF

phernandez and others added 2 commits October 7, 2026 09:45
…action

An accepted note generation is now never visible without its pending
publication marker. The marker used to be written by the post-commit
publisher in its own transaction, so a reader between the accept commit
and that step saw an accepted generation with no marker, and a hosted
runtime that needed one had to write its own after the fact.

The publisher's begin step reuses a marker already pending for its
generation instead of adding a second one, so a successful publication
leaves exactly one unit of refresh work. A failed publication still
leaves the marker pending, which keeps change detection's repair path.

Closes #1661

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
The create-runner tests drove the runner with a stand-in session and fake
repositories, so they could not see what the accept transaction writes.
They now run the production mutation dependencies in a real transaction
and assert the persisted note, project change, graph publication, pending
marker, path conflicts and directory casing from the database.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T16:02:54.591990Z 6f0735f New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37a3348d82

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

is accepted; the post-commit publisher converts it once the graph is written, and a
publication that fails leaves it pending for change detection to repair.
"""
session.add(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Update the fake sessions for the new marker write

When the unit suite exercises persist_accepted_note_snapshot() or persist_accepted_note_move(), four existing tests pass either a plain object() or _FlushSession, neither of which implements add(), so this new call raises AttributeError before their assertions run. uv run pytest -q tests/indexing/test_accepted_note_write_runner.py now reports 4 failures; route this persistence through the repository protocol or update those fake sessions so the required unit suite remains green.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 4afe067. The marker now goes through the repository protocol: RelationRepository.record_pending_relation_publication is the single writer, used by the accept transaction (via relation_repository(project_id)) and the publisher's begin step. The fake-session runner tests are replaced by real-DB tests in test-int/test_accepted_note_mutation_runner_db.py rather than teaching the fakes the new method.

phernandez and others added 4 commits October 7, 2026 10:19
…ository

The accept transaction recorded its pending marker with a bare session.add,
the only accept-time write that bypassed the repository capabilities. The
marker now has one writer, RelationRepository.record_pending_relation_publication,
used by both the accept transaction and the publisher's begin step.

The accepted-note runner tests drove every mutation with a stand-in session
and fake repositories, so they could not see what a transaction actually
writes. They now run the production mutation dependencies against a real
database: create, update, edit, move and delete, including base-checksum
conflicts, relay supersession, materialization states, directory casing,
graph policy, pending markers and inbound-link cleanup. Races are injected
at the note lock and say so. Only pure mapping tests remain as unit tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
A PUT whose destination differs from the note's path records the change as
a move, but its materialization carried no previous path, so runtimes that
announce moves from the materialization could only report an update and
left readers holding the old path. The project change already records what
the mutation did; the materialization now takes its previous path from it,
so a PUT rename and an explicit move carry the same move evidence and the
planners no longer accept a second, independent value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
Both tests built their engine without a config, so the database backend came
from the developer's home config. On a machine configured for Postgres they
ran SQLite-only SQL against Postgres and failed. They now pass an explicit
SQLite config.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
…s match

Text mode on Windows writes CRLF, so a seeded source object no longer hashed
to the accepted Markdown and the move-cleanup cases failed only there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8wjd6HrtSA2LN9ssC4NzF
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez phernandez changed the title fix(core): record the relation publication marker in the accept transaction fix(core): accept-time publication marker; PUT renames announce moves; real-DB runner tests Oct 7, 2026
@phernandez
phernandez merged commit 23b1b4a into main Oct 7, 2026
51 of 52 checks passed
@phernandez
phernandez deleted the accepted-write-publication-marker branch October 7, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Persist the relation-publication retry marker inside the accepted-write transaction

1 participant