fix: dogfood v2.4.0 — win32 native binary, embedder crashes, stdout pollution

[carlos-alm]

Summary

Fixes 3 bugs found during dogfooding v2.4.0:

• bug: @optave/codegraph-win32-x64-msvc missing from optionalDependencies #113 — @optave/codegraph-win32-x64-msvc was missing from optionalDependencies, causing Windows installs to fall back to WASM engine. Also pins all platform packages to 2.4.0.
• bug(embedder): extractLeadingComment crashes on out-of-bounds line access #114 — extractLeadingComment in embedder.js crashes with TypeError: Cannot read properties of undefined (reading 'trim') when node line numbers exceed file length (stale graph data).
• bug(embedder): search --json leaks model loading messages to stdout #115 — search --json leaks "Loading embedding model..." and "Model loaded." to stdout, breaking pipe-clean JSON. Rerouted through info() logger (which writes to stderr).

Not fixed: #116 (incremental rebuild drops edges) — requires deeper changes to the incremental rebuild algorithm.

Found during

Dogfooding v2.4.0 — see dogfood report in this PR.

Test plan

• npm install @optave/codegraph on Windows should pull win32 native binary
• codegraph embed --strategy structured on stale graph does not crash
• codegraph search "query" --json 2>/dev/null outputs clean JSON
• All 592 tests pass

[claude]

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

---

PR Review: Dogfood v2.4.0 Bug Fixes

Todo List:

• Read changed files to understand implementation
• Use codegraph to analyze context and impact
• Phase 1: Root Cause Analysis
• Phase 2: Configuration Change Detection
• Phase 3: Test Integrity Assessment
• Phase 4: Code Quality Review
• Provide final rating and recommendation

---

Phase 1: Root Cause Analysis ✅

Problem Being Solved: This PR addresses three legitimate bugs discovered during v2.4.0 dogfooding:

1. bug: @optave/codegraph-win32-x64-msvc missing from optionalDependencies #113 (Windows native binary missing): @optave/codegraph-win32-x64-msvc was absent from optionalDependencies, causing Windows users to fall back to WASM instead of the faster native engine
2. bug(embedder): extractLeadingComment crashes on out-of-bounds line access #114 (Embedder crash): extractLeadingComment function crashes with TypeError: Cannot read properties of undefined (reading 'trim') when node line numbers exceed file length due to stale graph data
3. bug(embedder): search --json leaks model loading messages to stdout #115 (Stdout pollution): Model loading messages leak to stdout instead of stderr, breaking search --json pipe-clean output

Why This Approach: All three fixes target root causes rather than symptoms:

• Missing dependency → add the dependency + pin all versions to 2.4.0 for consistency
• Array bounds issue → add proper bounds checking before accessing array elements
• stdout pollution → route through proper logging infrastructure (info() → stderr)

Risk Assessment: LOW RISK - These are targeted bug fixes with clear justification

Phase 2: Configuration Change Detection ✅

NO HIGH-RISK CONFIG CHANGES DETECTED

• No biome/linting rules modified
• No test timeouts or coverage thresholds changed
• No build configuration changes
• No TypeScript strict mode modifications

Only legitimate dependency additions and version pinning.

Phase 3: Test Integrity Assessment ✅

NO TEST INTEGRITY CONCERNS

• No test files modified
• No assertions removed or weakened
• No test logic changes
• Changes are in production code only

Phase 4: Code Quality Review ✅

package.json Changes

EXCELLENT - The optionalDependencies changes are exactly what they should be:

+    "@optave/codegraph-win32-x64-msvc": "2.4.0"  // Adds missing Windows support
-    "@optave/codegraph-darwin-arm64": "2.3.0",   // Pins to current version
+    "@optave/codegraph-darwin-arm64": "2.4.0",   // for consistency

This ensures platform-specific native binaries are consistently versioned and Windows gets proper native support instead of falling back to WASM.

embedder.js Changes

SOLID DEFENSIVE CODING:

1. Bounds checking fix (lines 138, 141):

   if (fnLineIndex > lines.length) return null;  // Early exit
   if (i >= lines.length) continue;              // Skip invalid indices

   This properly handles stale graph data where line numbers may exceed actual file length.

2. Logging infrastructure fix (lines 303, 326):

   - console.log(`Loading embedding model: ${config.name}...`);
   + info(`Loading embedding model: ${config.name}...`);

   Routes model loading messages through the proper logging system (stderr) instead of polluting stdout, which breaks JSON output piping.

3. Import added correctly (line 7):

   + import { info, warn } from './logger.js';

Code Analysis

• Security: No security concerns - all changes are defensive
• Performance: No performance impact, potentially positive (Windows native vs WASM)
• Maintainability: Improves maintainability by fixing crash conditions
• Best Practices: Follows existing code patterns and error handling

Critical Concerns

NONE IDENTIFIED - This is a well-executed bug fix PR.

Minor Observations

1. Incomplete fix mention: PR description notes bug(builder): incremental rebuild drops edges when re-parsing a file #116 (incremental rebuild drops edges) is not fixed, which is appropriate transparency
2. Good commit message: Detailed explanation with issue references and impact summary
3. Small scope: Only 2 files changed, keeping blast radius minimal

Final Recommendation

## Root Cause Analysis
**Problem Being Solved**: Three legitimate bugs found during v2.4.0 dogfooding - missing Windows binary, embedder crash on stale data, stdout pollution breaking JSON output
**Why This Approach**: Each fix targets the actual root cause with minimal scope and proper defensive coding
**Risk Assessment**: LOW RISK - Targeted fixes with clear justification and no configuration relaxations

## Critical Concerns (if any)
None. This is a well-executed bug fix PR.

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: APPROVE
- Reasoning: Solid engineering - fixes real problems with appropriate solutions, maintains code quality, no test integrity issues, and follows defensive coding practices. Small scope reduces risk. The only reason it's not 5 stars is that it's fixing bugs rather than adding exceptional value.

APPROVE - This PR demonstrates good engineering practices and addresses legitimate issues discovered through dogfooding.

---

[greptile-apps]

Greptile Summary

This PR fixes three bugs discovered during v2.4.0 dogfooding, all with targeted, minimal changes:

#113 — Windows native binary fallback: Added missing @optave/codegraph-win32-x64-msvc to optionalDependencies and pinned all platform packages to 2.4.0. Without this, Windows users fell back to the slower WASM engine.

#114 — Crash on stale graph data: Added bounds checks in extractLeadingComment() at embedder.js:138,141 to prevent TypeError when node line numbers exceed file length due to stale graph data. The guards ensure we never access lines[i] when i >= lines.length.

#115 — JSON output pollution: Changed model loading messages from console.log to info() (embedder.js:303,326), which writes to stderr instead of stdout. This prevents "Loading embedding model..." messages from breaking codegraph search --json output.

All fixes are defensive and backward-compatible.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• All three fixes are targeted, minimal, and defensive. The package.json change is a simple dependency addition, and the embedder.js changes add safety guards and proper logging without altering core logic. No new functionality is introduced, only bug fixes.
• No files require special attention

Important Files Changed

Filename	Overview
package.json	Added missing @optave/codegraph-win32-x64-msvc and pinned all platform packages to 2.4.0
src/embedder.js	Fixed crash in extractLeadingComment with bounds checks, and redirected model loading logs to stderr via info()

_{Last reviewed commit: 4d13b0b}

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[claude]

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

---

Claude is reviewing this PR

---

PR Review: Dogfood v2.4.0 Bug Fixes

Todo List:

• Read CLAUDE.md for repository context and guidelines
• Use codegraph to analyze the changes and their impact
• Read changed files to understand implementation
• Phase 1: Root Cause Analysis
• Phase 2: Configuration Change Detection
• Phase 3: Test Integrity Assessment
• Phase 4: Code Quality Review
• Provide final rating and recommendation

---

Phase 1: Root Cause Analysis ✅

Problem Being Solved: Three legitimate bugs found during v2.4.0 dogfooding - missing Windows binary, embedder crash on stale data, stdout pollution breaking JSON output
Why This Approach: Each fix targets the actual root cause with minimal scope and proper defensive coding
Risk Assessment: LOW RISK - Targeted fixes with clear justification and no configuration relaxations

Phase 2: Configuration Change Detection ✅

NO HIGH-RISK CONFIG CHANGES DETECTED

• No biome/linting rules modified
• No test timeouts or coverage thresholds changed
• No build configuration changes
• No TypeScript strict mode modifications

Only legitimate dependency additions and version pinning.

Phase 3: Test Integrity Assessment ✅

NO TEST INTEGRITY CONCERNS

• No test files modified
• No assertions removed or weakened
• No test logic changes
• Changes are in production code only

Phase 4: Code Quality Review ✅

package.json Changes

EXCELLENT - The optionalDependencies changes are exactly what they should be:

+    "@optave/codegraph-win32-x64-msvc": "2.4.0"  // Adds missing Windows support
-    "@optave/codegraph-darwin-arm64": "2.3.0",   // Pins to current version
+    "@optave/codegraph-darwin-arm64": "2.4.0",   // for consistency

This ensures platform-specific native binaries are consistently versioned and Windows gets proper native support instead of falling back to WASM.

embedder.js Changes

SOLID DEFENSIVE CODING:

1. Bounds checking fix (lines 138, 141):

   if (fnLineIndex > lines.length) return null;  // Early exit
   if (i >= lines.length) continue;              // Skip invalid indices

   This properly handles stale graph data where line numbers may exceed actual file length.

2. Logging infrastructure fix (lines 303, 326):

   - console.log(`Loading embedding model: ${config.name}...`);
   + info(`Loading embedding model: ${config.name}...`);

   Routes model loading messages through the proper logging system (stderr) instead of polluting stdout, which breaks JSON output piping.

3. Import added correctly (line 7):

   + import { info, warn } from './logger.js';

Code Analysis

• Security: No security concerns - all changes are defensive
• Performance: No performance impact, potentially positive (Windows native vs WASM)
• Maintainability: Improves maintainability by fixing crash conditions
• Best Practices: Follows existing code patterns and error handling

Critical Concerns

NONE IDENTIFIED - This is a well-executed bug fix PR.

Minor Observations

1. Incomplete fix mention: PR description notes bug(builder): incremental rebuild drops edges when re-parsing a file #116 (incremental rebuild drops edges) is not fixed, which is appropriate transparency
2. Good commit message: Detailed explanation with issue references and impact summary
3. Small scope: Only 2 files changed, keeping blast radius minimal

Final Recommendation

## Root Cause Analysis
**Problem Being Solved**: Three legitimate bugs found during v2.4.0 dogfooding - missing Windows binary, embedder crash on stale data, stdout pollution breaking JSON output
**Why This Approach**: Each fix targets the actual root cause with minimal scope and proper defensive coding
**Risk Assessment**: LOW RISK - Targeted fixes with clear justification and no configuration relaxations

## Critical Concerns (if any)
None. This is a well-executed bug fix PR.

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: APPROVE
- Reasoning: Solid engineering - fixes real problems with appropriate solutions, maintains code quality, no test integrity issues, and follows defensive coding practices. Small scope reduces risk. The only reason it's not 5 stars is that it's fixing bugs rather than adding exceptional value.

APPROVE - This PR demonstrates good engineering practices and addresses legitimate issues discovered through dogfooding.