fix(core): make Go benchmark metadata request-scoped by hfl0506 · Pull Request #238 · goptics/vizb

hfl0506 · 2026-07-18T16:57:11Z

Related Issue

Resolve #236

Changes:

Parser API now returns request-local *shared.Meta.
Go benchmark parsing no longer writes shared globals.
Removed shared/bench_state.go.
core.Convert passes parsed metadata into core.Assemble, while preserving
explicit caller metadata.
CLI Dataset metadata and JSON wire format remain unchanged.
Added a 64-way concurrent conversion regression test.

Test Result

task build:cli

printf '%s\n' \
    'goos: linux' \
    'goarch: amd64' \
    'pkg: example.com/local-test' \
    'cpu: Local Test CPU' \
    'BenchmarkExample-8 100 1234 ns/op' \
    > /tmp/vizb-benchmark.txt
    
 ./bin/vizb /tmp/vizb-benchmark.txt --output /tmp/vizb-result.json
  jq '.meta' /tmp/vizb-result.json

# result
{
    "cpu": {
      "name": "Local Test CPU",
      "cores": 8
    "os": "linux",
    "arch": "amd64",
    "pkg": "example.com/local-test"
  }

Summary by CodeRabbit

New Features
- Benchmark parsing and conversions now carry system metadata (OS, architecture, package, CPU) through to the assembled dataset and JSON output.
- Explicit metadata provided at conversion time is preserved.
Bug Fixes
- Metadata is now request-local, improving correctness for concurrent conversions.
- Temporary file management is now concurrency-safe.
Tests
- Updated parser/core pipeline tests for the new metadata return contract.
- Added coverage for metadata preservation, request-local behavior under concurrency, and concurrent temporary-file access.

coderabbitai · 2026-07-18T16:57:28Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4bb239e4-0912-42bd-a055-8bc23d8aa343

📥 Commits

Reviewing files that changed from the base of the PR and between 46b33d5 and 651a2aa.

📒 Files selected for processing (6)

pkg/parser/golang/golang.go
pkg/parser/javascript/tinybench.go
pkg/parser/javascript/vitest.go
pkg/parser/rust/criterion.go
pkg/parser/rust/divan.go
shared/tmp_files_manager_test.go

🚧 Files skipped from review as they are similar to previous changes (5)

shared/tmp_files_manager_test.go
pkg/parser/rust/divan.go
pkg/parser/rust/criterion.go
pkg/parser/javascript/vitest.go
pkg/parser/golang/golang.go

📝 Walkthrough

Walkthrough

The parser contract now returns request-local system metadata. Go benchmark parsing populates it, core conversion applies it as a fallback, and CLI assembly preserves it in dataset JSON. Shared temporary-file operations and related concurrency tests are synchronized.

Changes

Request-local benchmark metadata

Layer / File(s)	Summary
Parser metadata return contract `pkg/parser/...`	Parser functions return a system metadata slot; Go benchmark parsing populates OS, architecture, package, and CPU metadata locally.
Conversion metadata fallback `pkg/core/core.go`, `pkg/core/core_test.go`	`Convert` uses parser metadata when request metadata lacks a system value, while preserving explicit metadata and validating concurrent conversions.
CLI metadata assembly `cmd/cli/pipeline.go`, `cmd/cli/pipeline_test.go`	CLI preparation passes system metadata through dataset assembly, with updated call sites and wire-format coverage.

Concurrency safety updates

Layer / File(s)	Summary
Synchronized shared test state `shared/tmp_files_manager.go`, `shared/tmp_files_manager_test.go`, `shared/exit_with_err_test.go`	Temporary-file tracking is mutex-protected, and concurrent exit testing uses a direct stub and call channel.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BenchmarkInput
  participant ParseGoBenchmark
  participant Convert
  participant AssembleDataset
  BenchmarkInput->>ParseGoBenchmark: parse benchmark records
  ParseGoBenchmark-->>Convert: points, config, system metadata
  Convert->>Convert: apply system metadata fallback
  Convert-->>AssembleDataset: dataset metadata
  AssembleDataset-->>BenchmarkInput: marshaled dataset

Suggested reviewers: fahimfaisaal

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Unrelated changes were introduced, including TmpFilesManager concurrency locking and the exit-with-error test rewrite.	Split the unrelated shared/* changes into a separate PR so this one stays focused on benchmark metadata scoping.
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and matches the main change: request-scoped Go benchmark metadata.
Linked Issues check	✅ Passed	The PR returns request-local benchmark metadata, passes it into core.Assemble, preserves CLI wire format, and adds the concurrent regression test.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codecov-commenter · 2026-07-18T17:04:22Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@shared/tmp_files_manager_test.go`:
- Around line 103-108: Replace the invalid wg.Go call in the concurrent test
with standard sync.WaitGroup orchestration: call wg.Add(1) for each iteration,
launch a goroutine, and defer or invoke wg.Done() within it while preserving the
existing manager.Store and manager.RemoveAll operations.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dc80d89a-8506-4e86-9916-c3d358f04167

📥 Commits

Reviewing files that changed from the base of the PR and between 914ada1 and 46b33d5.

📒 Files selected for processing (24)

cmd/cli/pipeline.go
cmd/cli/pipeline_test.go
pkg/core/core.go
pkg/core/core_test.go
pkg/parser/csv/csv.go
pkg/parser/csv/csv_test.go
pkg/parser/golang/golang.go
pkg/parser/golang/golang_test.go
pkg/parser/javascript/tinybench.go
pkg/parser/javascript/tinybench_test.go
pkg/parser/javascript/vitest.go
pkg/parser/javascript/vitest_test.go
pkg/parser/json/json.go
pkg/parser/json/json_test.go
pkg/parser/registry.go
pkg/parser/registry_test.go
pkg/parser/rust/criterion.go
pkg/parser/rust/criterion_test.go
pkg/parser/rust/divan.go
pkg/parser/rust/divan_test.go
shared/bench_state.go
shared/exit_with_err_test.go
shared/tmp_files_manager.go
shared/tmp_files_manager_test.go

💤 Files with no reviewable changes (1)

shared/bench_state.go

fix(core): make Go benchmark metadata request-scoped

2fdd905

fix(shared): synchronize temporary file cleanup

46b33d5

hfl0506 marked this pull request as ready for review July 18, 2026 17:28

hfl0506 requested a review from fahimfaisaal as a code owner July 18, 2026 17:28

coderabbitai Bot reviewed Jul 18, 2026

View reviewed changes

Comment thread shared/tmp_files_manager_test.go

hfl0506 added bug Something isn't working cli Command line interface related tasks labels Jul 18, 2026

fix: address review feedback for metadata scoping

651a2aa

fahimfaisaal added this to the Release v0.16.0 milestone Jul 19, 2026

fahimfaisaal approved these changes Jul 20, 2026

View reviewed changes

fahimfaisaal merged commit fe6fe5a into goptics:main Jul 20, 2026
12 checks passed

coderabbitai Bot mentioned this pull request Jul 22, 2026

feat(cli): add installation-aware update command #263

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(core): make Go benchmark metadata request-scoped#238

fix(core): make Go benchmark metadata request-scoped#238
fahimfaisaal merged 3 commits into
goptics:mainfrom
hfl0506:fix/request-local-benchmark-metadata

hfl0506 commented Jul 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (2 warnings)

Uh oh!

codecov-commenter commented Jul 18, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

hfl0506 commented Jul 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issue

Changes:

Test Result

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (2 warnings)

Uh oh!

codecov-commenter commented Jul 18, 2026

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hfl0506 commented Jul 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading