Skip to content

fix(code-locator): #334 — validate_symbols returns indexed_at_sha for snapshot-drift detection#527

Merged
silongtan merged 2 commits into
devfrom
fix/334-validate-symbols-indexed-at-sha
May 28, 2026
Merged

fix(code-locator): #334 — validate_symbols returns indexed_at_sha for snapshot-drift detection#527
silongtan merged 2 commits into
devfrom
fix/334-validate-symbols-indexed-at-sha

Conversation

@silongtan

Copy link
Copy Markdown
Collaborator

Summary

ValidateSymbolsTool now reads head_commit + repo_path from the index_meta table at init (populated by code_locator_runtime.record_index_state after every rebuild) and exposes them as indexed_at_sha / indexed_at_path on every ValidatedSymbol. Caller-LLMs compare indexed_at_sha against authoritative_sha before bind to detect the snapshot drift that causes false-positive validate→reject sequences.

Closes #334 (option C lite — additive metadata) as the immediate, reversible mitigation. Option A (snapshot-pinned validate or ref-in-bind-payload) and #332 (bind ignoring link_commit HEAD advancement) remain as follow-ups that need Jin's RPC-contract input — see #334 design comment.

Why this shape

validate_symbols reads from the local SQLite symbol index. bind validates via git show {authoritative_sha}:{file_path} + tree-sitter. These are two data sources at two different refs. When the index is more recent than authoritative_sha (the field bug Jacob reported: feature-branch symbol satisfies validate with score 100, then bind hard-rejects), the documented preflight handshake misleads the caller.

This PR doesn't fix the underlying disagreement — it makes it visible. Pure additive metadata, no behavior change to bind, no daemon-side change. If empty (""), the index pre-dates ref tracking (legacy build or skipped record_index_state); caller-LLMs treat empty as "snapshot unknown" and apply caution per the updated skill.

What changed

File Change
code_locator/models.py ValidatedSymbol gains indexed_at_sha + indexed_at_path (both default "")
code_locator/indexing/sqlite_store.py New SymbolDB.read_index_meta(key) — handles missing-table case gracefully
code_locator/tools/validate_symbols.py Reads sha+path once at tool init (alongside the symbol-list cache); populates every result
skills/bicameral-bind/SKILL.md New "Snapshot drift (#334)" section + step 3 guidance per CLAUDE.md's Tool Changes Require Skill Changes mandate
tests/test_validate_symbols_indexed_at_sha.py 3 sociable tests (real SymbolDB, real build_index, real git subprocess)

Test plan

  • pytest tests/test_validate_symbols_indexed_at_sha.py -v — 3 passed
    • full build+record_index_state cycle → indexed_at_sha echoes git HEAD
    • missing-meta legacy path → returns "" without raising
    • init-time caching → post-init commit doesn't change the cached sha
  • pytest tests/test_phase1_code_locator.py -v — no regression (3 passed, 1 skipped as before)
  • Pydantic model construction smoke — defaults populate to ""
  • Pre-commit ruff check + ruff format clean
  • CI: MCP Regression Suite (ubuntu + windows), Lint & Type Check, Schema Persistence, Perf Gate, Secret Scan, v0 e2e

Out of scope (deliberate)

Audit trail

🤖 Generated with Claude Code

… snapshot-drift detection

`ValidateSymbolsTool` now reads `head_commit` + `repo_path` from the
`index_meta` table at init (populated by `code_locator_runtime.record_index_state`
after every rebuild) and exposes them as `indexed_at_sha` / `indexed_at_path`
on every `ValidatedSymbol`. Caller-LLMs compare `indexed_at_sha` against
`authoritative_sha` before bind to detect the snapshot drift that causes
false-positive validate→reject sequences (field bug, Jacob 2026-05).

Pure additive metadata — no behavior change to bind, no daemon-side change.
Empty string when the index pre-dates ref tracking (legacy build or skipped
`record_index_state`); caller treats that as "snapshot unknown."

Skill `bicameral-bind` SKILL.md updated per the Tool Changes Require
Skill Changes mandate — adds a "Snapshot drift (#334)" section and
extends step 3 with the ref-comparison guidance.

Three sociable tests (real SymbolDB, real `build_index`, real git
subprocess) cover: full-build sha echoes git HEAD; missing-meta legacy
path returns ""; init-time caching survives a post-init commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9f376859-ffab-40ef-8ee5-2b3c3fd06ee9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/334-validate-symbols-indexed-at-sha

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@silongtan

Copy link
Copy Markdown
Collaborator Author

e2e fail diagnosis: infra, not this PR

Downloaded the failing run's transcripts (v0-user-flow-e2e-transcripts artifact from run 26493052974). Every flow hits the same Claude API 403 on turn 1:

api_error_status: 403
error: "oauth_org_not_allowed"
text: "Your organization has disabled Claude subscription access for Claude Code · Use an Anthropic API key instead, or ask your admin to enable access"
duration_api_ms: 0
num_turns: 1
total tool calls: 0

The CLI is rejected at auth time before any prompt processing happens — that's why every flow reports total tool calls: 0 and identical-shaped downstream assertion failures (processed-transcripts is empty, expected bicameral.history; saw: [], etc.). The production GitHub environment's CLAUDE_CODE_OAUTH_TOKEN is no longer accepted at the Anthropic org level.

Filed as #528 — this affects every PR touching the workflow's path filter (skills/bicameral-, handlers/, ledger/, server.py, contracts.py, pyproject.toml, tests/e2e/) and needs an org-level toggle or workflow-side API-key migration.

This PR's actual test surface

All 10 non-e2e checks green:

  • ruff + mypy ✓
  • MCP Regression Suite (ubuntu + windows) ✓
  • Schema Persistence ✓ (both runners)
  • Perf Gate ✓
  • Install + import floor ✓
  • Preflight Failure-Mode Eval (advisory) ✓
  • TruffleHog ✓
  • PR body refs lint ✓

Plus locally: 3 new sociable tests pass, 3 Phase 1 regression tests pass, ruff + format clean.

Requesting merge despite the red e2e — the failure is upstream from any code in this PR. Happy to wait if a reviewer prefers to land #528's fix first.

🤖 Generated with Claude Code

…auth_org_not_allowed 403)

The `production` env's CLAUDE_CODE_OAUTH_TOKEN was tied to an org Claude
Code subscription that has been disabled, surfacing as
oauth_org_not_allowed 403 on every flow before turn 1 and red-gating every
PR. Switch the assertions job to the `ci-test` env's ANTHROPIC_API_KEY —
`claude -p` honours the key natively, the same env already powers
test-mcp-regression.yml + preflight-eval.yml, and CI is now decoupled from
subscription policy.

The recording job already uses ANTHROPIC_API_KEY for interactive `claude`;
this aligns the assertions job with that contract.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@silongtan silongtan requested a deployment to recording-approval May 28, 2026 01:28 — with GitHub Actions Waiting
@silongtan silongtan requested a review from jinhongkuan May 28, 2026 01:45
@silongtan silongtan merged commit 5619724 into dev May 28, 2026
11 of 12 checks passed
@silongtan silongtan deleted the fix/334-validate-symbols-indexed-at-sha branch May 28, 2026 02:33
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.

1 participant