refactor: improve TS code quality across modules

[carlos-alm]

Summary

• Split presentation formatters (manifesto, communities, inspect) into focused sub-renderer functions
• Extract watcher debounce and journal logic from watchProject (cog=59, bugs=1.30)
• Reduce complexity in TS extractors (javascript.ts, go.ts) and file-utils by splitting handlers
• Simplify AST store visitor dispatch and engine setup helpers

Titan Audit Context

• Phase: quality_fix
• Domain: presentation, domain-core, extractors, ast-analysis
• Commits: 4
• Depends on: none

Changes

• src/presentation/manifesto.ts -- split into sub-renderers
• src/presentation/communities.ts -- split into sub-renderers
• src/presentation/queries-cli/inspect.ts -- split into sub-renderers
• src/domain/graph/watcher.ts -- extract debounce and journal logic
• src/extractors/javascript.ts -- split dispatchQueryMatch into sub-handlers
• src/extractors/go.ts -- split inferShortVarType into sub-handlers
• src/shared/file-utils.ts -- split extractSummary into sub-functions
• src/ast-analysis/visitors/ast-store-visitor.ts -- lookup table for dispatch
• src/ast-analysis/engine.ts -- extract per-analysis setup helpers

Metrics Impact

• watchProject: bugs 1.30 -> 0.12, cog 59 -> 4, MI 38.9 -> 57.9
• manifesto, communities, inspect: reduced cognitive complexity
• createAstStoreVisitor: cog 35 -> 34 (minor, lookup table approach)

Test plan

• CI passes (lint + build + tests)
• Presentation output formatting unchanged
• Watcher correctly detects and rebuilds on file changes
• Parser extraction output unchanged
• AST analysis produces same complexity/CFG/dataflow results

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review: TypeScript Quality Improvements

Reviewing PR #847 following comprehensive 5-phase analysis:

Todo List

• Phase 1: Root Cause Analysis - understand problem being solved
• Phase 2: Configuration Change Detection - check for rule relaxations
• Phase 3: Test Integrity - verify no weakened assertions
• Phase 4: Code Quality Assessment - review 9 changed files
• Phase 5: Backlog Compliance Grading - rate against criteria
• Final Assessment: Provide rating and recommendation

Phase 2: Configuration Change Detection ✅

No Configuration Changes: No biome.json, tsconfig.json, or package.json changes detected. No linting rules were relaxed or disabled. No TODO/FIXME comments indicating workarounds.

Phase 3: Test Integrity ✅

No Test Modifications: Zero test files were modified in this PR. Only formatting changes found (whitespace/indentation adjustments). No test expectations were weakened or removed.

Phase 4: Code Quality Assessment ✅

Excellent Refactoring Quality - Each file shows clear improvements:

1. src/domain/graph/watcher.ts (+112/-88)

• ✅ Extracted complex watchProject function into focused helpers
• ✅ setupWatcher() - clean initialization separation
• ✅ scheduleDebouncedProcess() - debounce logic isolated
• ✅ processPendingFiles() - core rebuild logic extracted
• ✅ writeJournalAndChangeEvents() - I/O concerns separated
• 🔥 Impact: Cognitive complexity 59→4, bugs 1.30→0.12, MI 38.9→57.9

2. src/presentation/manifesto.ts (+35/-38)

• ✅ Split monolithic renderer into renderRulesTable(), renderViolationList(), renderViolations()
• ✅ Each function has single responsibility
• ✅ Improved testability and readability

3. src/presentation/communities.ts (+44/-39)

• ✅ Extracted renderCommunityList(), renderDriftAnalysis() helpers
• ✅ Clean separation of display concerns
• ✅ Consistent with other presentation module patterns

4. src/presentation/queries-cli/inspect.ts (+48/-46)

• ✅ Massive improvement: Split large renderContextResult() into focused sub-renderers
• ✅ renderSignature(), renderComplexity(), renderSource(), renderCallees(), renderCallers(), etc.
• ✅ Each renderer handles one concern - excellent SRP adherence

5. src/extractors/javascript.ts (+45/-27)

• ✅ dispatchQueryMatch() split into focused handlers per capture type
• ✅ handleFnCapture(), handleClassCapture(), handleMethodCapture(), etc.
• ✅ Much easier to debug and test individual capture types

6. src/extractors/go.ts (+57/-32)

• ✅ inferShortVarType() decomposed into specific inference functions
• ✅ inferCompositeLiteral(), inferAddressOfComposite(), inferFactoryCall()
• ✅ Each function handles one Go type inference pattern

7. src/shared/file-utils.ts (+116/-77)

• ✅ extractSummary() split into focused extraction functions
• ✅ extractSingleLineComment(), findJsdocEndLine(), extractJsdocDescription()
• ✅ Clean separation of comment extraction strategies

8. src/ast-analysis/visitors/ast-store-visitor.ts (+19/-21)

• ✅ Replaced complex conditionals with clean kindHandlers lookup table
• ✅ Each handler function focused on single AST node type
• ✅ resolveNameAndText() dispatch much cleaner

9. src/ast-analysis/engine.ts (+71/-49)

• ✅ Complex setup logic extracted into focused functions
• ✅ setupAstVisitor(), setupComplexityVisitorForFile(), setupCfgVisitorForFile()
• ✅ Each setup function handles one analysis type

Security & Performance: No security issues introduced. Performance unchanged or slightly improved due to better code organization. No new dependencies added.

Maintainability: Significant improvement across all files. Functions are now focused, testable, and follow consistent patterns throughout the codebase.

Phase 5: Backlog Compliance ✅

Zero-dep: ✅ — No new runtime dependencies added. Pure refactoring within existing codebase using tree-sitter/SQLite/existing AST infrastructure.

Foundation-aligned: ✅ — Fully compliant with all principles:

• P1 (always-current): No impact on core functionality
• P4 (zero-cost core): No external dependencies or cost increases
• P5 (embeddable-first): Maintains all programmatic API contracts
• P8 (we are not an agent): Pure internal code organization, no behavior changes

Problem-fit: 3/5 — Indirectly supports the core problem by:

• Reducing complexity makes the codebase easier for AI agents to understand
• Better organized code reduces token waste when agents read large files
• Improved maintainability helps prevent bugs that could cause agent confusion
• However, this is primarily technical debt reduction rather than direct AI assistance improvement

Breaking: No — All public APIs maintained. CLI output format unchanged. No DB schema changes. MCP tool contracts preserved.

Tier: 1 (zero-dep ✅ + foundation-aligned ✅)

---

Root Cause Analysis

Problem Being Solved: Technical debt reduction targeting specific complexity hotspots (watchProject cog=59, bugs=1.30) and general maintainability issues across presentation, extraction, and analysis modules.
Why This Approach: Standard refactoring patterns (Extract Method, SRP, lookup tables) applied systematically across related concerns.
Risk Assessment: Minimal risk - pure refactoring maintaining all interfaces and behaviors.

Backlog Compliance

• Zero-dep: ✅ — no new deps, builds on existing tree-sitter/SQLite/AST
• Foundation-aligned: ✅ — fully aligned, violates no principles
• Problem-fit: 3 — useful but doesn't directly address core "lost AI" problem
• Breaking: No
• Tier: 1

Critical Concerns

None identified. This is exemplary refactoring work.

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: High-quality technical debt reduction that significantly improves code maintainability. The metrics improvements are substantial (watchProject: cog 59→4, bugs 1.30→0.12). All 9 files show clear improvement with no test modifications or config relaxations. While not directly addressing the core AI assistance problem, the improved code organization will benefit future development and AI agent comprehension.

---

[greptile-apps]

Greptile Summary

This PR is a quality-focused refactor that splits monolithic formatter and extractor functions into focused sub-renderers and helper functions across presentation, domain, extractor, and AST-analysis modules. The complexity reductions are real and the extracted logic is faithful to the original in all files except one.

• P1 — src/domain/graph/watcher.ts: The setTimeout callback in scheduleDebouncedProcess is async but processPendingFiles is awaited without a try/catch. On Node ≥ 22.6 (required by this project), any throw from rebuildFile produces an unhandled promise rejection that terminates the process, crashing the watcher on the first rebuild error.

Confidence Score: 4/5

Safe to merge after fixing the unhandled rejection in the watcher debounce callback.

One P1 issue remains: an unhandled async rejection in scheduleDebouncedProcess that can crash the watcher process under Node >= 22.6. All other files are clean refactors with no logic changes. The prior SIGINT concern from the previous review round was resolved.

src/domain/graph/watcher.ts — the scheduleDebouncedProcess setTimeout callback needs a .catch() or try/catch wrapper around processPendingFiles.

Important Files Changed

