Skip to content

fix: match lowercase relationship types returned by API#19

Merged
jonathanpopham merged 1 commit intosupermodeltools:mainfrom
jonathanpopham:fix/lowercase-rel-types-v2
Apr 2, 2026
Merged

fix: match lowercase relationship types returned by API#19
jonathanpopham merged 1 commit intosupermodeltools:mainfrom
jonathanpopham:fix/lowercase-rel-types-v2

Conversation

@jonathanpopham
Copy link
Copy Markdown
Contributor

@jonathanpopham jonathanpopham commented Apr 2, 2026

Summary

The API returns relationship types in lowercase (imports, calls, defines_function) but handlers compared against uppercase (IMPORTS, CALLS, DEFINES_FUNCTION). Since Go string comparison is case-sensitive, every check silently failed.

What broke

  • dead-code: thought nothing calls anything → reported every function as dead (1,337 vs correct 717)
  • blast-radius: thought nothing imports anything → always returned 0 affected files
  • focus: couldn't find function definitions or call chains → returned empty context

Changes

10 string literals upppercase → lowercase across 7 files. Nothing else.

Test results (before → after)

dead-code:     1,337 → 717 unreachable functions
blast-radius:  0 → 25 affected files (AuthConstants.java)
focus:         ~12 empty tokens → 5 function definitions

Unit tests: all pass. Integration test: pass.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed relationship type matching in dead code detection, blast radius analysis, code focus analysis, and architecture validation to correctly identify code dependencies and affected files.
    • Improvements ensure accurate analysis results across all code scanning features.
  • Tests

    • Updated test fixtures to align with corrected relationship type handling.

The API returns lowercase types (imports, calls, defines_function)
but handlers compared against uppercase. This silently broke
dead-code, blast-radius, and focus commands.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 2, 2026

Walkthrough

A systematic normalization of relationship type identifiers across the codebase, changing string comparisons from uppercase to lowercase formats (e.g., "IMPORTS""imports", "CALLS""calls") in handlers for blast radius analysis, dead code detection, focus operations, MCP server, and architecture checks.

Changes

Cohort / File(s) Summary
Import relationship normalization
internal/blastradius/handler.go, internal/blastradius/handler_test.go, internal/mcp/server.go, scripts/check-architecture/main.go
Updated relationship type matching to use lowercase "imports" and "wildcard_imports" instead of uppercase variants.
Call relationship normalization
internal/deadcode/handler.go, internal/deadcode/handler_test.go, internal/mcp/server.go, internal/focus/handler.go
Updated relationship type matching to use lowercase "calls" and "contains_call" instead of uppercase variants.
Function definition relationship normalization
internal/focus/handler.go
Updated relationship type matching to use lowercase "defines_function" and "defines" instead of uppercase variants.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

Uppercase falls to lowercase grace,
Constants normalized in place,
From CALLS to calls, IMPORTS to imports,
The codebase finds consistent ports,
One pattern spreads through every line—
Refactoring, orderly and fine. 📝✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: converting uppercase relationship type strings to lowercase to match the API's actual output.
Description check ✅ Passed The description covers all template sections with clear explanations: What changed (uppercase to lowercase), Why it matters (critical bugs in dead-code, blast-radius, focus), and concrete before/after test results.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
internal/focus/handler.go (3)

133-137: ⚠️ Potential issue | 🔴 Critical

Inconsistent: External callers check still uses UPPERCASE.

Hey, looks like this spot got missed! Lines 134 still check for "CALLS" and "CONTAINS_CALL" in uppercase, but the API returns lowercase. This means the "Called by" section in focus output will always be empty.

Think of it this way: you fixed buildCalleesOf (line 228) to use lowercase "calls", but this loop does the same kind of check and still uses uppercase "CALLS". Same relationship type, different casing = one works, one doesn't.

🐛 Proposed fix
 	for _, rel := range rels {
-		if rel.Type != "CALLS" && rel.Type != "CONTAINS_CALL" {
+		if rel.Type != "calls" && rel.Type != "contains_call" {
 			continue
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/focus/handler.go` around lines 133 - 137, The loop iterating over
rels in the focus handler is still comparing rel.Type to uppercase "CALLS" and
"CONTAINS_CALL", but the API returns lowercase; update those comparisons to the
lowercase strings "calls" and "contains_call" (mirror the fix you made in
buildCalleesOf) so the external callers ("Called by") logic that checks
fnIDSet[rel.EndNode] actually matches returned relation types.

181-185: ⚠️ Potential issue | 🔴 Critical

Inconsistent: Import traversal still uses UPPERCASE.

Same deal here - reachableImports checks for "IMPORTS" and "WILDCARD_IMPORTS" in uppercase, but the API returns lowercase. This means the imports BFS won't find any edges, so sl.Imports will always be empty.

Compare this to blastradius/handler.go line 65, which you correctly updated to lowercase "imports" / "wildcard_imports".

🐛 Proposed fix
 		for _, cur := range queue {
 			for _, rel := range rels {
-				if rel.Type != "IMPORTS" && rel.Type != "WILDCARD_IMPORTS" {
+				if rel.Type != "imports" && rel.Type != "wildcard_imports" {
 					continue
 				}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/focus/handler.go` around lines 181 - 185, The loop in
reachableImports is checking rel.Type against uppercase strings ("IMPORTS",
"WILDCARD_IMPORTS") but the API returns lowercase; update the comparisons inside
reachableImports (where rels is iterated and rel.Type is tested) to use
"imports" and "wildcard_imports" (match the change made in
blastradius/handler.go) so the BFS will actually follow import edges (ref:
reachableImports, rels, rel.Type, visited, cur).

237-241: ⚠️ Potential issue | 🟠 Major

Fix uppercase relationship type checks in extractTypes.

The extractTypes function checks for uppercase "DECLARES_CLASS" and "DEFINES", but the API returns these in lowercase like all other relationship types (you can see defines and defines_function working correctly at line 209). This means when someone uses --include-types, sl.Types will always be empty because the relationship type comparisons never match.

Change line 238 from:

if rel.Type != "DECLARES_CLASS" && rel.Type != "DEFINES" {

to:

if rel.Type != "declares_class" && rel.Type != "defines" {

Also worth noting—lines 134 and 182 have the same issue with uppercase "CALLS", "CONTAINS_CALL", "IMPORTS", and "WILDCARD_IMPORTS". Those should be lowercase too to match what the API returns.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/focus/handler.go` around lines 237 - 241, The relationship-type
comparisons in extractTypes are using uppercase strings and thus never match the
API's lowercase types; update the checks in extractTypes (e.g., the condition
comparing rel.Type to "DECLARES_CLASS" and "DEFINES") to use "declares_class"
and "defines". Also find and fix the other uppercase rel.Type comparisons in
this file (the checks against "CALLS", "CONTAINS_CALL", "IMPORTS", and
"WILDCARD_IMPORTS") to their lowercase forms ("calls", "contains_call",
"imports", "wildcard_imports") so include-type and call/import handling works
correctly. Ensure you modify the exact rel.Type comparisons in the same
functions where those symbols appear (extractTypes and the call/import handling
blocks) without changing other logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@internal/focus/handler.go`:
- Around line 133-137: The loop iterating over rels in the focus handler is
still comparing rel.Type to uppercase "CALLS" and "CONTAINS_CALL", but the API
returns lowercase; update those comparisons to the lowercase strings "calls" and
"contains_call" (mirror the fix you made in buildCalleesOf) so the external
callers ("Called by") logic that checks fnIDSet[rel.EndNode] actually matches
returned relation types.
- Around line 181-185: The loop in reachableImports is checking rel.Type against
uppercase strings ("IMPORTS", "WILDCARD_IMPORTS") but the API returns lowercase;
update the comparisons inside reachableImports (where rels is iterated and
rel.Type is tested) to use "imports" and "wildcard_imports" (match the change
made in blastradius/handler.go) so the BFS will actually follow import edges
(ref: reachableImports, rels, rel.Type, visited, cur).
- Around line 237-241: The relationship-type comparisons in extractTypes are
using uppercase strings and thus never match the API's lowercase types; update
the checks in extractTypes (e.g., the condition comparing rel.Type to
"DECLARES_CLASS" and "DEFINES") to use "declares_class" and "defines". Also find
and fix the other uppercase rel.Type comparisons in this file (the checks
against "CALLS", "CONTAINS_CALL", "IMPORTS", and "WILDCARD_IMPORTS") to their
lowercase forms ("calls", "contains_call", "imports", "wildcard_imports") so
include-type and call/import handling works correctly. Ensure you modify the
exact rel.Type comparisons in the same functions where those symbols appear
(extractTypes and the call/import handling blocks) without changing other logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1e2bb0b2-f761-400d-8a34-9097942f8cf5

📥 Commits

Reviewing files that changed from the base of the PR and between 53b4918 and 14dca92.

📒 Files selected for processing (7)
  • internal/blastradius/handler.go
  • internal/blastradius/handler_test.go
  • internal/deadcode/handler.go
  • internal/deadcode/handler_test.go
  • internal/focus/handler.go
  • internal/mcp/server.go
  • scripts/check-architecture/main.go

@jonathanpopham
Copy link
Copy Markdown
Contributor Author

Before / After (tested against supermodel-public-api, same cached graph)

main branch (before)

dead-code:     1,337 unreachable function(s) found.
blast-radius:  No files are affected by changes to AuthConstants.java.
focus:         <!-- approx 12 tokens -->

This PR (after)

dead-code:     717 unreachable function(s) found.
blast-radius:  25 file(s) affected by changes to AuthConstants.java.
focus:         5 functions defined, <!-- approx 34 tokens -->

Unit tests: all pass. Integration test (TestIntegration_Analyze): pass.

@jonathanpopham
Copy link
Copy Markdown
Contributor Author

Validation: CLI vs raw API data

Computed dead code, blast radius, and focus independently from the raw API response JSON (same graph, same repo) using Python — no CLI involved. Results match exactly.

Dead code

Source Count
Raw API (Python) 717
CLI (this PR) 717
CLI (main) 1,337 ← broken, never matched any calls edges

Breakdown: 1,527 Function nodes total, 645 have incoming calls edges, 882 have none. After filtering entry points (main, init, Test*, exports), 717 remain.

Blast radius — AuthConstants.java

Source Affected files
Raw API (Python) 25 (11 at hop 1, 12 at hop 2, 2 at hop 3)
CLI (this PR) 25 (same files, same hops)
CLI (main) 0 ← broken, never matched any imports edges

Focus — codegraph.service.ts

Source Functions found
Raw API (Python) 5 (DeferredImportData, DeferredInheritanceData, PathAliasEntry, TsconfigAliasConfig, stripJsonComments)
CLI (this PR) 5 (same)
CLI (main) 0 ← broken, never matched any defines_function edges

@jonathanpopham jonathanpopham marked this pull request as ready for review April 2, 2026 18:23
@jonathanpopham jonathanpopham merged commit 2281ea3 into supermodeltools:main Apr 2, 2026
8 checks passed
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