Skip to content

Fix list of latest benchmarks - #1009

Closed
aantn wants to merge 2 commits into
masterfrom
benchmark-improvements
Closed

aantn wants to merge 2 commits into
masterfrom
benchmark-improvements

Conversation

@aantn

@aantn aantn commented Sep 29, 2025

Copy link
Copy Markdown
Collaborator

By using an mkdocs plugin to generate dynamically instead of relying on hardcoded markdown links. This also requires changing navigation across the docs due to the new plugin

@aantn
aantn requested a review from Sheeproid September 29, 2025 09:27
@coderabbitai

coderabbitai Bot commented Sep 29, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds timestamped historical evaluation report generation to CI and updates the report script to set headers based on filename. Overhauls MkDocs navigation to use mkdocs-awesome-nav with distributed .nav.yml files across docs sections. Updates evaluation docs (latest, history index, specific historical result) and fixes internal links.

Changes

Cohort / File(s) Summary of Changes
CI: Benchmark report history
.github/workflows/eval-benchmarks.yaml
Generates an additional timestamped historical report alongside latest results; sets TIMESTAMP and writes to docs/development/evaluations/history/results_.md.
Report generator logic
scripts/generate_eval_report.py
Adds conditional header generation: if output filename matches results_YYYYMMDD_HHMMSS.md, title is formatted date; otherwise uses existing static header.
MkDocs config overhaul
mkdocs.yml, pyproject.toml
Adds awesome-nav plugin and removes central nav from mkDocs config; adds mkdocs-awesome-nav dependency (regular and dev).
Docs nav: site root
docs/.nav.yml
Introduces top-level navigation structure for the site.
Docs nav: sections
docs/ai-providers/.nav.yml, docs/data-sources/.nav.yml, docs/data-sources/builtin-toolsets/.nav.yml, docs/development/.nav.yml, docs/development/evaluations/.nav.yml, docs/development/evaluations/history/.nav.yml, docs/installation/.nav.yml, docs/reference/.nav.yml, docs/walkthrough/.nav.yml
Adds per-section navigation files defining indices and pages for each area.
Evaluations docs content
docs/development/evaluations/index.md, docs/development/evaluations/latest-results.md, docs/development/evaluations/history/index.md, docs/development/evaluations/history/results_20250928_001434.md
Updates links to history index; retitles latest results and updates timestamps/judge model; simplifies history index content; adds/updates a timestamped historical results page.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as Developer
    participant GH as GitHub Actions (eval-benchmarks)
    participant Script as generate_eval_report.py
    participant Docs as docs/development/evaluations/...

    Dev->>GH: Push/Trigger workflow
    GH->>Script: Generate latest-results.md
    Script-->>Docs: Write latest-results.md (static header)
    GH->>GH: Set TIMESTAMP (YYYYMMDD_HHMMSS)
    GH->>Script: Generate results_<TIMESTAMP>.md
    Script-->>Docs: Write history/results_<TIMESTAMP>.md (date-based header)
Loading
sequenceDiagram
    autonumber
    participant WF as Workflow step
    participant RG as Report Generator
    participant FS as Filesystem

    WF->>RG: Invoke with output_path
    alt output filename matches results_YYYYMMDD_HHMMSS.md
        RG->>RG: Parse date from filename
        RG->>FS: Write header "Month Day, Year - HH:MM:SS"
    else default case
        RG->>FS: Write static header "Latest Benchmark Results"
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • Sheeproid
  • arikalon1

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly states that the pull request fixes the list of latest benchmarks, which directly reflects the primary change of replacing hardcoded links with a dynamic list generator and is concise enough for a reviewer to understand the main intent at a glance.
Description Check ✅ Passed The description clearly relates to the changeset by explaining that an mkdocs plugin is being used for dynamic generation of documentation links and that this requires updating the navigation configuration, matching the modifications in the pull request.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch benchmark-improvements

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4327725 and adb9dc7.

⛔ Files ignored due to path filters (1)
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .github/workflows/eval-benchmarks.yaml (1 hunks)
  • docs/.nav.yml (1 hunks)
  • docs/ai-providers/.nav.yml (1 hunks)
  • docs/data-sources/.nav.yml (1 hunks)
  • docs/data-sources/builtin-toolsets/.nav.yml (1 hunks)
  • docs/development/.nav.yml (1 hunks)
  • docs/development/evaluations/.nav.yml (1 hunks)
  • docs/development/evaluations/history/.nav.yml (1 hunks)
  • docs/development/evaluations/history/index.md (1 hunks)
  • docs/development/evaluations/history/results_20250928_001434.md (1 hunks)
  • docs/development/evaluations/index.md (2 hunks)
  • docs/development/evaluations/latest-results.md (1 hunks)
  • docs/installation/.nav.yml (1 hunks)
  • docs/reference/.nav.yml (1 hunks)
  • docs/walkthrough/.nav.yml (1 hunks)
  • mkdocs.yml (1 hunks)
  • pyproject.toml (1 hunks)
  • scripts/generate_eval_report.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

In MkDocs docs, always add a blank line between a header/bold text and a following list to render correctly

Files:

  • docs/development/evaluations/history/index.md
  • docs/development/evaluations/latest-results.md
  • docs/development/evaluations/index.md
  • docs/development/evaluations/history/results_20250928_001434.md
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

Define and maintain the canonical list of pytest markers in pyproject.toml

Files:

  • pyproject.toml
🧠 Learnings (1)
📚 Learning: 2025-07-02T10:27:17.231Z
Learnt from: Sheeproid
PR: robusta-dev/holmesgpt#586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.

Applied to files:

  • docs/development/evaluations/history/results_20250928_001434.md
🪛 Ruff (0.13.1)
scripts/generate_eval_report.py

1364-1364: Unpacked variable hour is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


1364-1364: Unpacked variable minute is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


1364-1364: Unpacked variable second is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build (3.12)
  • GitHub Check: build (3.11)
  • GitHub Check: build (3.10)
  • GitHub Check: llm_evals
🔇 Additional comments (3)
docs/development/evaluations/latest-results.md (1)

1-6: Nice refresh on the latest-results header and metadata.

The new title, timestamp, and judge model line accurately reflect the most recent GPT‑4.1-based run and keep the page aligned with OpenAI’s current release cadence. (openai.com)

docs/development/evaluations/index.md (1)

69-69: Updated history links look good.

Linking directly to ./history/index.md matches the new navigation layout and keeps the docs consistent.

Also applies to: 89-89

mkdocs.yml (1)

59-59: Good call enabling awesome-nav.

The plugin expects the nav block to be omitted so it can source .nav.yml files, and listing it here is exactly how the upstream docs recommend activating it. (github.com)

Comment on lines +1359 to +1368
match = re.match(
r"results_(\d{4})(\d{2})(\d{2})_(\d{2})(\d{2})(\d{2})\.md", output_filename
)
if match:
# Extract date components and format as title
year, month, day, hour, minute, second = match.groups()
date_obj = datetime(int(year), int(month), int(day))
# Format as "Month Day, Year" (no time)
title = date_obj.strftime("%B %d, %Y")
report_lines.append(f"# {title}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix unused timestamp captures to satisfy lint.

Ruff is flagging the unpacked hour, minute, and second values because we never use them. This will fail CI unless we either prefix them with underscores or remove them entirely.

-        year, month, day, hour, minute, second = match.groups()
+        year, month, day, _hour, _minute, _second = match.groups()

Based on static analysis hints.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
match = re.match(
r"results_(\d{4})(\d{2})(\d{2})_(\d{2})(\d{2})(\d{2})\.md", output_filename
)
if match:
# Extract date components and format as title
year, month, day, hour, minute, second = match.groups()
date_obj = datetime(int(year), int(month), int(day))
# Format as "Month Day, Year" (no time)
title = date_obj.strftime("%B %d, %Y")
report_lines.append(f"# {title}")
match = re.match(
r"results_(\d{4})(\d{2})(\d{2})_(\d{2})(\d{2})(\d{2})\.md", output_filename
)
if match:
# Extract date components and format as title
year, month, day, _hour, _minute, _second = match.groups()
date_obj = datetime(int(year), int(month), int(day))
# Format as "Month Day, Year" (no time)
title = date_obj.strftime("%B %d, %Y")
report_lines.append(f"# {title}")
🧰 Tools
🪛 Ruff (0.13.1)

1364-1364: Unpacked variable hour is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


1364-1364: Unpacked variable minute is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


1364-1364: Unpacked variable second is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

🤖 Prompt for AI Agents
In scripts/generate_eval_report.py around lines 1359 to 1368, the regex match
unpacks six capture groups but only uses the first three (year, month, day),
causing lint errors for unused variables; change the unpack to ignore the unused
time captures (e.g., year, month, day, *_ = match.groups() or year, month, day,
_, _, _ = match.groups()) or extract only the first three groups via
match.groups()[:3], then proceed to construct the date_obj and title as before.

@github-actions

Copy link
Copy Markdown
Contributor

Results of HolmesGPT evals

  • ask_holmes: 34/36 test cases were successful, 1 regressions, 1 setup failures
Test suite Test case Status
ask 01_how_many_pods ✅
ask 02_what_is_wrong_with_pod ✅
ask 04_related_k8s_events ✅
ask 05_image_version ✅
ask 09_crashpod ✅
ask 10_image_pull_backoff ✅
ask 110_k8s_events_image_pull ✅
ask 11_init_containers ✅
ask 13a_pending_node_selector_basic ✅
ask 14_pending_resources ✅
ask 15_failed_readiness_probe ✅
ask 17_oom_kill ✅
ask 19_detect_missing_app_details ✅
ask 20_long_log_file_search ✅
ask 24_misconfigured_pvc ✅
ask 24a_misconfigured_pvc_basic ✅
ask 28_permissions_error 🚧
ask 33_cpu_metrics_discovery ❌
ask 39_failed_toolset ✅
ask 41_setup_argo ✅
ask 42_dns_issues_steps_new_tools ✅
ask 43_current_datetime_from_prompt ✅
ask 45_fetch_deployment_logs_simple ✅
ask 51_logs_summarize_errors ✅
ask 53_logs_find_term ✅
ask 54_not_truncated_when_getting_pods ✅
ask 59_label_based_counting ✅
ask 60_count_less_than ✅
ask 61_exact_match_counting ✅
ask 63_fetch_error_logs_no_errors ✅
ask 79_configmap_mount_issue ✅
ask 83_secret_not_found ✅
ask 86_configmap_like_but_secret ✅
ask 93_calling_datadog[0] ✅
ask 93_calling_datadog[1] ✅
ask 93_calling_datadog[2] ✅

Legend

  • ✅ the test was successful
  • :minus: the test was skipped
  • ⚠️ the test failed but is known to be flaky or known to fail
  • 🚧 the test had a setup failure (not a code regression)
  • 🔧 the test failed due to mock data issues (not a code regression)
  • 🚫 the test was throttled by API rate limits/overload
  • ❌ the test failed and should be fixed before merging the PR

@aantn

aantn commented Sep 30, 2025

Copy link
Copy Markdown
Collaborator Author

Closing, replaced by #1017 which includes all changes here.

@aantn aantn closed this Sep 30, 2025
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.

1 participant