Filename	Overview
src/domain/graph/watcher.ts	Watcher logic extracted into focused helpers; P1 issue: unhandled async rejection in scheduleDebouncedProcess can crash the watcher process under Node >= 22.6
src/ast-analysis/engine.ts	Per-analysis setup helpers extracted cleanly; logic preserved correctly across setupAstVisitor, setupComplexityVisitorForFile, and setupCfgVisitorForFile
src/ast-analysis/visitors/ast-store-visitor.ts	Dispatch replaced with a kindHandlers lookup table; skip sentinel and defaultResult handle non-matched kinds correctly
src/extractors/go.ts	inferShortVarType cleanly split into inferCompositeLiteral, inferAddressOfComposite, and inferFactoryCall; logic unchanged
src/extractors/javascript.ts	Capture handlers already existed as named functions; dispatchQueryMatch now delegates to them without changing extraction logic
src/presentation/communities.ts	Rendering split into renderCommunityList and renderDriftAnalysis; drift is always rendered with an early-return guard, community list gated on !opts.drift
src/presentation/manifesto.ts	Rules table and violation list extracted into sub-renderers; P2: renderViolationList tag logic relies on a string-literal comparison against the label parameter
src/presentation/queries-cli/inspect.ts	Context/explain rendering split into focused sub-renderers (renderSignature, renderComplexity, renderSource, renderCallees, renderCallers, etc.); no logic changes
src/shared/file-utils.ts	extractSummary decomposed into extractSingleLineComment, findJsdocEndLine, and extractJsdocDescription with a shared truncate helper; behaviour preserved

_{Reviews (2): Last reviewed commit: "fix: address Greptile review feedback (#..." | Re-trigger Greptile}

[greptile-apps]

Consider process.once instead of process.on for SIGINT

process.on accumulates a new listener every time setupShutdownHandler is called. In the current code watchProject is intended to be called once per process, so this is fine in production, but test runners or future callers that invoke watchProject more than once will accumulate handlers and eventually trigger Node's MaxListenersExceededWarning. process.once avoids the leak without any functional change.

Suggested change

	process.on('SIGINT', () => {
	process.once('SIGINT', () => {

[carlos-alm]

Fixed in 03015d9 — changed process.on to process.once as suggested.

[carlos-alm]

Fixed in 03015d9 — changed process.on to process.once as suggested.

[github-actions]

Codegraph Impact Analysis

51 functions changed → 53 callers affected across 19 files

• indexNativeByLine in src/ast-analysis/engine.ts:219 (4 transitive callers)
• matchNativeResult in src/ast-analysis/engine.ts:230 (4 transitive callers)
• storeNativeComplexityResults in src/ast-analysis/engine.ts:240 (3 transitive callers)
• storeNativeCfgResults in src/ast-analysis/engine.ts:298 (3 transitive callers)
• fileNeedsWasmTree in src/ast-analysis/engine.ts:360 (2 transitive callers)
• ensureWasmTreesIfNeeded in src/ast-analysis/engine.ts:397 (1 transitive callers)
• runAnalyses in src/ast-analysis/engine.ts:671 (0 transitive callers)
• createAstStoreVisitor in src/ast-analysis/visitors/ast-store-visitor.ts:78 (6 transitive callers)
• resolveNameAndText in src/ast-analysis/visitors/ast-store-visitor.ts:121 (2 transitive callers)
• collectTrackedFiles in src/domain/graph/watcher.ts:140 (3 transitive callers)
• WatcherContext.rootDir in src/domain/graph/watcher.ts:161 (0 transitive callers)
• WatcherContext.db in src/domain/graph/watcher.ts:162 (0 transitive callers)
• WatcherContext.stmts in src/domain/graph/watcher.ts:163 (0 transitive callers)
• WatcherContext.engineOpts in src/domain/graph/watcher.ts:164 (0 transitive callers)
• WatcherContext.cache in src/domain/graph/watcher.ts:165 (0 transitive callers)
• WatcherContext.pending in src/domain/graph/watcher.ts:166 (0 transitive callers)
• WatcherContext.timer in src/domain/graph/watcher.ts:167 (0 transitive callers)
• WatcherContext.debounceMs in src/domain/graph/watcher.ts:168 (0 transitive callers)
• setupWatcher in src/domain/graph/watcher.ts:172 (2 transitive callers)
• scheduleDebouncedProcess in src/domain/graph/watcher.ts:210 (4 transitive callers)

[carlos-alm]

Both Greptile suggestions addressed in 03015d9:

1. process.once instead of process.on (watcher.ts:283) — Done.
2. ?? → || (engine.ts:693) — Changed to || to better communicate "any method exists" intent.

@greptileai