Skip to content

feat: sort results#59

Merged
GabrielePicco merged 3 commits intomainfrom
dode/sorted-bench-results
Apr 14, 2026
Merged

feat: sort results#59
GabrielePicco merged 3 commits intomainfrom
dode/sorted-bench-results

Conversation

@Dodecahedr0x
Copy link
Copy Markdown
Contributor

@Dodecahedr0x Dodecahedr0x commented Apr 9, 2026

Sorts keys of the benchmark result to avoid useless diffs

Summary by CodeRabbit

  • Chores
    • Normalized JSON output from test runs so generated metrics are consistently ordered and written atomically.
    • Ensured test metric files end with a corrected trailing newline.
    • Updated baseline metrics to reflect a revised set of tracked compute-unit operations and adjusted metric values.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5b8b704d-9857-436d-857b-b3d5eb28205f

📥 Commits

Reviewing files that changed from the base of the PR and between 3628dab and 6019d2c.

📒 Files selected for processing (1)
  • metrics/baseline.json

Walkthrough

Added JSON normalization to Makefile test targets (runs jq -S . and writes via a *.tmp atomic move) and updated metrics/baseline.json by removing many old metric keys and adding/reordering new metric keys; metric values for unchanged keys remain the same. (50 words)

Changes

Cohort / File(s) Summary
Build configuration
Makefile
Added JSON normalization to test-current and test-baseline: after cargo test-sbf with METRICS_PATH=$(CURRENT)/$(BASELINE), the generated JSON is piped through jq -S . and written atomically via a *.tmp file moved back to the original path.
Metrics baseline
metrics/baseline.json
Reorganized metric keys: removed many prior tq_auto::*, dep_queue::*, del_*, lamports_pda::*, and other entries; introduced new/renamed keys such as close_eata::close, create_eata_perm::{default,permissionless}, various del_* and del_*_perm variants, new dep_queue::* entries, and lamports_pda::sponsored_lamports_transfer. Several remaining keys have updated values; file now ends with a trailing newline.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ 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 'feat: sort results' directly relates to the main change: adding JSON normalization (sorting) steps to benchmark results via jq -S to produce consistent, diffable output.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dode/sorted-bench-results

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Makefile`:
- Around line 23-28: Add a runtime prerequisite check for the jq binary to the
Makefile targets that use it (test-current and test-baseline): detect if jq is
available before running the jq formatting step and if not print a clear error
message explaining "jq not found" and how to install it (e.g., apt/brew/pacman)
and exit non-zero; update the jq invocation blocks in the test-current and
test-baseline target recipes to first run a simple command like "command -v jq
>/dev/null 2>&1 || { echo 'jq is required...'; exit 1; }" so developers get a
helpful actionable message instead of "jq: command not found."
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b14abcac-8ace-4f3a-a8a1-c3314bf3de6b

📥 Commits

Reviewing files that changed from the base of the PR and between 569b053 and 9e25fbf.

📒 Files selected for processing (2)
  • Makefile
  • metrics/baseline.json

Comment thread Makefile
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@metrics/baseline.json`:
- Around line 146-180: The tq_auto::* entries are not in lexicographic order
(e.g., "tq_auto::tick_empty" appears before "tq_auto::crank_2",
"tq_auto::enqueue_delayed", "tq_auto::schedule", etc.); reorder the entire
tq_auto::* block so its keys are sorted lexicographically (ensure
"tq_auto::crank_2", "tq_auto::crank_bad_magic", "tq_auto::enqueue_delayed",
"tq_auto::enqueue_ready", "tq_auto::enqueue_with_client_ref_id",
"tq_auto::run_scheduled_tick", "tq_auto::run_scheduled_tick_via_magic_bundle",
"tq_auto::schedule", "tq_auto::schedule_with_client_ref_id",
"tq_auto::tick_after_bundle", "tq_auto::tick_empty", etc.), rerun your jq -S
normalization to verify no further reordering occurs, and commit the canonical
sorted JSON.
- Around line 59-60: This JSON contains duplicate metric keys (e.g.,
"del_tq::delegate", "dep_queue::return_to_shuttle",
"dep_queue::reject_zero_split", "tq_auto::tick_empty") which silently overwrite
earlier entries; remove the duplicate definitions and regenerate the baseline so
each metric key appears exactly once, ensuring the final object retains the
intended values for those metric keys and that only the first (or canonical)
occurrence remains for each of "del_tq::delegate",
"dep_queue::return_to_shuttle", "dep_queue::reject_zero_split", and
"tq_auto::tick_empty".
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 40c49424-56be-46f4-9c07-0e140e71747d

📥 Commits

Reviewing files that changed from the base of the PR and between 9e25fbf and 3628dab.

📒 Files selected for processing (1)
  • metrics/baseline.json

Comment thread metrics/baseline.json Outdated
Comment thread metrics/baseline.json Outdated
@GabrielePicco GabrielePicco merged commit 7bbe06f into main Apr 14, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants