RPOPC-1320: Fix remaining 7 processors version extraction by grdumas · Pull Request #52 · redhat-performance/chronicler

grdumas · 2026-06-16T04:25:14Z

Summary

Fixes 7 remaining processors to extract benchmark version and use it in test.version instead of wrapper version, with state reset to prevent leakage between sequential parses.

Acceptance Criteria

Partially affected processors (autohpl, passmark, speccpu2017): Override build_test_info() to use already-extracted version from config
Fully affected processors (coremark_pro, phoronix, specjbb, uperf): Extract benchmark version from results AND override build_test_info()
Each processor instance resets benchmark version state at the start of parse_runs() to prevent state leakage when processing multiple distinct test runs sequentially
All processors follow the same pattern as fio and coremark: store self._benchmark_version or equivalent state, reset it in parse_runs(), and override build_test_info()
Tests verify version extraction and state reset for each processor

Changes

Group 1 - Partially Affected (version already in config)

autohpl: Override build_test_info() to use version from CSV comments
passmark: Override build_test_info() to use version from YML config
speccpu2017: Override build_test_info() to use version from config file

Group 2 - Fully Affected (version extraction required)

coremark_pro: Extract version from CSV comments + override build_test_info()
phoronix: Extract version from CSV comments + override build_test_info()
specjbb: Extract version from CSV comments + override build_test_info()
uperf: Extract version from CSV comments + override build_test_info()

Implementation Pattern

All processors follow the same pattern:

Add __init__ to store self._benchmark_version = None (or similar)
Reset state at START of parse_runs() to prevent leakage
Extract version (Group 2 only - from CSV comments or other metadata)
Override build_test_info() to use benchmark version with fallback to wrapper version

Testing

25 new tests added (version extraction + state reset for all 7 processors)
All 306 tests passing (no regressions)
Each processor tested for:
- Version extraction to test.version
- Fallback to wrapper version when benchmark version missing
- State reset between parse_runs() calls (prevents leakage)

Technical Details

Used parallel subagent workflow for implementation efficiency
Group 1 (3 processors) completed in parallel
Group 2 (4 processors) completed in parallel
All changes follow TDD pattern (test first, then implementation)
Atomic commits per processor

Files Changed

7 processor files modified (autohpl, passmark, speccpu2017, coremark_pro, phoronix, specjbb, uperf)
7 test files added/modified
+1730 lines added, -1 line removed

- Add __init__ method that stores self._speccpu_version = None - Reset _speccpu_version at START of parse_runs() to prevent state leakage - Store extracted version from version file in _speccpu_version - Override build_test_info() to return TestInfo with benchmark version (with fallback to wrapper version) - Add comprehensive test suite for version extraction and state reset - All tests pass (4 new tests + 5 existing timestamp tests) Implements RPOPC-1319 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Following the pattern from fio_processor.py (RPOPC-1318): - Add __init__ to initialize _passmark_version = None state - Reset state at START of parse_runs() to prevent leakage between parses - Override build_test_info() to return TestInfo with benchmark version (with fallback to wrapper version) - Extract version from YML Version block (Major.Minor.Build format) - Only store version if at least one component is non-zero Tests (from previous commit) verify: - Benchmark version extracted to test.version - Wrapper version preserved in test.wrapper_version - Fallback to wrapper version when benchmark version missing - State reset between sequential parses (no stale version) Validates RPOPC-1319. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Following the coremark pattern for fully affected processors: - Add __init__ to store _benchmark_version state - Reset state at start of parse_runs() to prevent leakage - Extract version from CSV comments (# Results version: 1.0) - Override build_test_info() to use benchmark version with fallback to wrapper version Tests verify: - Version extraction from CSV comments - Fallback to wrapper version when benchmark version missing - State reset between parses Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Follows same pattern as coremark_processor.py: - Add __init__ to store _benchmark_version state - Reset state at start of parse_runs() to prevent leakage - Extract benchmark version from CSV comments (# Results version: X.X) - Override build_test_info() to use benchmark version with fallback to wrapper version Tests verify: - Benchmark version extraction from CSV comments - Fallback to wrapper version when benchmark version missing - State reset between parses prevents version leakage Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Follow same pattern as coremark_processor.py: - Add __init__ method storing self._benchmark_version = None - Reset state at START of parse_runs() to prevent leakage - Extract version from CSV comments (# Results version: v1.1.2743) - Override build_test_info() to use benchmark version with fallback Tests verify: - Version extraction from CSV comments - Fallback to wrapper version when missing - State reset between parse calls Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Following pattern from coremark_processor (RPOPC-1319), extract Phoronix benchmark version from CSV comments and store in test.version instead of conflating with wrapper version. Changes: - Add __init__ to store _benchmark_version state - Reset state at start of parse_runs() to prevent leakage - Extract version from CSV comment "# Results version: v10.8.1" - Override build_test_info() to use benchmark version with wrapper fallback Tests verify: - Benchmark version extracted to test.version - Wrapper version preserved in test.wrapper_version - Fallback to wrapper version when benchmark version missing - State reset between parse_runs() calls prevents leakage Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

coderabbitai · 2026-06-16T04:25:26Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 08b39b7d-5592-4f5d-ade4-2934239c345f

📥 Commits

Reviewing files that changed from the base of the PR and between 0e6029a and 2824a49.

📒 Files selected for processing (14)

src/chronicler/processors/autohpl_processor.py
src/chronicler/processors/coremark_pro_processor.py
src/chronicler/processors/passmark_processor.py
src/chronicler/processors/phoronix_processor.py
src/chronicler/processors/speccpu2017_processor.py
src/chronicler/processors/specjbb_processor.py
src/chronicler/processors/uperf_processor.py
tests/test_autohpl_version_extraction.py
tests/test_coremark_pro_timestamps.py
tests/test_passmark_version_extraction.py
tests/test_phoronix_version_extraction.py
tests/test_speccpu2017_version_extraction.py
tests/test_specjbb_version.py
tests/test_uperf_version.py

📝 Walkthrough

Summary by CodeRabbit

New Features
- Benchmark processors now extract version information directly from benchmark output files (AutoHPL, CoreMark Pro, Passmark, Phoronix, SpecCPU2017, SpecJBB, Uperf) and use this information in test metadata.
- Added fallback mechanisms to use wrapper version information when benchmark version extraction is unavailable.
Tests
- Added comprehensive test coverage for benchmark version extraction and fallback behavior across all updated processors.

Walkthrough

Seven benchmark processors (AutoHPLProcessor, CoreMarkProProcessor, PassmarkProcessor, PhoronixProcessor, SpecCPU2017Processor, SpecJBBProcessor, UperfProcessor) each gain a uniform pattern: an instance field (_benchmark_version or equivalent) initialized to None, reset at the start of each parse_runs, populated by a new extraction helper reading CSV comment lines or a dedicated version file, and used in an overridden build_test_info that sets TestInfo.version with fallback to the base wrapper version. New unit test files cover all four behavioral cases for each processor.

Changes

Benchmark version extraction — all processors

Layer / File(s)	Summary
State init and per-parse reset `src/chronicler/processors/autohpl_processor.py`, `src/chronicler/processors/coremark_pro_processor.py`, `src/chronicler/processors/passmark_processor.py`, `src/chronicler/processors/phoronix_processor.py`, `src/chronicler/processors/speccpu2017_processor.py`, `src/chronicler/processors/specjbb_processor.py`, `src/chronicler/processors/uperf_processor.py`	Each processor's `__init__` initializes `_benchmark_version` (or equivalent) to `None`, and each `parse_runs` resets it before extraction to prevent cross-call leakage.
Extraction helpers, parse_runs call sites, and build_test_info overrides `src/chronicler/processors/autohpl_processor.py`, `src/chronicler/processors/coremark_pro_processor.py`, `src/chronicler/processors/passmark_processor.py`, `src/chronicler/processors/phoronix_processor.py`, `src/chronicler/processors/speccpu2017_processor.py`, `src/chronicler/processors/specjbb_processor.py`, `src/chronicler/processors/uperf_processor.py`	Each processor adds `_extract_benchmark_version_from_csv` (or reads a `version` file / YML block) using `re` to match `Results version:` in comment lines, invokes it from `parse_runs`, and overrides `build_test_info` to set `TestInfo.version` from the extracted value (falling back to wrapper version) while preserving `wrapper_version`.
Unit tests for all seven processors `tests/test_autohpl_version_extraction.py`, `tests/test_coremark_pro_timestamps.py`, `tests/test_passmark_version_extraction.py`, `tests/test_phoronix_version_extraction.py`, `tests/test_speccpu2017_version_extraction.py`, `tests/test_specjbb_version.py`, `tests/test_uperf_version.py`	New and extended test files verify extraction sets `test_info.version`, fallback to wrapper version when the source is absent, `wrapper_version` is `"unknown"` when wrapper metadata is missing, and state resets between consecutive `parse_runs` calls on the same instance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing 7 remaining processors to support version extraction.
Description check	✅ Passed	The description is well-structured, directly related to the changeset, and comprehensively explains the implementation across all affected processors.
Docstring Coverage	✅ Passed	Docstring coverage is 84.48% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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 and usage tips.}

grdumas

PR Review: RPOPC-1320: Fix remaining 7 processors version extraction

Summary

This PR successfully replicates the version conflation fix across the remaining 7 processors. The pattern of extracting the benchmark version, resetting the state to prevent leakage, and overriding build_test_info() has been applied consistently and correctly.

Critical Issues (MUST FIX)

None found.

Major Issues (SHOULD FIX)

None found.

Minor Issues (NICE TO HAVE)

None found.

Nitpicks (OPTIONAL)

None found.

Positive Notes

Consistent Implementation: The implementation strategy across all 7 processors perfectly matches the established patterns from PR #49 (STREAMS), PR #50 (FIO), and PR #51 (CoreMark).
Proactive State Management: Crucially, the state reset (e.g., self._autohpl_version = None, self._passmark_version = None) is correctly positioned at the top of the parse_runs() method in every updated processor, preventing the stale data leakage issue discovered in earlier reviews.
Exhaustive Testing: The addition of 25 new tests explicitly validating the extraction, fallback, and state-reset behaviors for each processor demonstrates excellent rigor. All 306 tests pass.

Overall Assessment

Status: APPROVE
Reasoning: The implementation is highly consistent, defensive against state leakage, and thoroughly tested. This effectively completes the version conflation fix epic.
Next Steps: Ready to merge.

Reviewed by: Gemini Pro via automated code review

grdumas

LGTM

Agent VM and others added 6 commits June 16, 2026 00:24

grdumas self-assigned this Jun 16, 2026

grdumas added the bug Something isn't working label Jun 16, 2026

grdumas commented Jun 16, 2026

View reviewed changes

grdumas merged commit 5d5fa22 into main Jun 16, 2026
2 checks passed

grdumas deleted the fix/RPOPC-1320-remaining-processors-version-extraction branch June 16, 2026 04:33

grdumas mentioned this pull request Jun 17, 2026

Add benchmark version extraction for remaining affected processors #45

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RPOPC-1320: Fix remaining 7 processors version extraction#52

RPOPC-1320: Fix remaining 7 processors version extraction#52
grdumas merged 6 commits into
mainfrom
fix/RPOPC-1320-remaining-processors-version-extraction

grdumas commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Uh oh!

grdumas left a comment

Uh oh!

grdumas left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

grdumas commented Jun 16, 2026

Summary

Acceptance Criteria

Changes

Group 1 - Partially Affected (version already in config)

Group 2 - Fully Affected (version extraction required)

Implementation Pattern

Testing

Technical Details

Related

Files Changed

Uh oh!

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

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Uh oh!

grdumas left a comment

Choose a reason for hiding this comment

PR Review: RPOPC-1320: Fix remaining 7 processors version extraction

Summary

Critical Issues (MUST FIX)

Major Issues (SHOULD FIX)

Minor Issues (NICE TO HAVE)

Nitpicks (OPTIONAL)

Positive Notes

Overall Assessment

Uh oh!

grdumas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading