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

PR Review: feat(config): auto-enable TypeScript compiler resolver for TS projects

• Gather context and read changed files
• Phase 1: Root Cause Analysis
• Phase 2: Configuration Change Detection
• Phase 3: Test Integrity
• Phase 4: Code Quality
• Phase 5: Backlog Compliance
• Final Assessment

Phase 1: Root Cause Analysis

Problem being solved: The TypeScript compiler resolver pass (ts-resolver.ts, introduced in #1278) was gated behind typescriptResolver: false (opt-in). TS project owners had to explicitly discover and enable it — which is poor UX for a feature that is the highest-ROI resolution improvement for TS projects.

Why this approach: The ts-resolver is already designed for silent opt-out: it lazy-imports typescript, silently skips if the package is unavailable, and silently skips if no tsconfig.json is found (src/domain/graph/resolver/ts-resolver.ts). Flipping the default to true is the minimal safe change — the guard rails already exist.

Risk assessment: Low. The feature is production-hardened (21 unit tests, gated on tsconfig.json presence, lazy-loaded optional import). JS-only projects, environments without typescript installed, and explicit opt-out configs are all unaffected.

Phase 2: Configuration Change Detection

The single changed default is typescriptResolver: false → true at src/infrastructure/config.ts:27. This is a behavioral change, not a silencing of warnings or a relaxation of quality rules:

• No linting rules disabled
• No test timeouts widened
• No coverage thresholds lowered
• No strict mode flags disabled

The change is exactly one meaningful boolean flip — within-scope for a single PR.

Phase 3: Test Integrity ✓

The regression-guard.test.ts change is a comment-only update (line 244): "gated behind \typescriptResolver: false` (default)"→"gated behind `typescriptResolver: false` (was the default at the time)"`. Zero functional test logic is changed — no thresholds, no assertions, no exemptions added.

No assertions were weakened. The KNOWN_REGRESSIONS set is unchanged.

One concern: the PR test plan mentions npx vitest run tests/unit/ts-resolver.test.ts but no evidence that this runs in CI automatically on this PR. The test file exists at tests/unit/ts-resolver.test.ts. Since npm test runs vitest run (which discovers all tests), this would run automatically — no action needed.

Phase 4: Code Quality

src/infrastructure/config.ts:27 — Clean. typescript is in devDependencies (not dependencies or peerDependencies), so it is not bundled with the published package. The lazy-load guard in ts-resolver.ts (try { _ts = (await import('typescript')).default } catch { _ts = null }) correctly handles the case where typescript is absent at runtime in downstream consumer projects. The skip-if-no-tsconfig logic also handles JS-only monorepos.

Missing test coverage concern: There is no integration test that explicitly verifies the new default fires on a TS project without a .codegraphrc.json. The existing 21 unit tests in ts-resolver.test.ts test the resolver logic in isolation. A failing test (e.g., a build integration test that asserts typescriptResolver is not called by default) would reveal unexpected side-effects of this flip — worth adding, but not a blocker given the safety of the guard rails.

Performance concern: Enabling this by default will increase build time for all TS projects. The regression-guard exemptions for 3.12.0:Full build and 3.12.0:1-file rebuild are already documented as post-Phase-8.x feature cost in the test file comment. The PR does not update benchmark data, so the actual cost delta of enabling by default is unmeasured. This is a follow-up concern, not a blocker.

Phase 5: Backlog Compliance

• Zero-dep: ✓ — No new dependencies entry added. typescript is in devDependencies and lazy-loaded at runtime; consumer projects already have it or it silently skips.
• Foundation-aligned: ✓ — Pure local analysis, no cloud calls, no new infrastructure, no file edits or agent behavior. Improves resolution accuracy directly from existing AST+compiler data. Aligns with P1 (always-current: runs from local typescript installation) and P4 (zero-cost: already-paid dep).
• Problem-fit: 5 — This directly reduces hallucinated dependencies for AI agents by upgrading this.field.method() dispatch from heuristic (uncertain) to compiler-verified (confidence 1.0). This is the highest-ROI resolution improvement mentioned in ROADMAP.md §8.1, and making it the default means agents get accurate type-resolved call edges without any configuration ceremony.
• Breaking: Soft behavioral change — builds of TS projects will now produce more edges (compiler-verified method dispatch, typed-parameter call edges, return-type propagation). Downstream consumers relying on a specific edge count or specific absence of type-resolved edges could be surprised. This is the intended improvement, but it could surface in snapshot tests or assertion-based integration tests in user codebases. Not an API/CLI/MCP contract break, so no major version bump is required — but worth noting in release notes.
• Tier: 1 (zero-dep ✓, foundation-aligned ✓)

Critical Concerns

1. typescript in devDependencies, not peerDependencies: At runtime, when @optave/codegraph is installed as an npm package (not in development), typescript will not be present unless the consumer project has it installed separately. The lazy-load guard handles this correctly — the pass silently skips. But users who install codegraph globally or in a dedicated tooling environment (no typescript in scope) will silently get no benefit despite typescriptResolver: true being the default. This is acceptable behavior (documented by the lazy-load fallback), but the README update could be clearer: it says "when typescript is installed" which is accurate.

2. No updated benchmark data: The PR doesn't include a dev benchmark entry showing the actual wall-clock cost of enabling the resolver by default. Given the existing 3.12.0 exemptions, this is likely fine for the current release, but the next benchmark run will capture the combined cost of Phase 8.x + this default flip.

Final Assessment

## Root Cause Analysis
Problem Being Solved: TS projects got no benefit from the compiler resolver pass unless
  users explicitly added `"typescriptResolver": true` to their config — poor discoverability
  for the highest-ROI resolution feature for TS codebases.
Why This Approach: Minimal-change; all opt-out safety guards (lazy-load, tsconfig check)
  already exist in ts-resolver.ts. No new risk introduced.
Risk Assessment: Low. JS-only projects and envs without typescript are provably unaffected.

## Backlog Compliance
- Zero-dep: ✓ — typescript is a devDep, lazy-loaded, not bundled
- Foundation-aligned: ✓ — local-only, no cloud, no new infrastructure
- Problem-fit: 5 — directly improves agent accuracy on TS codebases (most common target)
- Breaking: Soft behavioral — more edges produced for TS projects; no API/CLI/MCP break
- Tier: 1

## Critical Concerns
None blocking. Minor: no updated benchmark data for the default-on cost. No integration
test explicitly exercises the new default path end-to-end.

## Final Recommendation
- Rating: ⭐⭐⭐⭐ (4/5)
- Action: APPROVE
- Reasoning: Single-line config change with well-tested, well-guarded implementation.
  The feature is production-ready (21 unit tests, lazy-load, tsconfig guard). Docs and
  regression-guard comment updated correctly. Problem-fit is 5 — this is the right
  default for the tool's primary use case. Withholding the 5th star: benchmark impact
  of the new default is unmeasured, and there's no integration test asserting the new
  default fires correctly end-to-end.

Greptile Summary

This PR flips the typescriptResolver default from false to true in src/infrastructure/config.ts, auto-enabling the TypeScript compiler API pass for any project where typescript is installed and a tsconfig.json is present. Documentation in README.md and ROADMAP.md is updated to match, and a historical comment in the regression-guard test is amended for accuracy.

• src/infrastructure/config.ts: One-line change — typescriptResolver: false → true; the resolver pass already lazy-loads typescript and silently no-ops on JS-only projects or missing tsconfig.json, so no new failure modes are introduced.
• tests/benchmarks/regression-guard.test.ts: Comment-only clarification noting the old default for historical context; no guard logic changed.
• README.md / ROADMAP.md: Limitations section and roadmap bullet updated to describe auto-enable behavior and document the opt-out config key.

Confidence Score: 5/5

Safe to merge — the change is a single boolean flip behind an already-battle-tested lazy-load guard that silently no-ops on JS-only projects and missing tsconfig.json environments.

The code change is minimal and low-risk: one constant value flipped, no new code paths, no schema changes, and the resolver pass has its own safety net (lazy-loads typescript, skips when tsconfig.json is absent). Documentation and comments are consistent with the new behavior.

No files require special attention. The benchmark regression risk (noted in the existing outside-diff comment) is the only follow-up item and is already tracked.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[loadConfig called] --> B{.codegraphrc.json found?}
    B -- Yes --> C[mergeConfig DEFAULTS + user config]
    B -- No --> D[Use DEFAULTS as-is]
    C --> E{user set typescriptResolver?}
    E -- Yes --> F[Use user value true/false]
    E -- No --> G[Default: true ← changed in this PR]
    D --> G
    F --> H{typescriptResolver === true?}
    G --> H
    H -- No --> I[Skip TS compiler pass]
    H -- Yes --> J{typescript installed AND tsconfig.json found?}
    J -- No --> K[Silently skip pass]
    J -- Yes --> L[Run ts.createProgram + type-checker enrichment]
    L --> M[Confidence 1.0 call edges written to graph]

Reviews (3): Last reviewed commit: "Merge branch 'main' into feat/auto-enabl..." | Re-trigger Greptile