feat(benchmark): add 1-file rebuild phase breakdown

[carlos-alm]

Summary

• Capture per-phase timing from incremental 1-file rebuilds (median run) in both benchmark.js and incremental-benchmark.js
• Render build and 1-file rebuild phases side-by-side in a single combined table: Native (build) | WASM (build) | Native (1-file) | WASM (1-file)
• Remove obsolete wasmPreMs phase tracking from builder and report generator — always 0ms now that CFG/dataflow run natively

Test plan

• Run node scripts/benchmark.js and verify oneFilePhases appears in JSON output for both engines
• Pipe output to node scripts/update-benchmark-report.js and verify the combined 4-column phase table renders correctly
• Verify older benchmark data (without oneFilePhases) falls back to the 2-column layout

[claude]

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

---

PR Review Complete

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze all changed files
• Root cause analysis - understand problem being solved
• Check for configuration changes or test modifications
• Review code quality and implementation
• Provide final assessment with rating and recommendation

---

Root Cause Analysis

Problem Being Solved: The benchmarking system lacked detailed timing breakdowns for incremental 1-file rebuilds, which is a critical performance metric for understanding real-world usage patterns. The existing system only captured total rebuild times without phase-level visibility.

Why This Approach:

• Captures phase timing from the median run (most representative) rather than averaging all runs
• Removes obsolete wasmPreMs tracking that was always 0ms after CFG/dataflow moved to native execution
• Creates a unified 4-column table layout that shows both full build and incremental rebuild phases side-by-side
• Maintains backward compatibility with older benchmark data that lacks oneFilePhases

Risk Assessment: LOW RISK - This is purely additive benchmarking instrumentation with clean removal of dead code.

Technical Implementation Review

✅ Clean Code Removal (src/builder.js)

• Correctly removes obsolete wasmPreMs tracking (lines 1451-1456, 1604)
• The removed code was indeed dead - WASM pre-parsing is no longer needed since CFG/dataflow run natively
• No logic changes to core functionality

✅ Well-Structured Data Collection (scripts/benchmark.js, scripts/incremental-benchmark.js)

• Lines 109-121: Improved data collection methodology:
  • Stores both timing and phases for each run
  • Sorts by timing and selects median run for phases (more accurate than averaging phases)
  • Proper error handling with finally block restoration
• Lines 164, 212, 227: Correctly adds oneFilePhases to output schema
• Consistent implementation across both benchmark scripts

✅ Robust Report Generation (scripts/update-benchmark-report.js)

• Lines 151-175: Well-organized phase breakdown logic:
  • Shared phase definitions and labels (good DRY principle)
  • Conditional 4-column vs 2-column layout based on data availability
  • Proper handling of null/missing values with fmtPhase helper
• Lines 176-184: Smart fallback mechanism for older benchmark data
• Removes wasmPreMs from phase keys (lines removed from original implementation)

✅ Backward Compatibility

• Report generator gracefully handles benchmark data with or without oneFilePhases
• Existing 2-column layout preserved when one-file phase data is unavailable
• No breaking changes to existing JSON schema

Quality Assessment

Strengths:

1. Methodologically sound: Using median run for phase data is more accurate than averaging
2. Clean refactoring: Properly removes dead wasmPreMs code without side effects
3. Future-proof design: 4-column layout provides valuable insights for performance optimization
4. Defensive programming: Proper null checks and fallback handling
5. Consistent implementation: Both benchmark scripts updated identically

Minor Observations:

1. Line 119 (scripts/benchmark.js): Math.floor(oneFileRuns.length / 2) correctly implements median selection
2. The phase key ordering is logical: parse → insert → resolve → edges → structure → roles → ast → complexity → cfg → dataflow
3. Good use of optional chaining (res?.phases) for safety

Critical Concerns

None identified. This is a well-executed enhancement with appropriate testing considerations.

Final Recommendation

## Root Cause Analysis  
**Problem Being Solved**: Missing phase-level visibility in incremental rebuild benchmarks, essential for performance optimization and regression detection
**Why This Approach**: Clean removal of dead code + methodologically sound median-based phase collection + backward-compatible reporting
**Risk Assessment**: LOW - Pure benchmarking enhancement with no functional changes

