-
Notifications
You must be signed in to change notification settings - Fork 15
fix(titan): verify embeddings per-worktree instead of trusting stale state flag #1805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,14 @@ codegraph embed -m minilm | |
|
|
||
| This enables `codegraph search` for duplicate code detection in downstream phases. If it fails (e.g., missing model), note it and continue — DRY checks will be grep-only. | ||
|
|
||
| **Verify, don't assume.** `.codegraph/` is gitignored — `graph.db` (and its embeddings) is local filesystem state, per worktree. It is never carried over by `git merge`, a branch switch, or a snapshot restore. Before setting `embeddingsAvailable` in `titan-state.json`, smoke-test the current worktree's DB directly: | ||
|
|
||
| ```bash | ||
| codegraph search "test query" --json | ||
| ``` | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The skill correctly smoke-tests the engine before setting
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — RECON now records |
||
| Only set `embeddingsAvailable: true` if this returns results (or a valid empty match, not an error/`ENGINE_UNAVAILABLE`). Alongside the flag, record `embeddingsWorktreePath` in `titan-state.json` as the output of `git rev-parse --show-toplevel` — this gives downstream phases a deterministic identity to compare against instead of having to re-derive trust from a smoke test alone. If a downstream phase (e.g. GAUNTLET) ends up operating in a **different worktree** than the one RECON ran `codegraph embed` in — including via "merge the other worktree's branch into mine" — its `graph.db` will not have embeddings even though a stale `titan-state.json` from elsewhere claims `embeddingsAvailable: true`; a mismatch between `embeddingsWorktreePath` and that worktree's own `git rev-parse --show-toplevel` is the unambiguous signal for this. Re-run `codegraph embed -m minilm` for the worktree actually being used, update both `embeddingsAvailable` and `embeddingsWorktreePath`, and re-verify with the smoke-test above, whenever the operating worktree changes. | ||
|
|
||
| --- | ||
|
|
||
| ## Step 3 — Collect baseline metrics | ||
|
|
@@ -236,6 +244,7 @@ Include the preserved `phaseTimestamps` in the state file below. | |
| "lastBatch": null | ||
| }, | ||
| "embeddingsAvailable": true, | ||
| "embeddingsWorktreePath": "<git rev-parse --show-toplevel>", | ||
| "stats": { | ||
| "totalFiles": 0, | ||
| "totalNodes": 0, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GAUNTLET smoke-test condition "if it errors or returns empty where a hit would be expected" leaves the executing agent to judge subjectively whether a hit from
codegraph search "test query" --jsonis "expected" — and for a generic query like"test query", the agent will almost always decide no hit is expected. This means GAUNTLET will not trigger regeneration on a silent-empty response, which is precisely what the incident report says happened:codegraph searchran without erroring and returned empty results without any explicitENGINE_UNAVAILABLEsignal.RECON's condition is more concrete ("not an error/
ENGINE_UNAVAILABLE"), but the two conditions aren't symmetrically guarding against the same failure mode. The most deterministic fix would be to have RECON record the worktree path alongsideembeddingsAvailable(e.g., a newembeddingsWorktreePathfield intitan-state.json) and have GAUNTLET compare its current working directory against that value — a mismatch would be an unambiguous trigger for regeneration regardless of what the smoke test returns.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — RECON now records
embeddingsWorktreePath(git rev-parse --show-toplevel) intitan-state.jsonalongsideembeddingsAvailable. GAUNTLET's Rule 11 note now does a deterministic path comparison against that field first (mismatch or missing field = don't trust the flag), and the smoke-test fallback now uses the same explicit error/ENGINE_UNAVAILABLEcriterion RECON uses instead of the subjective "empty where a hit would be expected" judgment call.