feat(oak): remove internal data, add policies with target_tools regression guard#97
Conversation
Remove proprietary Oak Health Insurance data files (data.py, models.py, main.py, oak_policies.py, oak_health_test_suite_v1_backup.json) and replace with the public cuga-project/oak-bench package. - Add cuga-oak-health dependency (oak_health FastAPI app from oak-bench) - Replace oak_health_test_suite_v1.json with the public version (1077 lines vs 720; adds more test cases and is sourced from oak-bench) - Update eval.sh/run_app.sh to launch server via installed package (uvicorn oak_health.main:app) instead of local main.py - Remove oak_policies import and policy loading from eval_bench_sdk.py (no public equivalent; eval now runs without custom policies) Closes #53 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Source the test suite from cuga-project/oak-bench instead of the internal version. The public suite has 1077 lines (vs 720) and exercises the same capabilities with anonymized/approved test data. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Playbooks for claims/EOB PDFs, care providers, benefits, payments, family members, and plan information tasks - Tool guides (NL triggers + AlwaysTrigger) for all oak tools (coverage, search_benefits, find_care_specialty, etc.) - `get_all_oak_policies()` helper returning full policy list Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add --agent flag (cuga|react) routing to setup_react_agent_for_evaluation - Load oak policies on setup via _load_oak_policies() (skippable with --no-policy) - Wire MetricsConfig with enable_api_metrics=True for both agent paths - Prefix result files with react_oak_health_ when running react agent - Name experiment per agent type for Langfuse tracing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…compare.sh eval.sh: - Parse --agent <name> and pass through to Python; default cuga - Bundle step now picks up react_oak_health_* result files too - Update --help output compare.sh: - Remove early error that blocked agent comparison - --compare-agents expands to cuga,react; builds cartesian product of model×agent CONFIGS - Pass --agent per config to each eval.sh invocation - Track results/trajectories keyed by model:agent config label Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
README: - Rewrite to reflect react agent support, --agent/--compare-agents flags - Add Metrics section documenting API call tracking fields - Add Policies section for oak_policies.py - Update file structure table to include new files - Remove emoji headings; drop stale manual multi-terminal setup steps config/oak_health_insurance.env: - Add DYNACONF_ADVANCED_FEATURES__REGISTRY=true (needed for policy loading) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds the external Oak Health package, a new benchmark suite and policy definitions, evaluator and shell-script updates, config-keyed comparison orchestration, and a restructured benchmark README. ChangesOak Health benchmark overhaul
Estimated code review effort: 4 (Complex) | ~75 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai can you run code review? |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
benchmarks/oak_health_insurance/eval_bench_sdk.py (1)
133-137: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
user_contextlocation format contradicts the policies and the new test suite — likely the root cause of provider-search task failures.The hardcoded context passes latitude/longitude:
Location: latitude(str):40.7128, longitude(str):-74.0060But
oak_health_test_suite_v1.json'suser_infospecifieslocation:[stateCode:NY, zipCode:11211], and every relevantoak_policies.pyplaybook/enrichment explicitly states the provider-search API "does NOT use latitude/longitude" and requiresstateCode/zipCodeextracted "from the member's user context".evaluate_all(lines 164-190) also never reads the suite'suser_infofield at all — it only extractstest_cases.This mismatch means the agent is given location data in a format the tools/policies explicitly say not to use, which would systematically break the provider-search tasks (
care_providers_mri,knee_surgery_providers,pcp_near_me,pcp_accept_new_patients,pcp_language,coverage_and_providers_mri_found_none,classic_hmo_provider_and_constraints) — plausibly accounting for several of the reported non-full-passes.🔧 Proposed fix
user_context = """ Member ID (string): 121231234 - Location: latitude(str):40.7128, longitude(str):-74.0060 + Location: stateCode:NY, zipCode:11211 Current Date: 2025-12-31 """🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/oak_health_insurance/eval_bench_sdk.py` around lines 133 - 137, The hardcoded user_context in eval_bench_sdk.py uses latitude/longitude, which conflicts with the provider-search policies and the test suite’s expected stateCode/zipCode format. Update the user_context construction (in the benchmark setup around the user_context assignment) to supply location in the same stateCode/zipCode structure used by oak_health_test_suite_v1.json and the relevant oak_policies.py playbooks, and make evaluate_all consume the suite’s user_info instead of ignoring it so the generated context matches the task requirements.
🧹 Nitpick comments (2)
benchmarks/oak_health_insurance/eval.sh (1)
181-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStatic analysis flags
ls -tfor filename parsing (SC2012).Low risk here since result filenames follow a controlled
{prefix}_{timestamp}.jsonpattern, butfind/stat-based sorting would be more robust long-term.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/oak_health_insurance/eval.sh` at line 181, The latest-result lookup in eval.sh uses ls -t, which trips static analysis and relies on filename parsing. Update the LATEST_RESULT selection logic in the result-discovery block to use a more robust find/stat-based ordering approach while preserving the existing oak_health_* and react_oak_health_* matching behavior, so the newest JSON file is chosen without shell-splitting issues.Source: Linters/SAST tools
benchmarks/oak_health_insurance/oak_policies.py (1)
77-509: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffFind-care-provider workflow content is duplicated across three playbooks plus two tool-guide enrichments.
The stateCode/zipCode kwarg requirements, specialty-category fallback codes ("75"/"220"/"25"/"231"), and pagination rules are repeated nearly verbatim in
create_find_care_providers_playbook,create_benefits_with_providers_playbook,create_coverage_and_providers_playbook,create_find_care_specialty_enrichment, andcreate_find_care_suggestions_enrichment. Any future change to these codes/fallbacks (e.g., adding a new specialty code) needs to be applied consistently in 5 places, which is error-prone.Consider centralizing the shared provider-search guidance (location format, fallback codes, pagination loop) into the tool-guide enrichments only, and have the playbooks reference that behavior rather than re-stating it in full.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/oak_health_insurance/oak_policies.py` around lines 77 - 509, The provider-search guidance is duplicated across the playbooks and tool-guide enrichments, making the stateCode/zipCode kwarg rules, specialty fallback codes, and pagination behavior hard to keep consistent. Move the shared provider-search instructions into the existing find_care_suggestions and find_care_specialty enrichment helpers, then trim create_find_care_providers_playbook, create_benefits_with_providers_playbook, and create_coverage_and_providers_playbook to reference those behaviors instead of restating them. Keep the unique workflow-specific steps in each playbook, but remove the repeated location/fallback/pagination prose so the shared symbols stay the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/oak_health_insurance/compare.sh`:
- Around line 97-116: The new --compare-agents flow in compare.sh is broken
because it now includes react even though eval.sh still only supports cuga and
exits otherwise. Update compare.sh and/or the compare-agents handling so it does
not schedule react configs until eval.sh can run them, and keep the
configuration logic around COMPARE_AGENTS, AGENTS, and the CONFIGS/CONFIG_LABELS
build in sync with what eval.sh actually accepts.
In `@benchmarks/oak_health_insurance/eval_bench_sdk.py`:
- Around line 93-123: Skip Oak policy loading when the agent is the ReAct
variant. In `EvalBenchSDK.setup`, only call `_load_oak_policies()` for the Cuga
agent path (or guard it with `hasattr(self.agent, "policies")`) so
`setup_react_agent_for_evaluation()` / `GenericReactAgent` does not enter
`add_policy_via_agent` and emit one warning per policy. Keep
`_load_oak_policies` unchanged except for assuming it runs only on agents that
can actually store policies.
In `@benchmarks/oak_health_insurance/eval.sh`:
- Line 69: The eval.sh flow parses AGENT but never forwards it to
eval_bench_sdk.py, so the selected agent stays at the default; update the
eval_bench_sdk.py invocation to pass --agent using the AGENT value, or remove
the unused AGENT default if it should not be configurable. Check the shell
script’s main Python call and keep PASSTHROUGH_ARGS intact while ensuring AGENT
is explicitly included.
In `@benchmarks/oak_health_insurance/oak_policies.py`:
- Around line 290-309: The playbook text in
create_benefits_with_providers_playbook has two sections labeled Step 4, which
makes the sequence ambiguous. Update the second “Step 4: Format Combined
Response” heading to Step 5 and keep the surrounding provider/benefit
instructions unchanged so the workflow remains strictly ordered.
In `@benchmarks/oak_health_insurance/README.md`:
- Around line 90-93: The README’s result-file note is inconsistent with the
supported invocation path for the react agent. Update the documentation around
the results/ output and the eval.sh/compare.sh workflow so it clearly states how
react results are actually produced, using the react agent references and the
result filename examples to guide readers to the correct entry point. If react
is only available through compare.sh or direct Python execution, make that
explicit and avoid implying it can be generated via the rejected --agent react
path.
- Around line 72-88: The README examples are currently inconsistent with the
actual CLI behavior because eval.sh rejects any --agent value other than cuga;
update the documentation to remove or replace the invalid react examples, or
change eval.sh so the --agent handling in its validation and dispatch logic can
forward react to the Python evaluator. Check the eval.sh argument validation and
execution flow, plus the README example block, to ensure the documented commands
match what the script באמת accepts.
---
Outside diff comments:
In `@benchmarks/oak_health_insurance/eval_bench_sdk.py`:
- Around line 133-137: The hardcoded user_context in eval_bench_sdk.py uses
latitude/longitude, which conflicts with the provider-search policies and the
test suite’s expected stateCode/zipCode format. Update the user_context
construction (in the benchmark setup around the user_context assignment) to
supply location in the same stateCode/zipCode structure used by
oak_health_test_suite_v1.json and the relevant oak_policies.py playbooks, and
make evaluate_all consume the suite’s user_info instead of ignoring it so the
generated context matches the task requirements.
---
Nitpick comments:
In `@benchmarks/oak_health_insurance/eval.sh`:
- Line 181: The latest-result lookup in eval.sh uses ls -t, which trips static
analysis and relies on filename parsing. Update the LATEST_RESULT selection
logic in the result-discovery block to use a more robust find/stat-based
ordering approach while preserving the existing oak_health_* and
react_oak_health_* matching behavior, so the newest JSON file is chosen without
shell-splitting issues.
In `@benchmarks/oak_health_insurance/oak_policies.py`:
- Around line 77-509: The provider-search guidance is duplicated across the
playbooks and tool-guide enrichments, making the stateCode/zipCode kwarg rules,
specialty fallback codes, and pagination behavior hard to keep consistent. Move
the shared provider-search instructions into the existing find_care_suggestions
and find_care_specialty enrichment helpers, then trim
create_find_care_providers_playbook, create_benefits_with_providers_playbook,
and create_coverage_and_providers_playbook to reference those behaviors instead
of restating them. Keep the unique workflow-specific steps in each playbook, but
remove the repeated location/fallback/pagination prose so the shared symbols
stay the single source of truth.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2c7f4ba-ed34-4ac9-8165-d3b8674b9be1
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (9)
benchmarks/oak_health_insurance/README.mdbenchmarks/oak_health_insurance/compare.shbenchmarks/oak_health_insurance/config/oak_health_insurance.envbenchmarks/oak_health_insurance/eval.shbenchmarks/oak_health_insurance/eval_bench_sdk.pybenchmarks/oak_health_insurance/oak_health_test_suite_v1.jsonbenchmarks/oak_health_insurance/oak_policies.pybenchmarks/oak_health_insurance/run_app.shpyproject.toml
…format Oak evaluation is cuga-only — strip all react agent code: - eval_bench_sdk.py: remove --agent flag, agent_type param, react imports (evaluate_task_with_langfuse_react, setup_react_agent_for_evaluation), react branch in setup/evaluate_task/save_results; also fix user_context location from lat/lon to stateCode:NY, zipCode:11211 to match the test suite and provider-search policies - eval.sh: remove --agent flag, AGENT variable, react_oak_health glob - compare.sh: remove --compare-agents, --agents, AGENT_LIST, cartesian CONFIG product; revert to simple model-only loop (cuga agent hardcoded) - README.md: remove react references, agent-comparison examples, react result filename note; align option docs with actual CLI Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (4)
benchmarks/oak_health_insurance/README.md (1)
124-131: 📐 Maintainability & Code Quality | 🔵 TrivialClarify the
setup()reference for policy loading.The phrase "loaded automatically on
setup()" refers to a specific function that readers may not be able to locate. Consider rephrasing to "loaded automatically at evaluation startup" or naming the actual entry point (e.g.,eval_bench_sdk.py) to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/oak_health_insurance/README.md` around lines 124 - 131, The policy-loading description is ambiguous because `setup()` is not clearly identifiable from this README. Update the wording in the Policies section to reference the actual entry point that loads `oak_policies.py` (for example `eval_bench_sdk.py`) or rephrase it as loading automatically at evaluation startup, and keep the `--no-policy` behavior mention intact.benchmarks/oak_health_insurance/compare.sh (1)
155-155: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
findinstead oflsfor glob-based file snapshotting.Shellcheck flags both occurrences: parsing
lsoutput is fragile for filenames containing whitespace or special characters.🛠️ Suggested fix
- before_files=$(ls -1 "$RESULTS_DIR"/oak_health_*.json 2>/dev/null | sort) + before_files=$(find "$RESULTS_DIR" -maxdepth 1 -name 'oak_health_*.json' 2>/dev/null | sort)- after_files=$(ls -1 "$RESULTS_DIR"/oak_health_*.json 2>/dev/null | sort) + after_files=$(find "$RESULTS_DIR" -maxdepth 1 -name 'oak_health_*.json' 2>/dev/null | sort)Also applies to: 183-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/oak_health_insurance/compare.sh` at line 155, Replace the glob-based snapshotting in the benchmark script by using find instead of ls in both places flagged by Shellcheck. The issue is in the logic that collects the before/after JSON file lists in compare.sh; update the commands used for before_files and the matching later snapshot to safely enumerate "$RESULTS_DIR"/oak_health_*.json without parsing ls output. Keep the surrounding sort/filter behavior the same so the rest of the comparison flow still works.Source: Linters/SAST tools
benchmarks/oak_health_insurance/eval_bench_sdk.py (1)
96-112: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBroad exception swallowing can mask real config/import failures as harmless warnings.
If
oak_policiesfails to import (syntax error, missing dependency) orget_all_oak_policies()raises, this is caught and logged only aslogger.warning, and evaluation proceeds silently with zero policies loaded. Given the PR notesDYNACONF_ADVANCED_FEATURES__REGISTRYis required for policy loading, a misconfiguration here would silently degrade the benchmark (running without policy guidance) rather than failing loudly.Consider distinguishing "import/setup" failures (which likely indicate a real bug and should be more visible, e.g.
logger.erroror re-raise whenload_policies=True) from expected per-policy runtime failures, and/or assertingloaded > 0when policies were requested.♻️ Possible tightening
async def _load_oak_policies(self): """Load oak health insurance policies into the agent.""" - try: - from benchmarks.oak_health_insurance.oak_policies import get_all_oak_policies - - policies = get_all_oak_policies() - logger.info(f"Loading {len(policies)} oak policies...") - loaded = 0 - for policy in policies: - try: - await add_policy_via_agent(self.agent, policy) - loaded += 1 - except Exception as e: - logger.warning(f"Skipping policy '{policy.id}': {e}") - logger.info(f"✅ Loaded {loaded}/{len(policies)} policies") - except Exception as e: - logger.warning(f"Could not load oak policies: {e}") + from benchmarks.oak_health_insurance.oak_policies import get_all_oak_policies + + policies = get_all_oak_policies() + logger.info(f"Loading {len(policies)} oak policies...") + loaded = 0 + for policy in policies: + try: + await add_policy_via_agent(self.agent, policy) + loaded += 1 + except Exception as e: + logger.warning(f"Skipping policy '{policy.id}': {e}") + logger.info(f"✅ Loaded {loaded}/{len(policies)} policies") + if policies and loaded == 0: + logger.error("No oak policies could be loaded; check DYNACONF_ADVANCED_FEATURES__REGISTRY config")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/oak_health_insurance/eval_bench_sdk.py` around lines 96 - 112, The _load_oak_policies method is swallowing import/setup failures by catching all exceptions and only logging a warning, which can hide real configuration problems and let the benchmark continue with no policies loaded. Update _load_oak_policies to distinguish the initial import/get_all_oak_policies path from per-policy add_policy_via_agent failures: treat import/setup failures as errors (or re-raise when policy loading is requested), keep per-policy failures as warnings, and consider asserting that loaded > 0 when policies were expected so misconfiguration is surfaced early.benchmarks/oak_health_insurance/eval.sh (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new
--agentflag in usage/help text.
--agentis now supported and forwarded viaPASSTHROUGH_ARGS(lines 133-136), but the usage line and examples still only mention--task,--difficulty,--no-bundle,--bundle-zip,--model-profile. Consider adding--agent NAMEto the usage string and an example (e.g../eval.sh --agent react) so users discover the new option.Also applies to: 36-38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/oak_health_insurance/eval.sh` at line 25, The usage/help text in eval.sh is missing the newly supported --agent flag, so update the script’s usage string and examples to mention --agent NAME (for example, an example invocation like ./eval.sh --agent react). Make sure the help text stays consistent with the PASSTHROUGH_ARGS handling and that the documented options match the flags the script actually forwards.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@benchmarks/oak_health_insurance/compare.sh`:
- Line 155: Replace the glob-based snapshotting in the benchmark script by using
find instead of ls in both places flagged by Shellcheck. The issue is in the
logic that collects the before/after JSON file lists in compare.sh; update the
commands used for before_files and the matching later snapshot to safely
enumerate "$RESULTS_DIR"/oak_health_*.json without parsing ls output. Keep the
surrounding sort/filter behavior the same so the rest of the comparison flow
still works.
In `@benchmarks/oak_health_insurance/eval_bench_sdk.py`:
- Around line 96-112: The _load_oak_policies method is swallowing import/setup
failures by catching all exceptions and only logging a warning, which can hide
real configuration problems and let the benchmark continue with no policies
loaded. Update _load_oak_policies to distinguish the initial
import/get_all_oak_policies path from per-policy add_policy_via_agent failures:
treat import/setup failures as errors (or re-raise when policy loading is
requested), keep per-policy failures as warnings, and consider asserting that
loaded > 0 when policies were expected so misconfiguration is surfaced early.
In `@benchmarks/oak_health_insurance/eval.sh`:
- Line 25: The usage/help text in eval.sh is missing the newly supported --agent
flag, so update the script’s usage string and examples to mention --agent NAME
(for example, an example invocation like ./eval.sh --agent react). Make sure the
help text stays consistent with the PASSTHROUGH_ARGS handling and that the
documented options match the flags the script actually forwards.
In `@benchmarks/oak_health_insurance/README.md`:
- Around line 124-131: The policy-loading description is ambiguous because
`setup()` is not clearly identifiable from this README. Update the wording in
the Policies section to reference the actual entry point that loads
`oak_policies.py` (for example `eval_bench_sdk.py`) or rephrase it as loading
automatically at evaluation startup, and keep the `--no-policy` behavior mention
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 832b82b4-5f32-4ac2-98a0-d717feba25b5
📒 Files selected for processing (4)
benchmarks/oak_health_insurance/README.mdbenchmarks/oak_health_insurance/compare.shbenchmarks/oak_health_insurance/eval.shbenchmarks/oak_health_insurance/eval_bench_sdk.py
…e-internal-oak-health # Conflicts: # benchmarks/oak_health_insurance/eval_bench_sdk.py
- oak_policies.py: fix duplicate "Step 4" heading in create_benefits_with_providers_playbook (second section is Step 5) - eval_bench_sdk.py: stop _load_oak_policies from swallowing import/setup failures as warnings; only per-policy add failures stay non-fatal, and an error is logged if zero policies load - README.md: replace ambiguous "loaded automatically on setup()" with the actual entry point (eval_bench_sdk.py)
| echo "Options:" | ||
| echo " --task TASK Run a specific task by ID/name (e.g., 'approved_claims')" | ||
| echo " --difficulty LEVEL Filter by difficulty level (easy, medium, hard)" | ||
| echo " --no-policy Skip loading oak policies" |
There was a problem hiding this comment.
why replace --no-policies with --no-policy?
| DRY_RUN="${DRY_RUN:-false}" | ||
| OUTPUT_FILE="${OUTPUT_FILE:-}" | ||
| MODELS="${MODELS:-gpt-oss}" | ||
| AGENT="${AGENT:-cuga}" |
There was a problem hiding this comment.
why remove these flags? they are there for consistency; i.e. if someone adds --agent to appworld, then runs oak with --agent the error message should be specific to oak.
| ) | ||
| parser.add_argument( | ||
| "--no-policies", | ||
| "--no-policy", |
There was a problem hiding this comment.
why replace --no-policies with --no-policy?
| Options: | ||
| --task TASK Run a specific task by ID/name | ||
| --difficulty LEVEL Filter by difficulty (easy, medium, hard) | ||
| --no-policy Skip loading oak policies |
There was a problem hiding this comment.
why replace --no-policies with --no-policy?
| ./eval.sh # Default evaluation, all tasks | ||
| ./eval.sh --task approved_claims # Single task | ||
| ./eval.sh --difficulty easy # Filter by difficulty | ||
| ./eval.sh --no-policy # Skip policy loading |
There was a problem hiding this comment.
why replace --no-policies with --no-policy?
| ## 📊 Test Suite | ||
| ## Policies | ||
|
|
||
| Oak-specific playbooks and tool enrichments are defined in `oak_policies.py`. `eval_bench_sdk.py` loads them automatically at evaluation startup unless `--no-policy` is passed. |
There was a problem hiding this comment.
why replace --no-policies with --no-policy?
|
|
||
| for policy in policies: | ||
| await add_policy_via_agent(self.agent, policy) | ||
| async def _load_oak_policies(self): |
There was a problem hiding this comment.
policy load failures are swallowed, letting a policy-free run masquerade as a policy-enabled one.
_load_oak_policies wraps add_policy_via_agent in try/except, logs a warning, and continues; even loaded == 0 only produces a logger.error, not a non-zero exit. Compare to m3's _load_m3_policies which propagates these failures.
Failure scenario: DYNACONF_ADVANCED_FEATURES__REGISTRY misconfigured → all 23 policies skipped → the run completes and its results/bundle are recorded as a normal policy-enabled run, silently skewing comparisons. At minimum, loaded == 0 (with policies requested) should abort; ideally match m3 and let failures propagate.
| triggers=[ | ||
| AlwaysTrigger(type="always"), | ||
| ], | ||
| target_tools=["oak_health_insurance_get_coverage_period"], |
There was a problem hiding this comment.
These target_tools names never match at runtime: the registry names every oak tool by operationId (e.g. oak_health_insurance_search_benefits_search_benefits_post — see this suite's own tool_calls[].name) because the /plans, /plans/compare, /plans/{plan_id} routes share a first path segment, which flips the whole app to operationId naming. Guide matching is an exact tool.name in target_tools (cuga-agent policy/enactment.py:619), so all 14 enrichments load fine but never attach — the 85.9% result was effectively measured with enrichments off.
To confirm: ./eval.sh --task member_mri_benefit_search, then grep -r "Supported Inquiry Keywords" logging/trajectory_data/ — zero hits means the oak-enrich-search-benefits guide content never reached the tool description, even though the policy log shows it loaded.
Note that there are no tests that test actual tool guides load and are activated - only mocks, which are named to pass but this is a naming issue between the actual tool name in the agent and the policy trigger.
A cheap fix: after setup_agent_with_tools(), assert each guide's target_tools intersects the actual tool names and fail loudly otherwise (in _load_oak_policies, or as a small startup check). That turns this entire class of silent mismatch into an immediate error — including future recurrences
|
Summary of requested changes — two blockers, plus questions: Blocking:
Questions: the --no-policy rename breaks the --no-policies convention shared with m3/bpo, and the agent-flag removal in c41a0ec also dropped oak's fast-fail guard for --agent/--agents/--compare-agents — see inline comments. Also:
|
|
Thanks for the review @haroldship — addressed all of it. Summary of each comment and the fix: 1.
|
- oak_policies.py: correct all 14 tool-guide target_tools to the real operationId runtime names so enrichments actually attach at runtime. - eval_bench_sdk.py: propagate policy-load failures instead of swallowing them, and assert every guide's target_tools intersect the agent's actual tool names (fail loudly on mismatch). - eval.sh / compare.sh: restore oak-specific rejection of non-cuga --agent. - Rename --no-policy back to --no-policies for consistency (eval.sh, eval_bench_sdk.py, README). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
benchmarks/oak_health_insurance/eval_bench_sdk.py (1)
175-177: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn a non-zero exit code when no tasks are found.
When the filtered
test_casesis empty due to an invalid--taskID, this method logs an error and returns early. Becauseself.total_tasksremains0, the partial-run check at the end of the script (completed < total) will not trigger, and the process will exit with0(success). This can cause CI pipelines to falsely report success when a task ID is misspelled.Consider raising an exception to ensure the failure propagates to the exit code.
🐛 Proposed fix
if not test_cases: - logger.error(f"Task(s) {self.task_ids} not found in test data") - return + raise ValueError(f"Task(s) {self.task_ids} not found in test data")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/oak_health_insurance/eval_bench_sdk.py` around lines 175 - 177, Update the early-return branch for empty test_cases in the task execution method to raise an exception after logging the missing task IDs, ensuring invalid --task input propagates a non-zero process exit code instead of returning successfully.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/oak_health_insurance/eval.sh`:
- Around line 46-52: Update the argument-validation loops in
benchmarks/oak_health_insurance/eval.sh lines 46-52 and
benchmarks/oak_health_insurance/compare.sh lines 32-38 to also reject --agent=*
values unless the assigned value is cuga. Preserve the existing
separate-argument validation and error behavior, applying the same check in both
scripts.
---
Outside diff comments:
In `@benchmarks/oak_health_insurance/eval_bench_sdk.py`:
- Around line 175-177: Update the early-return branch for empty test_cases in
the task execution method to raise an exception after logging the missing task
IDs, ensuring invalid --task input propagates a non-zero process exit code
instead of returning successfully.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 49e7121b-0ee1-4dc5-a615-a4acc4ee4272
📒 Files selected for processing (5)
benchmarks/oak_health_insurance/README.mdbenchmarks/oak_health_insurance/compare.shbenchmarks/oak_health_insurance/eval.shbenchmarks/oak_health_insurance/eval_bench_sdk.pybenchmarks/oak_health_insurance/oak_policies.py
🚧 Files skipped from review as they are similar to previous changes (2)
- benchmarks/oak_health_insurance/README.md
- benchmarks/oak_health_insurance/oak_policies.py
| for arg in "$@"; do | ||
| if [[ "$arg" == "--agent" ]]; then | ||
| _next=$((_i + 1)) | ||
| _EARLY_AGENT="${!_next:-}" | ||
| break | ||
| if [ "$_prev" == "--agent" ] && [ "$arg" != "cuga" ]; then | ||
| echo "Error: oak_health_insurance only supports --agent cuga (got '$arg')." >&2 | ||
| exit 2 | ||
| fi | ||
| _i=$((_i + 1)) | ||
| _prev="$arg" | ||
| done |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fast-fail validation misses --agent=value syntax.
The validation loop successfully catches --agent react, but it misses the assignment syntax (--agent=react). Because Python's argparse supports both formats, passing --agent=react will bypass this shell validation and reach the downstream script, executing the unsupported agent.
benchmarks/oak_health_insurance/eval.sh#L46-L52: Add a check for--agent=*to catch the assignment syntax.benchmarks/oak_health_insurance/compare.sh#L32-L38: Add the exact same check here.
💡 Proposed fix
if [ "$_prev" == "--agent" ] && [ "$arg" != "cuga" ]; then
echo "Error: oak_health_insurance only supports --agent cuga (got '$arg')." >&2
exit 2
fi
+ if [[ "$arg" == --agent=* ]] && [[ "$arg" != "--agent=cuga" ]]; then
+ echo "Error: oak_health_insurance only supports --agent cuga (got '${arg#*=}')." >&2
+ exit 2
+ fi
_prev="$arg"📝 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.
| for arg in "$@"; do | |
| if [[ "$arg" == "--agent" ]]; then | |
| _next=$((_i + 1)) | |
| _EARLY_AGENT="${!_next:-}" | |
| break | |
| if [ "$_prev" == "--agent" ] && [ "$arg" != "cuga" ]; then | |
| echo "Error: oak_health_insurance only supports --agent cuga (got '$arg')." >&2 | |
| exit 2 | |
| fi | |
| _i=$((_i + 1)) | |
| _prev="$arg" | |
| done | |
| for arg in "$@"; do | |
| if [ "$_prev" == "--agent" ] && [ "$arg" != "cuga" ]; then | |
| echo "Error: oak_health_insurance only supports --agent cuga (got '$arg')." >&2 | |
| exit 2 | |
| fi | |
| if [[ "$arg" == --agent=* ]] && [[ "$arg" != "--agent=cuga" ]]; then | |
| echo "Error: oak_health_insurance only supports --agent cuga (got '${arg#*=}')." >&2 | |
| exit 2 | |
| fi | |
| _prev="$arg" | |
| done |
📍 Affects 2 files
benchmarks/oak_health_insurance/eval.sh#L46-L52(this comment)benchmarks/oak_health_insurance/compare.sh#L32-L38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/oak_health_insurance/eval.sh` around lines 46 - 52, Update the
argument-validation loops in benchmarks/oak_health_insurance/eval.sh lines 46-52
and benchmarks/oak_health_insurance/compare.sh lines 32-38 to also reject
--agent=* values unless the assigned value is cuga. Preserve the existing
separate-argument validation and error behavior, applying the same check in both
scripts.
…e-internal-oak-health # Conflicts: # benchmarks/oak_health_insurance/README.md # benchmarks/oak_health_insurance/compare.sh # benchmarks/oak_health_insurance/eval.sh # benchmarks/oak_health_insurance/eval_bench_sdk.py # benchmarks/oak_health_insurance/run_app.sh
Pins every tool-guide target_tools entry against the tool names recorded in the benchmark's test suite, so a future rename to a non-attaching form fails in CI instead of silently degrading an evaluation run. Offline counterpart to the runtime guardrail in eval_bench_sdk._load_oak_policies. Registers benchmarks/oak_health_insurance/tests in pytest testpaths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
haroldship
left a comment
haroldship
left a comment
There was a problem hiding this comment.
Excellent PR. Thanks for addressing all of the fixes. It looks great now. Gonna edit the PR title and body a tad and merge.
haroldship
changed the title
2 participants
Closes #53.
TDLR
What
cuga-oak-health), aligning with the upstream open-source dataset.oak_policies.py: playbooks and tool enrichments for all oak task categories (claims, care providers, benefits, payments, family, plan info).DYNACONF_ADVANCED_FEATURES__REGISTRYin the oak env config (required for policy loading).Results
Validated on
oss-120b: 82.1% avg keyword match (32/39 full passes) — as expected for this model tier.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
--no-policy) and stricter compare mode.Documentation
eval/comparecommand usage, plus metrics and file structure sections.