fix(tracker): sync the replica before the bulk import's parity check - #1359
fix(tracker): sync the replica before the bulk import's parity check#1359joeharris76 wants to merge 2 commits into
Conversation
The bulk transfer writes straight to the primary, while the command's connection reads through a local embedded replica opened before it. Parity was computed against that stale replica, so a CORRECT import reported every record absent: the first production run printed 'PARITY FAILED' immediately after transferring all 432 rows successfully, and a fresh connection then confirmed parity OK 64/64 with zero diffs. It fails safe -- staleness can only under-report presence, never invent it -- but it makes the exit code untrustworthy, which defeats the point of a self-asserting rung. Sync before comparing, with a regression test asserting parity is never computed against an unsynced replica.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…tracker Phase 5's import is complete: all 65 records are in the tracker's findings tables at SCHEMA_VERSION 4, with verified field-level parity (every frontmatter key, every body section byte-for-byte, each triage-log line as one imported_triage_log event) and a byte-identical finding_events restore proof. Announce the freeze at the top of the directory README, point capture and triage at the todo finding subcommands, and state plainly that the freeze IS the rollback window: these files stay until phase 6 so a parity problem still has a fallback. Git history is untouched either way.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d2d237a21
ℹ️ 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".
| replica_sync = getattr(conn, "sync", None) | ||
| if callable(replica_sync): | ||
| replica_sync() |
There was a problem hiding this comment.
When another tracker process starts in the same worktree during this bulk import, this call can sync the shared replica concurrently because the connection-time _replica_setup_lock has already been released. _project/scripts/todo_db.py explicitly identifies concurrent multi-process sync as a corruption hazard and serializes every existing open-and-sync operation, so this additional sync should use the same lock or a synchronized connection helper.
Useful? React with 👍 / 👎.
| replica_sync = getattr(conn, "sync", None) | ||
| if callable(replica_sync): | ||
| replica_sync() |
There was a problem hiding this comment.
When the transfer succeeds but the subsequent replica refresh fails because of a network, authentication, or local-replica error, _HostedConnection.sync() forwards the raw libsql exception while todo_db.main() only catches TodoError and sqlite3.Error around dispatch. Such a failure therefore produces a traceback after the data has already landed instead of the tracker's clean, redacted exit-2 error; wrap or map the sync exception before it escapes.
Useful? React with 👍 / 👎.
| > All 65 records now live in the shared tracker's findings tables | ||
| > (`SCHEMA_VERSION` 4), imported with verified field-level parity: every |
There was a problem hiding this comment.
The directory contains 65 Markdown files only when README.md is counted; the importer and parity_report() explicitly exclude that file, leaving 64 actual records, consistent with the reported 64/64 parity run. Claiming that all 65 records were imported overstates the migration proof used to justify the phase-6 demolition, so this should say 64 or explicitly identify a separate 65th database record.
Useful? React with 👍 / 👎.
1 participant
The bulk transfer writes straight to the primary, while the command's connection reads through a local embedded replica opened before it. Parity was computed against that stale replica, so a CORRECT import reported every record absent: the first production run printed 'PARITY FAILED' immediately after transferring all 432 rows successfully, and a fresh connection then confirmed parity OK 64/64 with zero diffs.
It fails safe -- staleness can only under-report presence, never invent it -- but it makes the exit code untrustworthy, which defeats the point of a self-asserting rung. Sync before comparing, with a regression test asserting parity is never computed against an unsynced replica.