Skip to content

fix(core): persist observations and relations on DB-first accepted writes - #1079

Merged
phernandez merged 4 commits into
mainfrom
fix/1076-accepted-write-graph-persistence
Jul 16, 2026
Merged

fix(core): persist observations and relations on DB-first accepted writes#1079
phernandez merged 4 commits into
mainfrom
fix/1076-accepted-write-graph-persistence

Conversation

@phernandez

@phernandez phernandez commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #1076. DB-first write_note calls could return success while leaving the observation and relation tables empty — graph rows appeared only if a later index_file job happened to reparse the materialized file, making schema inference and relation traversal nondeterministic after a successful write.

The accepted-write path (accepted_note_write_runner / accepted_note_mutation_runner, used by cloud and the local v2 note-content route) persisted entity scalar fields, note_content, and search rows, but never the observations/relations that were already parsed during prepare.

Root cause

_run_accepted_note_create/_update/_edit called persist_accepted_note_write() (note_content + search) but nothing wrote the parsed graph. The graph was only populated later, out of band, by a storage-notification-triggered index_file pass — which is not a valid commit path and can no-op against a matching checksum.

Fix

Persist the graph in the same transaction as the entity and note_content, reusing the parsed markdown so there is no second parse:

  • Repositoriesreplace_accepted_observations / replace_accepted_outgoing_relations with AcceptedObservationWrite / AcceptedRelationWrite payloads. Delete-then-insert mirrors the replace-not-merge semantics EntityService.update_entity_and_observations / update_entity_relations already use for the file-indexing path. Ordinary relations are written unresolved (to_id=None) for the existing forward-reference job; ambiguity-safe self-relations carry to_id in the accepted transaction because deferred resolution intentionally skips self targets.
  • Runner — new replace_accepted_note_graph() wired into create/update/edit alongside persist_accepted_note_write(). Move is intentionally untouched (it only rewrites path/permalink, not the content graph).
  • Mapping / wiringPreparedEntityWrite exposes .observations / .relations mapped from the already-parsed entity_markdown; LocalAcceptedNoteRepositories gains the two repos.

Test coverage

  • Repository delete/replace/clear semantics; ordinary relations asserted unresolved and safe self-relations asserted resolved.
  • Runner graph handoff (replace_accepted_note_graph), including the empty-set clear.
  • Create/update/edit orchestration proving the graph is written immediately: create/replace persist, edit clears when the markdown drops it (issue items 1–2).
  • PreparedEntityWrite graph mapping through the real markdown parser (parse → map → payload).

Verification: just fast-check clean; targeted repository/indexing/services suites pass (1352 passed across the affected dirs); the focused PR regression suite passes (90 passed); just doctor end-to-end file↔DB loop passes.

🤖 Generated with Claude Code

…ites

DB-first `write_note` calls (the accepted-write path used by cloud, and by
the local v2 note-content route) persisted entity scalar fields, note_content,
and search rows, but never the parsed observations or relations. Graph rows
only appeared if a later `index_file` job happened to reparse the materialized
file, so schema inference and relation traversal were nondeterministic after a
successful write (issue #1076).

The accepted markdown is already parsed during prepare, so persist the graph in
the same transaction instead of waiting for a reindex:

- Add `replace_accepted_observations` / `replace_accepted_outgoing_relations`
  to the observation/relation repositories, with `AcceptedObservationWrite` /
  `AcceptedRelationWrite` payloads. Delete-then-insert mirrors the replace-not-
  merge semantics `EntityService.update_entity_and_observations` /
  `update_entity_relations` already use for the file-indexing path. Relations
  are written unresolved (`to_id=None`); the forward-reference resolution job
  links them later, matching cloud's one-file materialization semantics.
- Add `replace_accepted_note_graph()` to the storage-neutral accepted-write
  runner and wire it into `_run_accepted_note_create/_update/_edit` alongside
  `persist_accepted_note_write()`. Move is unchanged — it only rewrites the
  path/permalink, not the content graph.
- Expose `.observations` / `.relations` on `PreparedEntityWrite`, mapped from
  the already-parsed `entity_markdown`, and wire the two repos into
  `LocalAcceptedNoteRepositories`.

Regression coverage: repository delete/replace/clear semantics (relations
asserted unresolved); the runner graph handoff; create/update/edit orchestration
proving the graph is written immediately (create/replace persist, edit clears);
and the `PreparedEntityWrite` graph mapping through the real markdown parser.

Also grant the Claude issue-triage workflow `contents: write` +
`pull-requests: write` and broaden its `--allowed-tools` so it can implement,
self-verify, and open a PR. Previously the workflow's read-only GITHUB_TOKEN
made `git push` fail with a 403 for github-actions[bot], and the restricted
tool list blocked test runs — both hit while triaging this issue.

Closes #1076

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@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: 188286a3f7

ℹ️ 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".

Comment thread .github/workflows/claude-issue-triage.yml Outdated
Comment thread src/basic_memory/repository/observation_repository.py
Signed-off-by: phernandez <paul@basicmachines.co>
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@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: 1bab2c764b

ℹ️ 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".

Comment thread src/basic_memory/repository/relation_repository.py Outdated
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: a512224b94

ℹ️ 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".

@phernandez
phernandez merged commit b714252 into main Jul 16, 2026
26 checks passed
@phernandez
phernandez deleted the fix/1076-accepted-write-graph-persistence branch July 16, 2026 14:52
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.

DB-first note writes omit observations and relations until a later file re-index

1 participant