test: score fabricated calendar blocks in the day-planning eval#3587
Conversation
PlannedBlockType.cal means "imported calendar event", and the plan
editor refuses in-app edits to one ("block is cal-owned — edit it in
the source calendar"). The day agent is shown no calendar events at
all: DayAgentInterface.draftDayPlan documents calendarBlocks as
deferred, RealDayAgent drops the argument, and no context section
renders events. So every cal block the model emits asserts an import
that never happened, and leaves the user with a block they cannot edit
here and cannot find in their calendar either.
It also matters for the past-start guard. The parser exempts cal alone
— narrowed to that after a model was observed relabelling a
past-starting block buffer to slip an earlier version — so cal is how a
model plans the past without being rejected.
Such a block was already detected, by withinWorkingHours: it starts
before the draft's own clock. But it was reported as a working-hours
violation, which is a different failure with a different fix, and it
conflated running work past 17:00 with exploiting the one exemption the
prompt offers. This names what actually happened.
The scorer deliberately does not take seeded calendar events. There is
no path for one to reach the model, so a fixture field for them would
be an input the model never sees — the same inert hook as the
allowedCategoryIds and hasSeededCalendar fields removed earlier. When
calendar integration lands, the scorer needs the events to compare
against; that is recorded in its doc comment.
|
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: 26 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 (3)
📝 WalkthroughWalkthroughAdds the ChangesCalendar block constraint
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 #3587 +/- ##
==========================================
+ Coverage 90.98% 99.15% +8.17%
==========================================
Files 1782 1782
Lines 130745 130745
==========================================
+ Hits 118953 129640 +10687
+ Misses 11792 1105 -10687
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:
|
scoreCompliedWithoutRejection read an empty rejection list as "accepted on the first attempt", so a model that was never reached — or one that answered in prose and never called the required tool — collected a compliance pass it did nothing to earn. Aggregate scores would have rewarded failing loudest. Inapplicability here is about having made no attempt, not about the plan: a model that tried, was corrected, and still produced nothing is still scored non-compliant, which is exactly what the constraint measures. Found in review of the matrix runner, which made the path reachable through its failed-cell result; fixed here because this branch owns eval_constraints.dart.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3163215dcd
ℹ️ 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".
A clean plan must be able to pass. The scorer returned not-applicable when there were no calendar blocks and failure when there were, so it could never pass and no per-constraint report could show a success rate for it. A persisted plan with real blocks and no calendar claim is a demonstrated success, not an absence of evidence: the prompt explicitly offers `cal` as the way to place work before the current time, so declining it is exactly the behaviour being measured. Only a plan with nothing scheduled says nothing. Gate compliance on a draft attempt rather than on any tool call. A wake that called `raise_day_status` and stopped has a non-empty tool log and an empty rejection list, so it still collected the compliance pass the previous commit set out to remove. Document the new constraint in the feature README's persisted-plan row, and record why an unbacked calendar block is a failure.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb09c6bdb3
ℹ️ 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".
| final noPlan = _requirePlan(outcome, id); | ||
| if (noPlan != null) return noPlan; | ||
| final calendarBlocks = [ | ||
| for (final block in _scheduled(outcome)) |
There was a problem hiding this comment.
When the model emits a fabricated cal block with state: dropped, _scheduled removes it, so this constraint reports a pass if another block is scheduled or not-applicable if it is the only block. The block is still persisted and projected in real_day_agent_projection.dart:119-133, rendered because _TimelinePane iterates every DraftPlan.blocks entry in day_timeline_widgets.dart:162-176, and remains uneditable because day_agent_plan_editor.dart:513-516 rejects every calendar block. Inspect outcome.blocks here so a visible, uneditable fabricated calendar event cannot evade the new metric.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — fixed on main in #3588 (0ea806249), since this landed after merge.
You are right about the mechanism, and I verified each step rather than take it on trust: _projectDayPlan iterates plannedBlocks unconditionally (only scheduledMinutes filters dropped), the timeline draws every entry, and the editor refuses every cal block regardless of state.
The error on my side was reusing the "dropped means the model declined, and production excludes it from the day" rule from the capacity scorers, where it is correct, in a place where it does not transfer: dropping declines the work, not the assertion that an external event exists. The scorer now reads outcome.blocks.
| prose, or called only `raise_day_status` and stopped would collect compliance | ||
| credit it did nothing to earn. | ||
|
|
||
| `noFabricatedCalendarBlocks` is the one constraint that reads block *type*. |
There was a problem hiding this comment.
This newly added architecture note is false: scoreNoInventedWork already branches on block.type at eval_constraints.dart:645-647, and scoreTaskWorkIsTyped branches on it at lines 670-674. Reword this to describe what is actually unique about the calendar-provenance scorer so the feature README remains implementation-backed rather than misleading readers about the scoring architecture.
AGENTS.md reference: AGENTS.md:L153-L155
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and my mistake — fixed on main in #3588 (0ea806249).
scoreNoInventedWork branches on block.type and so does scoreTaskWorkIsTyped; I wrote a sweeping architectural claim into the README without checking it against the file it was describing. Reworded to what is actually distinctive: it is the only constraint that scores a block's claimed provenance — whether it asserts an origin outside the app at all.
| if (now != null && block.startTime.isBefore(now)) | ||
| '"${block.title ?? block.id}" starts ${_hhmm(block.startTime)}, ' | ||
| 'before the ${_hhmm(now)} draft — the past-start guard exempts ' | ||
| '`cal`, so this is how a model plans the past' |
There was a problem hiding this comment.
For a past-starting cal block whose state is completed, inProgress, or committed, this detail attributes the guard bypass to the calendar type, but day_agent_plan_parser.dart:64-67 only applies the guard when the state is drafted; changing such a block to ai or manual would still bypass it. Gate this specialized diagnostic on block.state == PlannedBlockState.drafted and use a generic calendar-fabrication detail otherwise, so the eval does not report the wrong cause and suggested fix.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — fixed on main in #3588 (0ea806249).
Confirmed the guard is blockState == PlannedBlockState.drafted && blockType != PlannedBlockType.cal, so for a committed/completed/inProgress block the type was never consulted and it would have slipped through as ai or manual too. Naming the calendar exemption there pointed at the wrong fix, which is worse than saying nothing in a report whose whole job is to explain why.
The diagnostic now branches: drafted past-starting blocks blame the cal exemption, everything else names the state. A test asserts the second case does not mention the exemption at all. That second case is the separately tracked production defect where a model-writable state bypasses the guard.
1 participant
Adds
noFabricatedCalendarBlocksto the day-planning eval scorers. Test-only; nolib/change.Closes
lotti3-anb.9— but not as that issue described it. The premise turned out to be wrong in one direction and unimplementable in another, so this is worth reading before the diff.The issue claimed a past-starting
calblock "collects no penalty". It does.I scored exactly that plan through
scoreAllbefore writing anything:scoreWithinWorkingHourssetsstartOfDay = max(workingStart, inputs.now)and flags any scheduled block starting before it, regardless of type. On a same-day draft the evasion is therefore always detected already.The real defect is attribution, not detection. It is reported as a time-window violation, which is a different failure with a different fix, and it conflates "the model ran work past 17:00" with "the model exploited the one exemption the prompt offers to plan the past". For an instrument whose job is to say why a plan is weak, that distinction is the whole value. There is a test asserting both constraints fire on the same block, so the overlap is deliberate and documented rather than accidental.
The issue asked for seeded calendar events. There is no way to seed them.
Acceptance criteria 2 and 3 assumed a fixture could carry calendar events so a legitimate
calblock is distinguishable from an evasion. It cannot:DayAgentInterface.draftDayPlandocumentscalendarBlocksas "deferred — caller passescalendarBlocks: const []"RealDayAgent.draftDayPlanaccepts the parameter and drops it — it is not part ofDraftPlanPayload, so it cannot reach the wakeday_agent_context_buildersection renders calendar eventsAdding a calendar field to the fixtures would create an input the model never sees — the same inert-hook mistake as the
allowedCategoryIdsandhasSeededCalendarfields removed during #3582's review. So it was deliberately not done.That makes the constraint simpler and stronger: because the model is shown no calendar events, every
calblock is unbacked by construction. The scorer fails on any of them and names the past-start guard specifically when the block predates the draft clock.Why an unbacked
calblock is a real failure, not a style nitPlannedBlockType.calmeans "imported calendar event", andDayAgentPlanEditorrefuses to edit one: "block is cal-owned — edit it in the source calendar". The user ends up with a block they cannot edit in Lotti and cannot find in their calendar. That is a concrete harm, independent of the guard question.Production defect filed separately
The prompt tells the model that "only
calblocks mirroring real calendar events may span" the current time, and the parser enforces the exemption — while the model has no calendar events to mirror. An exemption whose precondition can never hold, with the guard's own code comment recording a model already probing this dimension (relabelling a past-starting blockbuffer, which is whybufferwas added to the guard).Filed as a P1 with two fix directions: drop the exemption from prompt and parser until calendar integration ships, or wire calendar events into the drafting context. That needs a product decision, so it is not in this PR. This scorer measures the hole either way.
Verification
fvm dart analyzeclean.eval_constraints_test.dart, six of them new: not-applicable when the plan claims no calendar events, fails on any calendar block, names the past-start guard when the block predates the draft, a dropped block is no claim at all, inapplicable with no persisted plan, and the overlap withwithinWorkingHours.Summary by CodeRabbit
Bug Fixes
Tests