Skip to content

fix(embeddings): enumerate nodes independently of stored paths - #962

Open
y4ho0 wants to merge 1 commit into
tirth8205:mainfrom
y4ho0:fix/embed-legacy-paths
Open

fix(embeddings): enumerate nodes independently of stored paths#962
y4ho0 wants to merge 1 commit into
tirth8205:mainfrom
y4ho0:fix/embed-legacy-paths

Conversation

@y4ho0

@y4ho0 y4ho0 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Linked issue

Closes #945

What & why

Graphs with legacy Windows path spellings can produce zero embeddings because node enumeration passes stored paths through normalized file lookups. The tool then incorrectly reports that semantic search is active.

Explicit embedding and opted-in refresh now enumerate all non-File nodes directly, including virtual nodes outside the file inventory. The summary reports that semantic search is unavailable when the resulting embedding count is zero; an already-current index still reports active. Stored paths, node identities, and provider/model checks are preserved.

How it was tested

Python 3.13.15 on macOS. Regression tests use real SQLite stores with simulated legacy Windows paths and deterministic embedding providers. They cover initial embedding, explicit refresh, repeat runs, virtual nodes, and empty/File-only graphs.

uv run --frozen pytest tests/ --tb=short -q --disable-warnings
# 2993 passed, 9 skipped, 2 xpassed.
uv run --frozen ruff check code_review_graph/ tests/test_embedding_refresh.py
# All checks passed.
uv run --frozen mypy code_review_graph/ --ignore-missing-imports --no-strict-optional
# Success: no issues found in 72 source files.

The coverage run reached 85.06% (65% required), with 2,992 passed, 9 skipped, 2 xpassed, and one failure in the unchanged real filesystem-watcher test test_recreated_directory_survives_and_is_reindexed. That test passed in isolated reruns on both this branch and clean upstream. An initial Python 3.14 run also hit the existing Windows event-loop test failure, reproduced on the unmodified base; final validation uses CI-supported Python 3.13.

Checklist

  • Regression tests added
  • Full test suite passes on Python 3.13
  • Linting passes
  • Type checking passes
  • Changed lines are at most 100 characters
  • Tool return documentation describes the zero-embedding summary

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

code-review-graph review

Overall risk: 0.40 (MEDIUM) — 8 changed function(s)/class(es), 17 affected flow(s), 1 test gap(s)

Risk-scored changes

Risk Level Symbol Location Tested
0.40 medium code_review_graph/tools/docs.py::embed_graph code_review_graph/tools/docs.py:28 yes
0.30 low code_review_graph/embeddings.py::embed_all_nodes code_review_graph/embeddings.py:1307 yes
0.30 low code_review_graph/embeddings.py::refresh_embeddings code_review_graph/embeddings.py:1318 yes
0.05 low tests/test_embedding_refresh.py::TestEmbeddingNodeEnumeration tests/test_embedding_refresh.py:114 no
0.05 low tests/test_embedding_refresh.py::TestEmbeddingNodeEnumeration.test_embeds_non_file_nodes_outside_the_file_i... tests/test_embedding_refresh.py:115 (test)
0.05 low tests/test_embedding_refresh.py::TestEmbeddingNodeEnumeration.test_manual_embed_reaches_nodes_regardless_of_... tests/test_embedding_refresh.py:134 (test)
0.05 low tests/test_embedding_refresh.py::TestEmbeddingNodeEnumeration.test_refresh_reaches_changed_nodes_regardless_o... tests/test_embedding_refresh.py:152 (test)
0.05 low tests/test_embedding_refresh.py::TestEmbeddingNodeEnumeration.test_tool_summary_reflects_whether_any_vectors... tests/test_embedding_refresh.py:173 (test)

Affected execution flows

  • embed_query — criticality 0.48, 2 node(s) across 1 file(s)
  • embed_query — criticality 0.48, 2 node(s) across 1 file(s)
  • embed_query — criticality 0.45, 3 node(s) across 1 file(s)
  • embed_query — criticality 0.45, 3 node(s) across 1 file(s)
  • embed_query — criticality 0.41, 2 node(s) across 1 file(s)
  • ...and 12 more affected flow(s)

Test gaps

  • tests/test_embedding_refresh.py::TestEmbeddingNodeEnumeration (tests/test_embedding_refresh.py:114)

Token savings: this graph-backed report used ~14,880 fewer tokens (~69%) than reading every changed file in full (estimated, chars/4 approximation).


Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

@tirth8205

Copy link
Copy Markdown
Owner

Integrated on integration/token-efficiency-hardening as f68182d. Embedding refresh now enumerates non-File nodes directly, retaining legacy paths and virtual nodes, and reports zero-vector availability accurately. Targeted refresh tests passed with the existing provider opt-in rules preserved; two unchanged loopback-server tests were unavailable in the sandbox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants