fix(types): create TriageNodeRow type for triage query results#576
fix(types): create TriageNodeRow type for triage query results#576carlos-alm merged 3 commits intomainfrom
Conversation
…#573) findNodesForTriage returns computed columns (fan_in, cognitive, mi, cyclomatic, max_nesting, churn) that don't exist on the base NodeRow. Add a dedicated TriageNodeRow interface extending NodeRow and plumb it through Repository, SqliteRepository, InMemoryRepository, and the standalone findNodesForTriage function.
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Todo List
Root Cause AnalysisProblem Being Solved: Type safety issue where Why This Approach: The PR properly extends the existing type hierarchy by creating Risk Assessment: Very low risk - purely additive type changes with no runtime behavior modifications. Phase 2: Configuration Changes✅ No configuration changes detected - this PR only adds types and updates method signatures. Phase 3: Test Integrity✅ All tests still pass - PR description confirms 171 existing unit tests pass, and Biome lint passes. No test modifications were needed since this is purely a type-level change. Phase 4: Code Quality AssessmentStrengths:
Technical Review:
No Issues Found:
Phase 5: Backlog ComplianceZero-dep: ✅ No new runtime dependencies added - purely type-level changes to existing interfaces Foundation-aligned: ✅ Fully aligned - this improves type safety for the existing graph query API without violating any principles Problem-fit: 4/5 - While not directly addressing AI context navigation, this enables more precise tooling by ensuring triage result properties are properly typed. This prevents runtime errors when AI tools or other consumers access these computed metrics. Breaking: No - This is purely additive. Existing code using Tier: 1 (zero-dep ✅ + foundation-aligned ✅) Critical ConcernsNone - This is an exemplary type safety improvement that follows all best practices. Final Recommendation
|
Greptile SummaryThis PR introduces Key changes:
The change is purely additive at the type level — Confidence Score: 5/5
Important Files Changed
Class Diagram%%{init: {'theme': 'neutral'}}%%
classDiagram
class NodeRow {
+number id
+string name
+AnyNodeKind kind
+string file
+number line
+number|null end_line
+number|null parent_id
+0|1|null exported
+string|null qualified_name
+string|null scope
+string|null visibility
+Role|null role
}
class NodeRowWithFanIn {
+number fan_in
}
class TriageNodeRow {
+number fan_in
+number cognitive
+number mi
+number cyclomatic
+number max_nesting
+number churn
}
class Repository {
+findNodesForTriage(opts?) TriageNodeRow[]
}
class SqliteRepository {
+findNodesForTriage(opts?) TriageNodeRow[]
}
class InMemoryRepository {
+findNodesForTriage(opts?) TriageNodeRow[]
}
NodeRow <|-- NodeRowWithFanIn
NodeRow <|-- TriageNodeRow
Repository <|-- SqliteRepository
Repository <|-- InMemoryRepository
Repository ..> TriageNodeRow : returns
Reviews (2): Last reviewed commit: "fix(types): include all NodeRow columns ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export interface TriageNodeRow extends NodeRow { | ||
| fan_in: number; | ||
| cognitive: number; | ||
| mi: number; | ||
| cyclomatic: number; | ||
| max_nesting: number; | ||
| churn: number; | ||
| } |
There was a problem hiding this comment.
TriageNodeRow extends NodeRow over-promises available fields
TriageNodeRow extends NodeRow, which tells TypeScript consumers that all NodeRow fields (parent_id, exported, qualified_name, scope, visibility) are present. However, the SQLite-side findNodesForTriage query only selects n.id, n.name, n.kind, n.file, n.line, n.end_line, n.role plus the triage signals — those other NodeRow fields will be undefined at runtime for any SQLite-backed call.
The in-memory path avoids this because it uses { ...n, ... } (spreading a full NodeRow), so that path is fine. But the SQLite path silently violates the structural contract implied by extends NodeRow.
Two ways to fix this:
Option A – extend only the fields the query actually returns, so the type is structurally honest:
/** A node row augmented with triage signals (from findNodesForTriage). */
export interface TriageNodeRow extends NodeRowWithFanIn {
cognitive: number;
mi: number;
cyclomatic: number;
max_nesting: number;
churn: number;
}Then update the SQL SELECT in nodes.ts to also select the remaining NodeRow fields (parent_id, exported, qualified_name, scope, visibility), or alternatively, define a narrower base type with only the fields the query actually returns.
Option B – keep extends NodeRow but update the SQL SELECT to include all NodeRow columns, so the runtime data actually matches the declared type:
n.id, n.name, n.kind, n.file, n.line, n.end_line, n.parent_id,
n.exported, n.qualified_name, n.scope, n.visibility, n.role,
COALESCE(fi.cnt, 0) AS fan_in, ...Right now no consumer of findNodesForTriage accesses the missing fields, so there is no active bug — but the type contract is misleading and could cause a hard-to-trace undefined if anyone adds such access in the future.
There was a problem hiding this comment.
Fixed — updated the SQL SELECT in findNodesForTriage to include all NodeRow columns (parent_id, exported, qualified_name, scope, visibility) so the runtime shape matches the TriageNodeRow extends NodeRow type contract (Option B from your suggestion).
The SQL SELECT only returned id, name, kind, file, line, end_line, role but TriageNodeRow extends NodeRow which includes parent_id, exported, qualified_name, scope, and visibility. Add the missing columns so the runtime shape matches the declared type contract.
1 participant
Summary
TriageNodeRowinterface tosrc/types.ts, extendingNodeRowwith the computed columns returned by the triage SQL query:fan_in,cognitive,mi,cyclomatic,max_nesting,churnfindNodesForTriagereturn type fromNodeRow[]toTriageNodeRow[]acrossRepository,SqliteRepository,InMemoryRepository, and the standalone query functionas NodeRowcast inInMemoryRepositorywithas TriageNodeRowCloses #573
Test plan