Skip to content

test: measure the binding day directive in the eval#3592

Merged
matthiasn merged 1 commit into
mainfrom
test/eval-binding-directive
Jul 25, 2026
Merged

test: measure the binding day directive in the eval#3592
matthiasn merged 1 commit into
mainfrom
test/eval-binding-directive

Conversation

@matthiasn

Copy link
Copy Markdown
Owner

The prompt calls the day directive "BINDING, not a hint" and names exactly three legitimate responses to a commitment: represent it in the plan, trade it away in a diff whose reason names the colliding commitment, or escalate via raise_day_status with reason directiveUnsatisfiable. Nothing enforces any of it — the directive is prompt text and the plan writer never reads it back — so a model can drop a commitment the user asked for and the plan persists looking clean.

That was the strongest contract in the prompt and the only major one with no coverage at all. Stronger than ADR 0043's blocked-work rule, which does have a scenario. Closes lotti3-anb.10. Test-only.

The scenario

bindingDirective seeds a real DayDirectiveEntity: three commitments totalling 390 minutes against a 240-minute remaining budget (300 available, 60 already scheduled). Deliberately over budget — a directive that fits would let a model honour everything by accident and prove nothing, since the contract only bites when something has to give. A fixture test asserts the arithmetic, so a later edit cannot quietly make it satisfiable.

It is seeded directly rather than by running a digest wake, for the same reason the capture is: a digest is a second model call with its own prompt and tool budget, and the unit of measurement here is one drafting wake. directiveForDay reads it by deterministic id and the prompt builder renders the real section — a runner test asserts <day_directive>, the commitment id and its title all reach the prompt, so the scenario cannot silently stop testing anything.

The scorer

directiveHonoured classifies each commitment as represented, traded, escalated, or silently dropped, and fails on the last. Details worth flagging:

  • A trade must name the commitment it collides with, as the prompt specifies. "Something had to give" fails; naming the colliding commitment passes. Tested both ways.
  • Escalation is directive-level. A model that says the orders cannot be satisfied has answered for every commitment it did not place — that is what the prompt asks. But a rejected escalation, a different status, or an unrelated reason (processingBlocked) answers for none; all three are tested.
  • A dropped block does not represent a commitment. Dropping is the model declining the work, which is the opposite of honouring the order.
  • Representation is matched on the commitment's title or id appearing in a block's title, reason or note. That is string matching, with the limits string matching has — but gaming it requires actually placing a block named after the commitment, and whether that block is real is what the capacity, estimate and working-hours scorers already measure.

Verification

  • fvm dart analyze clean; 164 tests in the eval framework, 11 new.
  • The existing "drives every shipped scenario end to end" test now covers bindingDirective, so the seeding is exercised through the real pipeline.

The prompt calls the day directive "BINDING, not a hint" and names
exactly three legitimate responses to a commitment: represent it in the
plan, trade it away in a diff whose reason names the colliding
commitment, or escalate via raise_day_status with reason
directiveUnsatisfiable. Nothing enforces any of it — the directive is
prompt text and the plan writer never reads it back — so a model can
drop a commitment the user asked for and the plan persists clean.

That was the strongest contract in the prompt and the only major one
with no coverage at all, stronger than the ADR 0043 blocked-work rule
which does have a scenario.

Adds a bindingDirective scenario seeding a real DayDirectiveEntity —
390 minutes of commitments against a 240-minute remaining budget, so
something has to give and the contract actually bites. It is seeded
directly rather than issued by a digest wake, for the same reason the
capture is: a digest is a second model call with its own prompt and
tool budget, and the unit of measurement here is one drafting wake.

The scorer distinguishes all three legitimate outcomes from silence,
and a trade must name the commitment it collides with, as the prompt
specifies — "traded something away" without saying what gives the user
nothing to act on. Escalation is directive-level and answers for every
commitment left unplaced; a rejected escalation, a different status, or
an unrelated reason answers for none.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@matthiasn, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7d4ed2ed-da37-4230-90bf-3f6c4bdb91c4

📥 Commits

Reviewing files that changed from the base of the PR and between 617cadb and e6d3cf0.

📒 Files selected for processing (7)
  • test/features/daily_os_next/eval/framework/eval_constraints.dart
  • test/features/daily_os_next/eval/framework/eval_constraints_test.dart
  • test/features/daily_os_next/eval/framework/eval_models.dart
  • test/features/daily_os_next/eval/framework/eval_runner.dart
  • test/features/daily_os_next/eval/framework/eval_runner_test.dart
  • test/features/daily_os_next/eval/framework/eval_scenario.dart
  • test/features/daily_os_next/eval/framework/eval_scenario_test.dart
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/eval-binding-directive

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.

@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.15%. Comparing base (9513642) to head (e6d3cf0).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3592   +/-   ##
=======================================
  Coverage   99.15%   99.15%           
=======================================
  Files        1782     1782           
  Lines      130810   130810           
=======================================
  Hits       129704   129704           
  Misses       1106     1106           
Flag Coverage Δ
glados 14.08% <ø> (ø)
standard 98.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@matthiasn
matthiasn merged commit bfabc04 into main Jul 25, 2026
26 of 27 checks passed
@matthiasn
matthiasn deleted the test/eval-binding-directive branch July 25, 2026 19:25

@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: e6d3cf0569

ℹ️ 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 on lines +764 to +765
if (named(planProse)) {
dispositions.add('${commitment.id}: represented');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Account for directive minutes before marking commitments represented

In the bindingDirective scenario, merely mentioning each commitment in scheduled-block prose makes this constraint pass, regardless of the block durations. A model can therefore emit three tiny blocks titled after the 180/120/90-minute commitments and receive a pass even though it neither reconciled the requested 390 minutes against the directive's 240-minute budget nor surfaced the conflict; the estimate scorers only cover corpus tasks, and the scenario's ordinary capacity remains the 480-minute default, so no other constraint catches this false positive.

Useful? React with 👍 / 👎.

Comment on lines +718 to +719
final directive = outcome.inputs.directive;
if (directive == null || directive.commitments.isEmpty) {

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 Treat runs without a persisted plan as inapplicable

When this scenario fails before producing a plan—for example, EvalModelTarget.open throws and _failedResult creates an outcome with planPersisted: false and no tool calls—the scorer continues and reports every directive commitment as silently dropped. This turns an infrastructure failure into a model-quality failure, unlike the other plan-reading constraints, and contaminates the directive constraint's results; check planPersisted before scoring the plan or its missing dispositions.

Useful? React with 👍 / 👎.

Comment on lines +744 to +746
final tradeProse = [
for (final call in outcome.toolCalls)
if (call.accepted && call.name.contains('propose_plan_diff'))

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 Make the scored trade path reachable

In every bindingDirective cell the runner creates a fresh harness with no existing plan, so production's DayAgentPlanEditor.proposePlanDiff rejects propose_plan_diff until draft_day_plan has persisted one; the drafting prompt simultaneously requires draft_day_plan to be the final tool call. Consequently this accepted trade branch can never be reached end to end, and a model following the prompt's explicit trade option is recorded as rejected and forced to choose another response. Seed a baseline and exercise a refine wake, or otherwise align the scenario with a workflow where a proposed diff can succeed.

Useful? React with 👍 / 👎.

Comment on lines +748 to +750
final List<Object?> changes => [
for (final change in changes)
if (change is Map) '${change['reason'] ?? ''}',

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 Identify the traded commitment from the dropped change

The prompt requires a traded-away commitment's diff reason to name the other commitment it collides with, but this code treats whichever commitment is mentioned in the reason as the one that was traded. For example, dropping the weekly-1:1s block with reason Prepare the board deck collides credits the board-deck commitment as traded while leaving the actual casualty unrecognized; reasons that mention both sides can incorrectly credit both. Inspect the change action and dropped blockId/snapshot to identify the casualty, using the reason only to verify that its collision was named.

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