iframe-proxy

carlos-alm · 2026-02-26T05:45:43Z

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

…ashes - Add @optave/codegraph-win32-x64-msvc to optionalDependencies (was missing, causing fallback to WASM on Windows) - Pin all native binary packages to 2.4.0 - Add bounds check in extractLeadingComment to prevent crash on stale graph data where node line numbers exceed file length - Route model loading messages through logger (stderr) instead of console.log (stdout) to fix pipe-clean JSON output for search --json Closes #113 Closes #114 Closes #115 Impact: 2 functions changed, 9 affected

claude · 2026-02-26T05:45:59Z

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:

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
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
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:

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.

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.

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

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
Good commit message: Detailed explanation with issue references and impact summary
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 · 2026-02-26T05:48:32Z

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 · 2026-02-26T05:48:43Z

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:

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.

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.

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

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
Good commit message: Detailed explanation with issue references and impact summary
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.

All 4 bugs now fixed (PR #117 merged, #116 closed via reverse-dep cascade). 3 of 4 suggestions addressed. MCP tool counts updated 18→23 / 19→24. Rating upgraded 7/10 → 9/10 post-fix.

* fix: strict type validation for threshold values in complexity queries Replace loose `!= null` checks with `typeof === 'number' && Number.isFinite()` to prevent `Number("")`, `Number(null)`, and `Number(true)` from silently coercing into valid SQL values. Add integration test verifying exceeds arrays and summary.aboveWarn are correctly computed. Addresses Greptile review feedback on #136. Impact: 2 functions changed, 3 affected * docs: add complexity, communities, and manifesto to all docs Update README, CLAUDE.md, BACKLOG, titan-paradigm, recommended-practices, and CLI/MCP examples to reflect today's merged PRs: complexity metrics (#130/#139), Louvain community detection (#133/#134), and manifesto rule engine (#138). Updates MCP tool count from 21 to 24 (25 in multi-repo), marks backlog items 6/11/21/22 as done, and adds real CLI output examples. * fix: remove redundant condition in paginate guard clauses When limit === undefined, limit !== 0 is always true — the && check was dead code. Simplified to just check limit === undefined. Impact: 2 functions changed, 18 affected * docs: update dogfood report with fix statuses All 4 bugs now fixed (PR #117 merged, #116 closed via reverse-dep cascade). 3 of 4 suggestions addressed. MCP tool counts updated 18→23 / 19→24. Rating upgraded 7/10 → 9/10 post-fix. * fix: rename misleading test to match actual behavior Test was named "handles non-numeric thresholds gracefully" but only validated baseline exceeds/aboveWarn with valid thresholds. Actual non-numeric threshold tests exist separately. Renamed to "produces correct exceeds and aboveWarn with valid thresholds". * fix: update stale MCP tool count in dogfood skill (21→24) --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* fix: strict type validation for threshold values in complexity queries Replace loose `!= null` checks with `typeof === 'number' && Number.isFinite()` to prevent `Number("")`, `Number(null)`, and `Number(true)` from silently coercing into valid SQL values. Add integration test verifying exceeds arrays and summary.aboveWarn are correctly computed. Addresses Greptile review feedback on #136. Impact: 2 functions changed, 3 affected * docs: add complexity, communities, and manifesto to all docs Update README, CLAUDE.md, BACKLOG, titan-paradigm, recommended-practices, and CLI/MCP examples to reflect today's merged PRs: complexity metrics (#130/#139), Louvain community detection (#133/#134), and manifesto rule engine (#138). Updates MCP tool count from 21 to 24 (25 in multi-repo), marks backlog items 6/11/21/22 as done, and adds real CLI output examples. * fix: remove redundant condition in paginate guard clauses When limit === undefined, limit !== 0 is always true — the && check was dead code. Simplified to just check limit === undefined. Impact: 2 functions changed, 18 affected * docs: update dogfood report with fix statuses All 4 bugs now fixed (PR #117 merged, #116 closed via reverse-dep cascade). 3 of 4 suggestions addressed. MCP tool counts updated 18→23 / 19→24. Rating upgraded 7/10 → 9/10 post-fix. * fix: rename misleading test to match actual behavior Test was named "handles non-numeric thresholds gracefully" but only validated baseline exceeds/aboveWarn with valid thresholds. Actual non-numeric threshold tests exist separately. Renamed to "produces correct exceeds and aboveWarn with valid thresholds". * fix: update stale MCP tool count in dogfood skill (21→24) * feat: add complexity analysis for Python, Go, Rust, Java, C#, Ruby, PHP Parameterize the complexity algorithm to support all 10 languages instead of just JS/TS/TSX. Add per-language COMPLEXITY_RULES, HALSTEAD_RULES, and COMMENT_PREFIXES with three else-if detection patterns (else-wraps-if, explicit elif, alternative field). Guard against tree-sitter keyword leaf tokens that share node type names with their parent constructs. Impact: 4 functions changed, 4 affected --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

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

* fix: strict type validation for threshold values in complexity queries Replace loose `!= null` checks with `typeof === 'number' && Number.isFinite()` to prevent `Number("")`, `Number(null)`, and `Number(true)` from silently coercing into valid SQL values. Add integration test verifying exceeds arrays and summary.aboveWarn are correctly computed. Addresses Greptile review feedback on optave#136. Impact: 2 functions changed, 3 affected * docs: add complexity, communities, and manifesto to all docs Update README, CLAUDE.md, BACKLOG, titan-paradigm, recommended-practices, and CLI/MCP examples to reflect today's merged PRs: complexity metrics (optave#130/optave#139), Louvain community detection (optave#133/optave#134), and manifesto rule engine (optave#138). Updates MCP tool count from 21 to 24 (25 in multi-repo), marks backlog items 6/11/21/22 as done, and adds real CLI output examples. * fix: remove redundant condition in paginate guard clauses When limit === undefined, limit !== 0 is always true — the && check was dead code. Simplified to just check limit === undefined. Impact: 2 functions changed, 18 affected * docs: update dogfood report with fix statuses All 4 bugs now fixed (PR optave#117 merged, optave#116 closed via reverse-dep cascade). 3 of 4 suggestions addressed. MCP tool counts updated 18→23 / 19→24. Rating upgraded 7/10 → 9/10 post-fix. * fix: rename misleading test to match actual behavior Test was named "handles non-numeric thresholds gracefully" but only validated baseline exceeds/aboveWarn with valid thresholds. Actual non-numeric threshold tests exist separately. Renamed to "produces correct exceeds and aboveWarn with valid thresholds". * fix: update stale MCP tool count in dogfood skill (21→24) --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* fix: strict type validation for threshold values in complexity queries Replace loose `!= null` checks with `typeof === 'number' && Number.isFinite()` to prevent `Number("")`, `Number(null)`, and `Number(true)` from silently coercing into valid SQL values. Add integration test verifying exceeds arrays and summary.aboveWarn are correctly computed. Addresses Greptile review feedback on optave#136. Impact: 2 functions changed, 3 affected * docs: add complexity, communities, and manifesto to all docs Update README, CLAUDE.md, BACKLOG, titan-paradigm, recommended-practices, and CLI/MCP examples to reflect today's merged PRs: complexity metrics (optave#130/optave#139), Louvain community detection (optave#133/optave#134), and manifesto rule engine (optave#138). Updates MCP tool count from 21 to 24 (25 in multi-repo), marks backlog items 6/11/21/22 as done, and adds real CLI output examples. * fix: remove redundant condition in paginate guard clauses When limit === undefined, limit !== 0 is always true — the && check was dead code. Simplified to just check limit === undefined. Impact: 2 functions changed, 18 affected * docs: update dogfood report with fix statuses All 4 bugs now fixed (PR optave#117 merged, optave#116 closed via reverse-dep cascade). 3 of 4 suggestions addressed. MCP tool counts updated 18→23 / 19→24. Rating upgraded 7/10 → 9/10 post-fix. * fix: rename misleading test to match actual behavior Test was named "handles non-numeric thresholds gracefully" but only validated baseline exceeds/aboveWarn with valid thresholds. Actual non-numeric threshold tests exist separately. Renamed to "produces correct exceeds and aboveWarn with valid thresholds". * fix: update stale MCP tool count in dogfood skill (21→24) * feat: add complexity analysis for Python, Go, Rust, Java, C#, Ruby, PHP Parameterize the complexity algorithm to support all 10 languages instead of just JS/TS/TSX. Add per-language COMPLEXITY_RULES, HALSTEAD_RULES, and COMMENT_PREFIXES with three else-if detection patterns (else-wraps-if, explicit elif, alternative field). Guard against tree-sitter keyword leaf tokens that share node type names with their parent constructs. Impact: 4 functions changed, 4 affected --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

docs: add dogfood report for v2.4.0

4d13b0b

greptile-apps Bot reviewed Feb 26, 2026

View reviewed changes

carlos-alm mentioned this pull request Feb 26, 2026

docs: update build performance benchmarks #112

Closed

carlos-alm merged commit 9689a32 into main Feb 26, 2026
17 of 18 checks passed

carlos-alm deleted the fix/dogfood-v2.4.0-bugs branch February 26, 2026 06:52

Zeeeepa pushed a commit to Zeeeepa/codegraph that referenced this pull request Jun 22, 2026

Merge pull request optave#117 from optave/fix/dogfood-v2.4.0-bugs

78ac2fb

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

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

fix: dogfood v2.4.0 — win32 native binary, embedder crashes, stdout pollution#117
carlos-alm merged 2 commits into
mainfrom
fix/dogfood-v2.4.0-bugs

carlos-alm commented Feb 26, 2026

Uh oh!

claude Bot commented Feb 26, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Feb 26, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

claude Bot commented Feb 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

carlos-alm commented Feb 26, 2026

Summary

Found during

Test plan

Uh oh!

claude Bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Dogfood v2.4.0 Bug Fixes

Todo List:

Phase 1: Root Cause Analysis ✅

Phase 2: Configuration Change Detection ✅

Phase 3: Test Integrity Assessment ✅

Phase 4: Code Quality Review ✅

package.json Changes

embedder.js Changes

Code Analysis

Critical Concerns

Minor Observations

Final Recommendation

Uh oh!

greptile-apps Bot commented Feb 26, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Dogfood v2.4.0 Bug Fixes

Todo List:

Phase 1: Root Cause Analysis ✅

Phase 2: Configuration Change Detection ✅

Phase 3: Test Integrity Assessment ✅

Phase 4: Code Quality Review ✅

package.json Changes

embedder.js Changes

Code Analysis

Critical Concerns

Minor Observations

Final Recommendation

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Feb 26, 2026 •

edited

Loading

claude Bot commented Feb 26, 2026 •

edited

Loading