## Critical Concerns
None identified.

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: **APPROVE**  
- Reasoning: High-quality implementation that enhances benchmark visibility without risk. Well-structured code with proper error handling and backward compatibility. The removal of dead `wasmPreMs` code is a welcome cleanup.

Test Plan Validation: The proposed test plan is appropriate:

• ✅ Verify oneFilePhases appears in JSON output
• ✅ Confirm 4-column table renders correctly
• ✅ Verify backward compatibility with older data

This PR delivers exactly what it promises with clean, professional implementation.

[greptile-apps]

Greptile Summary

This PR extends the benchmark tooling to capture per-phase timing from the median incremental (1-file) rebuild run alongside full-build phase data, and removes the now-obsolete wasmPreMs tracking from the builder.

Changes:

• scripts/benchmark.js & scripts/incremental-benchmark.js: Collect 1-file rebuild runs with phase data, sort by elapsed time, pick the median run, and surface both timing and phase breakdown as oneFilePhases in JSON output.
• scripts/update-benchmark-report.js: Consolidates phase definitions into shared constants and renders a 4-column phase table when both full-build and 1-file phases are available, falling back cleanly to the 2-column layout for older data.
• src/builder.js: Removes the wasmPreMs start/stop instrumentation and its conditional inclusion in the phases object.
• generated/benchmarks/BUILD-BENCHMARKS.md: Updates documentation to reflect removal of WASM pre-parse tracking.

The implementation correctly handles backward compatibility and conditional rendering based on data availability.

Confidence Score: 5/5

• Safe to merge. Changes are confined to benchmark tooling and removal of dead instrumentation code with no impact on production graph-building logic.
• All file changes are correct and well-implemented. The core logic for capturing and selecting median runs is sound. The conditional rendering in update-benchmark-report.js correctly handles both new (with 1-file phases) and older (without) benchmark data. The removal of wasmPreMs is clean with no dependencies elsewhere. Backward compatibility is properly maintained for older benchmark data.
• No files require special attention

Important Files Changed

Filename	Overview
scripts/benchmark.js	Correctly captures per-phase timing from the median incremental run; index-based median selection (Math.floor(runs.length / 2)) is consistent with the current INCREMENTAL_RUNS=3 configuration.
scripts/incremental-benchmark.js	Adds oneFilePhases capture alongside existing 1-file rebuild timing metrics. Output structure is appropriate for the focused scope of this benchmark (build tiers and import resolution).
scripts/update-benchmark-report.js	Correctly implements conditional rendering: 4-column phase table when both full-build and 1-file phases are present, 2-column layout for full-build phases alone, and clean fallback for older data. Shared phase constants are well-structured.
src/builder.js	Clean removal of wasmPreMs timing instrumentation and its conditional inclusion in the returned phases object. Reflects the documented fact that CFG/dataflow now run natively, with no impact on production graph-building logic.
generated/benchmarks/BUILD-BENCHMARKS.md	Documentation update accurately reflects removal of WASM pre-parse phase and corrected cumulative overhead totals (~1,187 ms instead of ~1,575 ms).

Sequence Diagram

sequenceDiagram
    participant B as benchmark.js / incremental-benchmark.js
    participant BG as buildGraph (builder.js)
    participant R as update-benchmark-report.js

    Note over B: 1-file rebuild loop (INCREMENTAL_RUNS=3)
    loop Each run i
        B->>BG: buildGraph(root, { engine, incremental: true })
        BG-->>B: { phases: { parseMs, insertMs, ... } }
        B->>B: oneFileRuns.push({ ms, phases })
    end
    B->>B: sort by ms, pick index Math.floor(len/2)
    B->>B: oneFileRebuildMs = medianRun.ms
    B->>B: oneFilePhases = medianRun.phases

    B->>B: JSON.stringify({ ..., phases (full build), oneFilePhases })
    B->>R: pipe JSON output

    R->>R: hasPhases = latest.native?.phases || latest.wasm?.phases
    R->>R: hasOneFilePhases = latest.native?.oneFilePhases || latest.wasm?.oneFilePhases

    alt hasPhases && hasOneFilePhases
        R->>R: Render 4-column table (build + 1-file)
    else hasPhases only
        R->>R: Render 2-column table (build only)
    else neither
        R->>R: Skip phase section entirely
    end

Loading

_{Last reviewed commit: 8433660}