Skip to content

Add role-based model eval workload#20

Merged
dzarlax merged 1 commit into
mainfrom
codex/role-based-model-evals
Jun 16, 2026
Merged

Add role-based model eval workload#20
dzarlax merged 1 commit into
mainfrom
codex/role-based-model-evals

Conversation

@dzarlax

@dzarlax dzarlax commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • replace the ad hoc eval workload with a production-shaped role-based suite
  • add grouped content expectations, tool argument checks, max tool calls, and max response length checks
  • show case purpose, limitations, and new expectation fields in the Evals UI

Tests

  • go test -count=1 ./...

Summary by CodeRabbit

  • New Features

    • Enhanced evaluation suite (v2) with expanded diagnostic cases and improved tool-routing validation
    • Added new evaluation constraints including synonym matching, tool argument verification, and response length limits
    • Upgraded admin dashboard to display richer evaluation metadata, case purposes, and detailed expectation criteria
  • Documentation

    • Updated evaluation suite documentation clarifying role-fit scope and synthetic tool usage

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The evalpack schema is extended with Purpose, Limitations, MustContainAny, ToolArgs, MaxToolCalls, and MaxChars fields. Validation, evaluation logic, and helper functions are updated accordingly. The workload suite is rewritten as v2 with role-based conversational diagnostics and six tool-routing cases. The admin UI template and README documentation are updated to reflect the new fields.

Changes

Evalpack v2: schema, logic, workloads, and UI

Layer / File(s) Summary
Case/Expect schema extension and ValidateSuite enforcement
internal/evalpack/evalpack.go
Case gains Purpose and Limitations; Expect gains MustContainAny, ToolArgs, MaxToolCalls, and MaxChars. ValidateSuite rejects negative limits, requires tool_call when tool_args is set, and requires non-empty MustContainAny groups.
Evaluate enforcement logic and helpers
internal/evalpack/evalpack.go
Evaluate adds MustContainAny group matching, tool-argument JSON key/value comparison, MaxToolCalls count enforcement, and MaxChars rune-count enforcement. New helpers containsAny, findToolCall, evaluateToolArgs, actualArgString replace the removed hasToolCall.
evalpack tests for new constraints
internal/evalpack/evalpack_test.go
TestLoadWorkloadSuite asserts suite name, version, case count, and first-case Purpose. TestEvaluateContentExpectations adds MustContainAny and MaxChars sub-cases. New TestEvaluateToolArgsExpectation verifies argument matching, mismatch detection, and MaxToolCalls enforcement.
Workload suite v2: conversational and tool-routing cases
evals/workload.json
Suite renamed to personal-assistant-role-workloads at version 2. Conversational diagnostics use must_contain_any, must_not_contain, no_tool_call, and max_chars. Six tool-routing cases target calendar__get_events, tasks__get_tasks, personal-memory__recall_facts, health-dashboard__get_sleep_summary, fs_search, and web_search with explicit tool_args and max_tool_calls: 1.
Admin UI rendering and suite documentation
internal/adminapi/templates/evals.html, evals/README.md
evals.html adds a "manual only" badge, conditionally renders Purpose and Limitations per case, and extends expectations with MustContainAny, ToolArgs, MaxToolCalls, and MaxChars. README.md documents suite scope, model eligibility, expectation types, and synthetic-tool-only policy.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Hoppy little runner checks each tool call tight,
Synonyms and arg maps brought into the light.
Version two is planted, role-based and clean,
The workload JSON bloomed where blank fields had been.
No real calendars harmed — synthetic all the way! 🗓️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add role-based model eval workload' directly and clearly summarizes the main change: introducing a role-based evaluation workload to replace the existing ad hoc suite.
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/role-based-model-evals

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.

@dzarlax dzarlax merged commit ed3f6e5 into main Jun 16, 2026
5 of 6 checks passed
@dzarlax dzarlax deleted the codex/role-based-model-evals branch June 16, 2026 16:04

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da09c33102

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread evals/workload.json
"category": "admin_debug",
"system_prompt": "Act as a pragmatic senior engineer. Give the next diagnostic step first.",
"prompt": "После деплоя админка открывается, но список моделей пустой. Что проверишь первым?",
"id": "multimodal-absent-image",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Increase the admin timeout for the expanded eval suite

With this suite now at 13 sequential cases, the admin path still wraps the whole run in modelEvalTimeout (45s) while runModelEval gives each case up to 12s. Any model averaging more than about 3.5s per case, or one slow tool case plus normal latency, will hit the outer context deadline and turn later cases into timeout failures unrelated to model quality; because the final save uses that same context, the UI can also be left with the earlier running status instead of the completed report.

Useful? React with 👍 / 👎.

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