feat: add core TypeScript type definitions (src/types.ts)#516
feat: add core TypeScript type definitions (src/types.ts)#516carlos-alm merged 4 commits intomainfrom
Conversation
…odel New file src/types.ts defines 153 exported types and interfaces covering the entire codegraph domain: symbol/edge kind enumerations, database row shapes, Repository contract, extractor output types, parser/language registry, import resolution, AST visitor system, analysis engine, CodeGraph model, build pipeline context, configuration, pagination, error codes, all feature result shapes (audit, complexity, diff-impact, triage, communities, check, batch, flow, manifesto, exports, path, stats, module-map, roles), registry, MCP server, CLI command interface, risk scoring, sequence diagrams, and graph export formats.
|
Claude finished @carlos-alm's task —— View job PR Review: Core TypeScript Type DefinitionsConducting comprehensive review of the TypeScript type definitions PR following the 5-phase framework. Review Progress
Root Cause AnalysisProblem Being Solved: This PR creates comprehensive TypeScript type definitions ( Why This Approach: The approach is sensible — creating types upfront establishes the contract before migrating individual modules. This prevents type drift and provides a clear target for each module migration. Risk Assessment: Low risk. This is purely additive type definitions with no runtime impact. Configuration Change Detection ✅No high-risk configuration changes detected:
Test Integrity ✅No test integrity concerns:
Code Quality ReviewStrengths
Specific Analysis§1 Kind Enumerations: Types match runtime constants in §2 Database Row Shapes: Field types appear accurate based on SQLite schema and repository queries. Boolean flags correctly typed as §3 Repository Interface: Comprehensive interface covering all database operations. Method signatures align with actual §4-20 Additional Sections: Extensive coverage of extractors, parser registry, import resolution, AST analysis, graph model, build pipeline, configuration, and all feature result shapes. Minor Concerns
Backlog Compliance AssessmentZero-dep: ✓ — No new runtime dependencies. Pure TypeScript definitions. Foundation-aligned: ✓ — Fully aligned with core principles:
Problem-fit: 3/5 — Useful for developers and future TypeScript migration but doesn't directly address the core "AI agents wasting tokens" problem. This is foundation work that enables better tooling but isn't immediately visible to agents. Breaking: No — Purely additive. No existing behavior changes. Tier: Tier 1 — Zero-dep ✓ + Foundation-aligned ✓ Critical ConcernsNo critical concerns identified. This is a well-executed foundation piece. Final Recommendation
Recommended Next Steps
|
Greptile SummaryThis PR introduces The file is comprehensive and well-organized. Several issues from the previous review round have been addressed (edge kind narrowing on
Confidence Score: 3/5
Important Files Changed
Class Diagram%%{init: {'theme': 'neutral'}}%%
classDiagram
class CoreSymbolKind {
<<type union>>
'function' | 'method' | 'class'
'interface' | 'type' | 'struct'
'enum' | 'trait' | 'record' | 'module'
}
class ExtendedSymbolKind {
<<type union>>
'parameter' | 'property'
'constant' | 'method' ⚠️ overlap
}
class SymbolKind {
<<type union>>
CoreSymbolKind | ExtendedSymbolKind
}
class AnyNodeKind {
<<type union>>
SymbolKind | FileNodeKind
}
class SubDeclaration {
name: string
kind: 'parameter'|'property'|'constant'|'method'
line: number
}
class Definition {
name: string
kind: SymbolKind
line: number
children?: SubDeclaration[]
}
class NodeRow {
id: number
kind: AnyNodeKind
file: string
role: Role | null
}
class EdgeRow {
id: number
kind: EdgeKind
confidence: number | null
}
class ComplexityMetrics {
cognitive: number
cyclomatic: number
maintainability_index: number | null
}
class ComplexityEntry {
cognitive: number
cyclomatic: number
maintainabilityIndex: number ⚠️ non-nullable
halstead: HalsteadMetrics ⚠️ non-nullable
}
class Repository {
<<interface>>
findNodeById()
findCallees()
getComplexityForNode()
}
CoreSymbolKind --> SymbolKind
ExtendedSymbolKind --> SymbolKind
SymbolKind --> AnyNodeKind
AnyNodeKind --> NodeRow
Definition --> SubDeclaration
ExtendedSymbolKind ..> SubDeclaration : should reference
ComplexityMetrics ..> ComplexityEntry : maps to (nullability mismatch)
NodeRow --> Repository
EdgeRow --> Repository
Last reviewed commit: "fix: narrow edge kin..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/types.ts
Outdated
| kind: AnyEdgeKind; | ||
| confidence: number | null; | ||
| dynamic: 0 | 1; | ||
| } |
There was a problem hiding this comment.
EdgeRow.kind is too wide — includes dataflow edge kinds that never appear in the edges table
DataflowEdgeKind values (flows_to, returns, mutates) are stored in a separate dataflow table, not in the main edges table. The runtime code in src/features/dataflow.js inserts these using a separate table alias (d vs e) and the edges table only ever holds EdgeKind values (CoreEdgeKind | StructuralEdgeKind).
Using AnyEdgeKind here makes the type wider than what the database actually produces, allowing downstream code to unsoundly pattern-match on flows_to | returns | mutates for an EdgeRow that will never carry those values.
| kind: AnyEdgeKind; | |
| confidence: number | null; | |
| dynamic: 0 | 1; | |
| } | |
| kind: EdgeKind; |
The same applies to AdjacentEdgeRow.edge_kind at line 164 — that field is populated via e.kind AS edge_kind from the edges table and should also be narrowed to EdgeKind.
There was a problem hiding this comment.
Fixed — EdgeRow.kind now uses EdgeKind instead of AnyEdgeKind. Dataflow edge kinds live in a separate table and never appear in the edges table.
src/types.ts
Outdated
| | 'module'; | ||
|
|
||
| /** Sub-declaration kinds (Phase 1). */ | ||
| export type ExtendedSymbolKind = 'parameter' | 'property' | 'constant'; |
There was a problem hiding this comment.
ExtendedSymbolKind and SubDeclaration.kind are misaligned
ExtendedSymbolKind is documented as "Sub-declaration kinds" and only covers 'parameter' | 'property' | 'constant', but SubDeclaration.kind at line 356 also allows 'method'. The actual runtime behaviour confirms this gap: findNodeChildren (in src/db/repository/nodes.js) performs an unfiltered SELECT … FROM nodes WHERE parent_id = ?, meaning nodes with kind = 'method' can and do appear as children of class nodes.
Because SubDeclaration is the TypeScript mirror of "things that appear in a definition's children array", ExtendedSymbolKind is incomplete as currently defined. Any code that accepts ExtendedSymbolKind where a SubDeclaration.kind value is expected will silently reject valid 'method' children.
Consider either:
- Adding
'method'toExtendedSymbolKind(and updating its doc comment), or - Explicitly expressing
SubDeclaration.kindasExtendedSymbolKind | 'method'so the relationship is clear
| export type ExtendedSymbolKind = 'parameter' | 'property' | 'constant'; | |
| export type ExtendedSymbolKind = 'parameter' | 'property' | 'constant' | 'method'; |
There was a problem hiding this comment.
Fixed — added 'method' to ExtendedSymbolKind to match SubDeclaration.kind. findNodeChildren returns method nodes as class children.
| modularity: number; | ||
| drift: { | ||
| splitCandidates: Array<{ directory: string; communityCount: number }>; | ||
| mergeCandidates: unknown[]; | ||
| }; |
There was a problem hiding this comment.
mergeCandidates is typed as unknown[] while splitCandidates has a concrete shape
Both fields are members of the same drift sub-object, but splitCandidates has a well-defined element type { directory: string; communityCount: number } while mergeCandidates provides no type information at all. This asymmetry likely means either the merge candidate shape hasn't been settled yet or it was an oversight.
If merge candidates share the same structure as split candidates, align the types:
| modularity: number; | |
| drift: { | |
| splitCandidates: Array<{ directory: string; communityCount: number }>; | |
| mergeCandidates: unknown[]; | |
| }; | |
| drift: { | |
| splitCandidates: Array<{ directory: string; communityCount: number }>; | |
| mergeCandidates: Array<{ directory: string; communityCount: number }>; | |
| }; |
If the merge candidate shape is genuinely different or still evolving, at minimum add a comment and narrow it to Array<Record<string, unknown>> to signal the intent.
There was a problem hiding this comment.
Fixed — mergeCandidates now has the concrete shape from the runtime: { communityId: number; size: number; directoryCount: number; directories: string[] }.
src/types.ts
Outdated
| edge_kind: AnyEdgeKind; | ||
| } |
There was a problem hiding this comment.
AdjacentEdgeRow.edge_kind should also be EdgeKind, not AnyEdgeKind
As described in the EdgeRow.kind comment above, edge_kind here is sourced from e.kind AS edge_kind in SQL queries against the main edges table (confirmed in src/db/repository/edges.js). The edges table never stores DataflowEdgeKind values — those live in a separate dataflow table.
| edge_kind: AnyEdgeKind; | |
| } | |
| edge_kind: EdgeKind; |
There was a problem hiding this comment.
Fixed — AdjacentEdgeRow.edge_kind and ImportEdgeRow.edge_kind both narrowed to EdgeKind.
| export interface GraphNodeAttrs { | ||
| [key: string]: unknown; | ||
| } | ||
|
|
||
| /** Edge attributes stored in the in-memory graph. */ | ||
| export interface GraphEdgeAttrs { | ||
| [key: string]: unknown; | ||
| } |
There was a problem hiding this comment.
GraphNodeAttrs and GraphEdgeAttrs provide no structural type safety
Both are defined as bare index signatures identical to Record<string, unknown>, meaning the CodeGraph model's node and edge attributes carry zero compile-time shape information. Common attribute keys (e.g. kind, file, name, line for nodes; kind, confidence for edges) could be promoted as optional named fields alongside the catch-all index signature. This would give callers useful intellisense and catch obvious attribute typos without over-constraining the flexible graph model.
There was a problem hiding this comment.
Fixed — GraphNodeAttrs now has optional named fields (label, kind, file, name, line, dbId) and GraphEdgeAttrs has (kind, confidence, weight), both alongside the catch-all index signature. Derived from actual usage in graph/builders/.
- EdgeRow.kind, AdjacentEdgeRow.edge_kind, ImportEdgeRow.edge_kind now use EdgeKind instead of AnyEdgeKind since the edges table never stores dataflow edge kinds (those live in a separate table) - ExtendedSymbolKind now includes 'method' to match SubDeclaration.kind which allows methods as class children - mergeCandidates typed with actual runtime shape instead of unknown[] - GraphNodeAttrs/GraphEdgeAttrs now have common optional named fields alongside the catch-all index signature for better intellisense Impact: 13 functions changed, 0 affected
1 participant
Summary
src/types.tswith 153 exported types and interfaces covering the entire codegraph domain modelTest plan
tsc --noEmit --strict --target ES2022