fix: preserve hand-written notes in benchmark regeneration

[carlos-alm]

Summary

• Adds NOTES_START/NOTES_END sentinel preservation to scripts/update-benchmark-report.js so CI-triggered regenerations no longer clobber hand-written analysis sections
• Seeds the benchmark report with WASM regression analysis notes (v2.0.0 → v2.1.0 ↑32%, persisting through v2.3.0)

Test plan

• Run node scripts/update-benchmark-report.js and verify the notes section survives regeneration
• Verify the <!-- NOTES_START --> / <!-- NOTES_END --> block is preserved across multiple runs

[claude]

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

---

PR Review Complete

Reviewing PR #128: fix: preserve hand-written notes in benchmark regeneration

Todo List:

• Read and analyze changed files
• Perform root cause analysis
• Check for configuration changes and test integrity
• Evaluate code quality and approach
• Provide final assessment with rating

---

Root Cause Analysis

Problem Being Solved: CI-triggered benchmark regeneration was clobbering hand-written analysis notes in the benchmark report, causing valuable investigative content (like WASM regression analysis) to be lost on each update.

Why This Approach: The sentinel-based preservation approach (/) is a standard, proven pattern for preserving hand-written content in generated files. This is solving a legitimate problem where automation was destroying manually curated content.

Risk Assessment: LOW RISK - This is a preservation feature that prevents data loss, not a configuration relaxation or symptom masking.

Code Quality Assessment

Strengths

• Clean implementation: Lines 203-214 in update-benchmark-report.js implement the preservation logic cleanly with regex matching and conditional insertion
• Non-breaking: The change gracefully handles files without existing notes sections
• Valuable content preserved: The seeded notes contain detailed WASM regression analysis (v2.0.0 → v2.1.0 ↑32%) with technical root cause investigation
• Follows existing patterns: Uses HTML comment sentinels consistent with the existing BENCHMARK_DATA preservation pattern

Technical Implementation

The preservation logic is sound:

let notes = '';
if (fs.existsSync(benchmarkPath)) {
    const existing = fs.readFileSync(benchmarkPath, 'utf8');
    const notesMatch = existing.match(/\n([\s\S]*?)/);
    if (notesMatch) {
        notes = notesMatch[1];
    }
}
if (notes) {
    md += `\n${notes}\n\n`;
}

Minor Observations

• Regex is correctly non-greedy ([\s\S]*?) to avoid over-matching
• Sentinel placement is logical (after data tables, before raw JSON)
• No error handling around regex matching, but failure would just result in no preservation (acceptable degradation)

Critical Concerns

None identified. This is a legitimate bug fix that preserves valuable content without introducing security, performance, or maintainability issues.

Test Plan Verification

The PR includes appropriate test steps:

• Manual verification that notes survive regeneration
• Verification that sentinel blocks are preserved across multiple runs

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: This is a well-implemented fix for a legitimate problem. The code is clean, follows established patterns, and preserves valuable investigative content that was being lost to automation. The implementation is minimal and non-breaking, with appropriate degradation behavior. The only minor improvement would be adding automated tests, but for a utility script this level of testing may be proportionate to the risk.

Quality indicators:

• Solves real problem (CI clobbering manual content)
• Minimal, focused change (+13 lines script, +20 lines preserved content)
• Uses standard preservation patterns
• Preserves valuable technical analysis content
• Non-breaking implementation

[greptile-apps]

Greptile Summary

Added sentinel preservation to the benchmark report regeneration script so CI can update metrics without clobbering hand-written analysis. The implementation uses <!-- NOTES_START --> / <!-- NOTES_END --> sentinels with regex extraction to preserve notes across regenerations. The PR seeds the report with detailed WASM regression analysis explaining the 32% performance degradation from v2.0.0 to v2.1.0.

• Implementation is clean with proper edge case handling (missing file, no match, empty notes)
• Regex pattern correctly uses non-greedy matching to capture content between sentinels
• Notes are placed logically after metrics tables and before raw JSON data
• The analysis notes provide valuable context about WASM/native performance divergence

Confidence Score: 5/5

• This PR is safe to merge with no risk
• The change is well-implemented with proper edge case handling, uses standard regex patterns correctly, and solves a clear problem (preventing CI from overwriting hand-written notes). No logical issues, security concerns, or breaking changes.
• No files require special attention

Important Files Changed

Filename	Overview
scripts/update-benchmark-report.js	Added sentinel-based preservation logic for hand-written notes sections - implementation is clean and handles edge cases properly
generated/BUILD-BENCHMARKS.md	Added WASM regression analysis notes between sentinels - provides detailed context for performance regression from v2.0.0 to v2.1.0

_{Last reviewed commit: 299da2a}

[greptile-apps]

_{2 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}