Skip to content

Merge train: 8 tested PRs and a test-gate fix (2026-10-15) - #1115

Merged
tirth8205 merged 35 commits into
stagingfrom
train/2026-10-15
Oct 15, 2026
Merged

tirth8205 merged 35 commits into
stagingfrom
train/2026-10-15

Conversation

@tirth8205

Copy link
Copy Markdown
Owner

This merges 8 tested PRs and one test fix into staging in a single CI run.

Each PR was merged with the current staging, tested end to end, and checked by a second pass that tried to break it. Extra edge-case tests were added on each PR branch.

Included:

Merge conflicts: one, in CHANGELOG.md (#1096 and #1071 both added an Unreleased entry). Both entries are kept.

Local checks on the combined branch: full test suite with -m "not browser and not upgrade" (4,974 passed, 0 failed), ruff, mypy and bandit all pass.

Merge with a merge commit, so each included PR is marked as merged.

🤖 Generated with Claude Code

alex-feel and others added 30 commits September 28, 2026 01:46
An explicit embedding refresh (provider and model supplied) on a graph that was never embedded returned before provider resolution and left no trace in the result, so it looked the same as a build that never asked for a refresh.

build, update and postprocess now set embeddings_refresh_skipped and add a warning that names the embed command.
An update that finds no changes returns before post-processing, so it checks for an index directly and reports the same notice.
No model is loaded and no provider is contacted in either case; has_embeddings() is the check refresh_embeddings() already made, now shared.

watch fails closed on post-processing warnings, so the notice would have ended it on the first update.
The watch boundary exempts that one notice: watch logs it and keeps running, and every other warning, including a refresh that fails against an existing index, still fails the update.
With deferred tool loading an agent sees the tool names and the server
instructions, not the tool descriptions, while Grep's description is always in
context. "Token-efficient, context-aware code reviews" was the only thing it
knew about the graph, so outside a review it searched callers, importers and
covering tests with Grep. The instructions now map each relationship question
to its tool and leave plain-text search to Grep.

tests/test_server_instructions.py keeps every tool and query pattern the text
names registered, and the text under Claude Code's 2,048-character cut.
TypeScript interfaces and type aliases are stored as Class nodes with no runtime body, so a changed interface was reported as an untested gap. The parser now records the declaration kind in extra, and the gap filter skips it. Interfaces and type aliases are still stored and still appear in other analyses. Graphs built before this change keep the old behaviour until they are rebuilt.

Refs #1109
- dispatch names must be concrete, visible node kinds: rejects supertypes
  (python "expression"), hidden rules, anonymous keywords, and the empty
  string, which id_for_node_kind maps to ERROR's id
- checker self-test over known-good and known-bad kinds, including the five
  names from issue #986
- every dispatch-table key is a detected, loadable language; no list repeats
  a name
- PowerShell commands (top-level, in a function body, in a script-block
  argument; unicode and spaces in the path) reach call extraction
- 2,000-command script dispatches every command
- pin _CALL_TYPES["powershell"] == ["command"]
- C# record struct, Objective-C category, Swift struct and Ruby receiver
  calls are still extracted after the unreal names were removed

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- every removal subset of object members and array elements (sizes 1-5,
  compact and indented), adjacent or not, parses as strict JSON
- commented JSONC: all comments survive and no new trailing comma appears
- empty path list returns the document unchanged
- a surviving member keeps its own JSONC trailing comma; a removed tail
  that carried one still leaves strict JSON
- duplicate paths, parent+child overlapping paths, removals in several
  containers at once, nested hook groups emptied at the end
- unicode, escaped quotes and Windows paths before the edit point; CRLF
- large documents (300 members/elements)
- unresolvable paths still raise KeyError/IndexError/ValueError
- install -> uninstall leaves strict JSON with user hooks for Claude,
  Qoder, CodeBuddy, Codex, Gemini (own and shared events) and Cursor
- Continue duplicate registrations at the end of mcpServers
- hooks written by an earlier release, owned hooks inside a user group
- second uninstall is byte-stable; install/uninstall cycles; dry run
- the issue #1068 reproduction through the CLI

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cases added to tests/test_storybook_dead_code.py:
- story exports in a graph built before #1085 (is_test = 0) are not reported
- CALLS, IMPORTS_FROM and REFERENCES from .stories.tsx/.js each alone do not
  keep a component alive; the same edge from application code does
- TESTED_BY from a story (qualified and bare-name source) is not test
  evidence; TESTED_BY from Foo.test.tsx still is
- class-qualified (Widget::label) story calls; story vs production subclass
- Windows backslash paths in legacy rows
- 2000 story member-call rows plus one production row
- story call to a base method must not keep an override alive (fails: the
  polymorphic-dispatch query still counts story CALLS)
- story-only type annotation must not keep a type alive (fails:
  _collect_type_referenced_names reads story functions)
- full build + incremental update on real files under a unicode, spaced path;
  idempotent re-run
- story exports are not reported as test gaps by analyze_changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Simulate Windows text-mode newline translation in-process (newline="\r\n"
on every text write via io.open, builtins.open and Path.open) and check the
written artifact, since POSIX runners cannot tell a text write from a byte
write and the windows-native CI job does not run this module:

- the simulation itself translates (the issue's 3000 vs 2000 bytes)
- 59,999 / 60,000 / 60,001-byte bodies stay within the 60,000-byte cap
- a 59,999-byte body is written whole as exactly 60,000 bytes
- truncated multibyte (UTF-8) reports fit the cap and stay valid UTF-8
- analysis, non-ASCII, legacy report shape, empty report, CRLF
  "No changes detected." input and not-analyzed (exit 4) bodies all land
  on disk as exactly the measured UTF-8 bytes
- the risk-gate breach (exit 3) still writes the exact artifact
- output paths with spaces and non-ASCII characters
- rerunning into a larger existing file replaces it exactly (idempotent)
- unwritable output (missing directory, path is a directory) returns 2

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- incremental update that re-parses files reports the skip, including a
  repo path with spaces and non-ASCII characters
- repeated build/update requests report every time and never create the
  embeddings table
- no-change update reports the skip at postprocess none/minimal/full
  (guards the #1105 no-op rewrite in tools/build.py)
- interrupted-build repair reports the skip
- postprocess with an empty embeddings table keeps status ok
- skip, then embed, then refresh reports embeddings_refreshed and no notice
- has_embeddings: no table, empty table, current and legacy rows
- legacy index without provider identity fails the refresh and stops watch;
  an empty legacy table is a skip
- watch exempts only the exact notice (duplicates ok; half-pair, altered
  text still fail); None/non-dict results are ignored
- real MCP tools and CLI build/update/postprocess/--quiet report the
  notice, exit cleanly and never resolve a provider

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- instructions reach the client in the real stdio initialize handshake
- serve --tools / CRG_TOOLS allow-list (README example) must not leave
  instructions naming removed tools (fails on this PR: names 5 removed tools)
- every name=value argument on a routing line is a parameter of a tool on
  that line (query_graph_tool takes `pattern`)
- every alternative after pattern= (a / b / c, comma lists) is a real
  query pattern, not only the first two
- callers_of and get_impact_radius reach a consumer through an
  `export * from` barrel, the claim the instructions make
- importers_of misses a barrel re-exporter (strict xfail, pre-existing
  graph limitation the new text routes to)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pin the single-pass dead-guard scan for header classification:
- issue #977 MemMap generator header (LF and CRLF) parses with no
  per-node ancestor walk and stays a single C File node
- the header scan checks each parent-child edge once (linear work)
- #elif 0, #else of #if 0, #elif after #if 0, #else of #elif 0
- guards nested inside #if 0 stay dead; #if 0 nested in a live guard
- dead state does not leak to following siblings
- unicode comments, CRLF, empty, whitespace-only, comment-only headers
- deepest branch of a 150-branch chain: live, dead, and #else under a
  dead #elif 0 chain
- single-walk _is_in_static_dead_guard matches the previous two-walk
  result on every node of Go, TS, JS, C and C++ sources
- other callers: C++ callable declarations and C CALLS under #if 0
- full build, incremental update to C++ and back, update run twice

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- TSX shape from #1109: props/type tagged, component and React class not
- .mts, .cts and .d.ts files
- export default, namespace, declare global/module, declare, generic,
  method-signature, function-type and union declarations
- unicode declaration names
- enums and classes (runtime) stay untagged; JS, Java and C# not tagged
- Vue and Svelte <script lang="ts"> declarations keep the tag
- full_build -> analyze_changes in a path with spaces and non-ASCII:
  runtime gaps kept, declarations still in changed_functions with a risk
  score, summary counts consistent
- whole-file fallback and an interface-only change
- graph from an earlier release: stale until reparsed, healed by
  incremental_update, idempotent on repeat update and full rebuild
- class of the same name in another file; class/interface declaration
  merging keeps method gaps
- NULL extra on an old row; 400 declarations in one file

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- PowerShell pipeline stages and class-method commands reach call extraction
- .psm1 module files take the same call dispatch
- a half-written script still parses and dispatches recovered commands
- an Objective-C category declared with @interface only is still a Class
- drop the pin on _CALL_TYPES["powershell"], which only copied the source
- rename the 2,000-command test; it checks coverage, not timing

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rewrite the allow-list handshake test: with serve --tools / CRG_TOOLS the
server lists exactly the allowed tools and keeps the routing lines that
point at them. The text naming removed tools matches what prompts and
next_tool_suggestions already do on staging, so it is not a merge blocker.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…cle test gates

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

code-review-graph review

Overall risk: 0.65 (MEDIUM) — 281 changed function(s)/class(es), 68 affected flow(s), 8 test gap(s) (4 with no tested caller found, 4 reached only through a caller)

Risk-scored changes

Risk Level Symbol Location Tested
0.65 medium code_review_graph/tools/build.py::_report_refresh_skipped code_review_graph/tools/build.py:79 indirect
0.60 medium code_review_graph/embeddings.py::has_embeddings code_review_graph/embeddings.py:1327 yes
0.60 medium code_review_graph/parser.py::CodeParser._has_cpp_header_evidence code_review_graph/parser.py:3390 yes
0.60 medium scripts/render_pr_comment.py::main scripts/render_pr_comment.py:548 no
0.55 medium code_review_graph/changes.py::analyze_changes code_review_graph/changes.py:655 yes
0.55 medium code_review_graph/embeddings.py::refresh_embeddings code_review_graph/embeddings.py:1345 yes
0.55 medium code_review_graph/incremental.py::_raise_watch_postprocess_warnings code_review_graph/incremental.py:2187 yes
0.55 medium code_review_graph/jsonc.py::remove_paths code_review_graph/jsonc.py:288 yes
0.55 medium code_review_graph/parser.py::_is_in_static_dead_guard code_review_graph/parser.py:464 yes
0.55 medium code_review_graph/parser.py::CodeParser code_review_graph/parser.py:2942 yes

Affected execution flows

  • process — criticality 0.74, 538 node(s) across 14 file(s)
  • _run — criticality 0.74, 565 node(s) across 16 file(s)
  • build_or_update_graph_tool — criticality 0.73, 619 node(s) across 24 file(s)
  • main — criticality 0.72, 925 node(s) across 40 file(s)
  • run_token_benchmark — criticality 0.70, 38 node(s) across 5 file(s)
  • ...and 63 more affected flow(s)

Test gaps

  • code_review_graph/main.py::build_or_update_graph_tool (code_review_graph/main.py:249)
  • code_review_graph/main.py::run_postprocess_tool (code_review_graph/main.py:306)
  • code_review_graph/parser.py::CodeParser._extract_classes (code_review_graph/parser.py:11635)
  • scripts/render_pr_comment.py::main (scripts/render_pr_comment.py:548)

Reached only through a caller

No test names these directly, but a tested caller reaches them along the call path shown. That is a path in the call graph, not a record of execution -- the caller's tests may never take this branch. Treat it as where to look, not as coverage: these count as gaps and are scored as untested.

  • code_review_graph/parser.py::_is_static_dead_guard_child (code_review_graph/parser.py:433) — via code_review_graph/parser.py::CodeParser._has_cpp_header_evidence, 1 hop(s)
  • code_review_graph/postprocessing.py::_refresh_embeddings (code_review_graph/postprocessing.py:200) — via code_review_graph/postprocessing.py::run_post_processing, 1 hop(s)
  • code_review_graph/tools/build.py::_run_embedding_refresh (code_review_graph/tools/build.py:47) — via code_review_graph/tools/build.py::_run_postprocess, 1 hop(s)
  • code_review_graph/tools/build.py::_report_refresh_skipped (code_review_graph/tools/build.py:79) — via code_review_graph/tools/build.py::build_or_update_graph, 1 hop(s)

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

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.

7 participants