fix: full-project review findings + retrieval precision (26% fewer tokens served)

[fazleelahhee]

Summary

Two bodies of work, sequenced: fixes for all findings from the 2026-07-03 full-project review, then Phase 1 of the token-savings program (design: docs/specs/2026-07-03-token-savings-design.md).

Review fixes (~30 findings, ~60 regression tests)

• Config writers: string-aware JSONC stripping (URLs no longer corrupt configs); parse failures skip-with-warning instead of overwriting user MCP configs; cce uninstall matches CCE markers only, never bare "cce"
• Memory: vec tables now created on later connect after a no-sqlite-vec bootstrap; turn re-compression fires FTS/vec cleanup triggers (DELETE+INSERT); compression queue dead-letters at 5 attempts; savings recorded only on success; PII scrub on prompt/tool-payload writes
• Retrieval/storage: FTS-only chunks no longer get perfect vector scores; per-token FTS queries (BM25 leg actually works); true cosine metric with legacy-L2 detect-and-rebuild (existing indexes need one cce index --full); rollback on mid-batch store failures
• Indexer: tree-sitter byte offsets slice bytes (non-ASCII files chunk correctly); watcher path honors secret/ignore rules; deletions prune the index; unquoted credentials redacted; Stripe/GitHub vendor regexes no longer leak key bodies (capturing-group bug)
• Dashboard: all XSS sinks escaped; inline handlers → data attributes; atomic state.json writes

Token-savings Phase 1 — retrieval precision

A/B-measured on this repo: 26.1% fewer tokens served per query, hit rate unchanged (4/8 → 4/8). Evidence with rejected parameter runs and limitations: benchmarks/results/cce-phase1-ab.md.

• modified_ts persisted through the vector store (file mtime + git commit time) — activates the previously-inert recency weight
• Overlap dedup: same-file chunks overlapping >50% collapse to the higher-scored one
• Marginal-utility stop (retrieval.marginal_ratio, default 0.75 evidence-tuned, 0 disables) with a top-1 guarantee
• Truthful "results omitted" note driven by an actual dropped-count signal, pointing at the right knob
• Benchmark --ab mode for reproducible before/after comparisons

Notes

• Branch history was rebased pre-push to replace fake Stripe-key test fixtures with runtime concatenations (GitHub push protection flagged the literals).
• Known follow-ups (logged in decision records): standard run_benchmark() mode has a pre-existing storage-path bug; re-validate marginal_ratio=0.75 on external corpora; cce search CLI doesn't yet use the retrieval knobs.

Test plan

• Full suite: 1056 passed, 1 skipped (uv run --no-sync pytest -q)
• ~70 new regression tests, each confirmed failing before its fix (TDD)
• A/B benchmark gate: ≥25% reduction with hit rate ≥ baseline — PASS (26.1%, unchanged)
• Final whole-branch review (cross-task integration focus) passed; both findings fixed

🤖 Generated with Claude Code

[rajkumarsakthivel]

One bug, same root cause in both failures.

Lint: F821 Undefined name 'source' at chunker.py:176
Tests: test_extract_imports_csharp — NameError: name 'source' is not defined

What happened: PR #123 correctly refactored _parse_import_module to use src_bytes + _node_text() for multi-byte correctness. But the C# using_directive block — added by #121 after this branch was cut — was rebased in still using the old source variable, which no longer exists in this scope.

Fix needed in _parse_import_module:

elif node.type == "using_directive":
    # C#: "using System.Collections.Generic;" — take root namespace
    for child in node.children:
        if child.type in ("qualified_name", "identifier"):
            name = _node_text(src_bytes, child).strip()  # was: source[...] — source no longer exists
            return name.split(".")[0]

Two-character change: source[child.start_byte:child.end_byte] → _node_text(src_bytes, child). That's the only thing blocking CI.

Windows passes because the test fixture is ASCII — no byte/str index divergence triggered there, but the NameError is still present and would surface on any non-ASCII C# file.

[fazleelahee]

Thanks for the catch @rajkumarsakthivel — that was exactly right. Fixed in c0dc8a2: the C# using_directive block now uses _node_text(src_bytes, child) instead of the stale source[...], resolving both the F821 lint and the test_extract_imports_csharp NameError. The collision came from #121's C# support landing after this branch was cut; merging main in surfaced it.

Status now:

• All 19 CI checks green (lint + test across Ubuntu/macOS/Windows × 3.11–3.13)
• Branch merged up to date with main; conflict resolved (the fix: address four security/reliability issues (#127-130) #131 secret-target fix and this branch's watcher fix reconciled)
• Full suite: 1073 passed, 1 skipped

Also verified at runtime (not just tests) in a throwaway project via the real CLI + dashboard:

• Indexed a C# file with using Société.Paiements; + non-ASCII comments → no NameError, chunk content byte-perfect (the multi-byte case the lint error warned about)
• .env.production with a Stripe key never enters the index (raw vectors.db grep = 0)
• cce init merges context-engine into an existing valid .mcp.json with a https:// URL server without corrupting it; skips-and-warns (preserving user config) on an unparseable one

Could you re-review when you have a moment? This is stacked ahead of the token-savings Phase 2–4 work.

[rajkumarsakthivel]

Solid work. CI passes. Here are my notes — all minor, none blocking.

What's good

• C# multi-byte fix (_node_text) is correct and applied consistently everywhere — this was the root cause of the original failure.
• FTS-only chunk distance fix is the right call: defaulting to 0.0 was silently granting perfect vector similarity to keyword-incidental hits. Using max(observed_distances) as the fallback is defensible and conservative.
• marginal_ratio stop is clean: sorted input means the break is valid, diverse and … ensures the first chunk always passes (implicit top-1 guarantee at the diversity level), 0 disables.
• _dedupe_overlaps O(n²) is fine at these set sizes; the >50% shorter-chunk threshold avoids false matches between adjacent but non-overlapping regions.
• Top-1 guarantee on the confidence threshold prevents an overly tight config from returning empty results.
• stats_out is a clean way to thread stats back without changing the return type.
• project_dir.resolve() — the macOS /tmp → /private/tmp symlink issue is a real footgun, good catch.
• Non-capturing groups in the secret regexes ((?:...) instead of (...)) — the comment explaining why is exactly right.
• atomic_write_text for state.json fixes an actual race.
• .mcp.json parse error returning None instead of clobbering is the correct behavior; the docstring update makes the contract clear.

Minor observations (no changes needed)

dropped_low_value note: the counter is accumulated across both the confidence threshold step and the marginal stop. Chunks that the per-file cap would have excluded anyway are also counted once the marginal stop fires. The note is still accurate ("lower-confidence results omitted") — just note that the count can be slightly higher than the pure marginal-stop count when chunks from a single file dominate the tail.

docs/plans/2026-07-03-retrieval-precision.md: 753 lines of agentic scaffolding committed to the permanent repo is some noise. Fine to merge as-is; worth having a team convention on whether plans live in docs/plans/ long-term or get cleaned up after the work lands.

The omitted-results note is appended with a single \n separator — if body already ends with a newline (which the format functions usually produce), this yields a blank line before the note. Cosmetically fine.