fix: preserve per-instance params for duplicate benchmark names

[k-rister]

Summary

• A run file with two benchmarks[] entries sharing the same name (e.g. two concurrent engine pairs of the same benchmark running in opposite directions) silently collapsed onto one entry's params. Found live while validating a new benchmark's bidirectional test scenario.
• get_mv_params() now resolves a benchmark's mv-params by its array index instead of first-name-match, and each occurrence gets its own intermediate files.
• load_bench_params() now auto-scopes every param from an ambiguous (duplicate-name) benchmark instance to that instance's own ids by default -- an individual param can still set an explicit id to override this.
• Adds a shared expand_id_ranges() helper (also used to de-duplicate assign_bench_ids()), and generalizes dump_params()'s existing per-param id scoping to accept "+"-joined id sets.
• Adds 20 unit tests (tests/test_dump_params.py, new tests/test_load_bench_params_duplicate_names.py, util/tests/test-blockbreaker.py + new fixture).

Fixes #873

Test plan

• pytest -v tests/*.py -- 72 passed, 0 regressions
• pytest -v blockbreaker.py validate_run_file.py tests/*.py (from util/) -- 14 passed, 1 skipped
• Verified live against the original bug scenario (two benchmarks[] entries sharing a name, different ids) before writing the fix

🤖 Generated with Claude Code

[k-rister]

PR Review: rickshaw#874 — fix: preserve per-instance params for duplicate benchmark names

Summary: Resolves parameter collision for run files containing multiple benchmark instances sharing the same name by indexing blockbreaker.py's get_mv_params() by array position, generating per-occurrence intermediate parameter files, and automatically scoping parameters to an ambiguous instance's assigned IDs.
Changed files: 6
Review dimensions: Correctness, API & Contracts, Build & Deploy, Documentation, Style, Completeness

Bugs

• [rickshaw-post-process-bench.py:56-57] dump_params() in post-processing does not support "+"-joined ID sets — When duplicate benchmark instances have multi-engine ID ranges (e.g. ids: "1-2"), load_bench_params() auto-scopes the instance's parameters with param["id"] = "1+2" in rickshaw-run.json. While dump_params() in rickshaw-run.py was updated to split on "+", dump_params() in rickshaw-post-process-bench.py still compares str(param_id) != str(cs_id). For engine 1, "1+2" != "1" evaluates to True, causing post-processing benchmark scripts (controller.post-script) to omit all parameters for multi-ID duplicate instances.

Issues

• [rickshaw-post-process-bench.py:132-152] Stale duplicate ID expansion logic in post-processor — rickshaw-post-process-bench.py duplicates the older manual ids_str expansion logic inline instead of sharing expand_id_ranges(), creating potential divergence as ID syntax evolves.

File Coverage

• rickshaw-run.py — No issues found; expand_id_ranges(), dump_params(), _process_from_file(), load_bench_params(), and assign_bench_ids() correctly isolate duplicate instances.
• tests/test_dump_params.py — No issues found; verified +-joined ID filtering tests pass.
• tests/test_load_bench_params_duplicate_names.py — No issues found; comprehensive unit coverage for single-instance preservation, ambiguous name scoping, and explicit ID overrides.
• util/blockbreaker.py — No issues found; get_mv_params() properly resolves by positional index with fallback protection.
• util/tests/JSON/input-duplicate-benchmark-name.json — No issues found; schema-valid test fixture.
• util/tests/test-blockbreaker.py — No issues found; index resolution and occurrence listing test cases all pass.

Missing from diff:

• rickshaw-post-process-bench.py — Companion dump_params() implementation needs if str(cs_id) not in str(param_id).split("+"): continue to handle +-joined IDs during post-processing.

Limitations

• Verified via unit test suites (tests/*.py and util/tests/*.py). Did not execute full multi-host / multi-container Kubernetes runs against live hardware.

Verdict

Request changes — The core fix in rickshaw-run.py and util/blockbreaker.py is solid and well-tested, but the companion dump_params() in rickshaw-post-process-bench.py (lines 56-57) must also be updated to split on "+" so post-processing does not drop parameters on multi-engine duplicate instances.

[atheurer]

The approach for isolating duplicate benchmark instances via positional indexing in blockbreaker.py, indexing intermediate parameter files, and auto-scoping ambiguous instance parameters in rickshaw-run.py looks very good and well-tested.

One spot that needs a companion update:

• rickshaw-post-process-bench.py (lines 56–57):
  dump_params() in the post-processor still checks strict equality:

  if param_id is not None and cs_id is not None and str(param_id) != str(cs_id):
      continue

  When an ambiguous benchmark instance has multiple IDs (e.g. ids: "1-2"), load_bench_params() assigns param["id"] = "1+2". During post-processing for engine 1, str("1+2") != "1" causes all instance parameters to be skipped for post-processing scripts.

  Please update dump_params() in rickshaw-post-process-bench.py to match rickshaw-run.py:

  if param_id is not None and cs_id is not None:
      if str(cs_id) not in str(param_id).split("+"):
          continue

With that update in place, this will be ready to go!

[k-rister]

Thanks for the review — both points confirmed and addressed in 055dadd:

• Bug (rickshaw-post-process-bench.py:56-57): fixed. This script's dump_params() now splits param_id on "+" the same way rickshaw-run.py's does, so multi-id duplicate instances (e.g. ids: "1-2" → id: "1+2") no longer get all their params dropped during post-processing.
• Issue (stale duplicate ID expansion logic): fixed by extracting expand_id_ranges() into a new shared rickshaw_lib/id_ranges.py module. rickshaw-run.py and rickshaw-post-process-bench.py both import it now instead of each maintaining their own copy — which is exactly how the bug above happened in the first place.

Also added tests/test_post_process_bench_dump_params.py (4 tests) covering this script's dump_params() id-scoping, since it had no prior coverage. Full suite: 76 + 14 passing, zero regressions.