Skip to content

Avoid rewriting Phase 2 selection on clean workspace#19812

Merged
jif-oai merged 1 commit intomainfrom
jif/concurrency-load
Apr 27, 2026
Merged

Avoid rewriting Phase 2 selection on clean workspace#19812
jif-oai merged 1 commit intomainfrom
jif/concurrency-load

Conversation

@jif-oai
Copy link
Copy Markdown
Collaborator

@jif-oai jif-oai commented Apr 27, 2026

Why

Phase 2 can now claim the global consolidation lock on startup even when the git-backed memory workspace is already clean. The clean-workspace path still finalized through the normal Phase 2 success path, which clears and re-marks selected_for_phase2 rows. That made no-op startups perform avoidable writes to stage1_outputs, creating unnecessary DB I/O and contention when no memory files changed.

What Changed

  • Added a preserving-selection Phase 2 finalizer in codex-state that only marks the global job row as succeeded.
  • Kept the existing mark_global_phase2_job_succeeded behavior for real consolidation runs, where the selected Phase 2 snapshot must be rewritten.
  • Switched the succeeded_no_workspace_changes branch in core/src/memories/phase2.rs to use the preserving-selection finalizer.
  • Added a regression test that installs a SQLite trigger on stage1_outputs and verifies the clean finalizer performs zero updates there.

Testing

  • cargo test -p codex-state
  • cargo test -p codex-core memories::tests::phase2

@jif-oai jif-oai requested a review from a team as a code owner April 27, 2026 12:59
@jif-oai jif-oai changed the title fix: load on empty phase 2 Avoid rewriting Phase 2 selection on clean workspace Apr 27, 2026
@jif-oai
Copy link
Copy Markdown
Collaborator Author

jif-oai commented Apr 27, 2026

@codex review

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

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: 2fc7a2bfb4

ℹ️ 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 on lines +125 to 132
job::succeed_preserving_selection(
session,
db,
&claim,
new_watermark,
&raw_memories,
"succeeded_no_workspace_changes",
)
.await;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Badge Rebuild selection snapshot before clean-phase2 finalize

This clean-workspace path now skips rewriting stage1_outputs.selected_for_phase2. After migrations 0017/0018, existing rows default to 0/NULL; if Phase 2 is clean, the snapshot never gets repaired. Code that depends on this flag (mark_thread_memory_mode_polluted enqueue and retention pruning) can then treat baseline rows as unselected, causing missed forgetting runs or pruning of active baseline memories.

Useful? React with 👍 / 👎.

@jif-oai jif-oai merged commit 79b4f69 into main Apr 27, 2026
24 of 25 checks passed
@jif-oai jif-oai deleted the jif/concurrency-load branch April 27, 2026 13:14
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant