test: measure the binding day directive in the eval#3592
Conversation
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.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 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".
| if (named(planProse)) { | ||
| dispositions.add('${commitment.id}: represented'); |
There was a problem hiding this comment.
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 👍 / 👎.
| final directive = outcome.inputs.directive; | ||
| if (directive == null || directive.commitments.isEmpty) { |
There was a problem hiding this comment.
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 👍 / 👎.
| final tradeProse = [ | ||
| for (final call in outcome.toolCalls) | ||
| if (call.accepted && call.name.contains('propose_plan_diff')) |
There was a problem hiding this comment.
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 👍 / 👎.
| final List<Object?> changes => [ | ||
| for (final change in changes) | ||
| if (change is Map) '${change['reason'] ?? ''}', |
There was a problem hiding this comment.
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 👍 / 👎.
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_statuswith reasondirectiveUnsatisfiable. 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
bindingDirectiveseeds a realDayDirectiveEntity: 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.
directiveForDayreads 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
directiveHonouredclassifies each commitment as represented, traded, escalated, or silently dropped, and fails on the last. Details worth flagging:processingBlocked) answers for none; all three are tested.Verification
fvm dart analyzeclean; 164 tests in the eval framework, 11 new.bindingDirective, so the seeding is exercised through the real pipeline.