Skip to content

test: live day-planning eval entry point, and what its first runs found#3593

Merged
matthiasn merged 9 commits into
mainfrom
test/day-planning-eval-live
Jul 25, 2026
Merged

test: live day-planning eval entry point, and what its first runs found#3593
matthiasn merged 9 commits into
mainfrom
test/day-planning-eval-live

Conversation

@matthiasn

@matthiasn matthiasn commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Closes lotti3-anb.5 (live entry point), lotti3-anb.6 (one eval path), lotti3-0qq, and carries everything lotti3-anb.7 — actually running it — exposed.

The entry point

eval/day_planning_eval_live_test.dart supplies only the inference layer and the output paths; everything below the model is the production pipeline.

set -a; source .env; set +a   # MELIOUS_API_KEY / MELIOUS_BASE_URL
LOTTI_DAY_PLANNING_EVAL_LIVE=1 DAY_PLANNING_EVAL_MODELS=glm-5.2 \
  fvm flutter test test/features/daily_os_next/eval/day_planning_eval_live_test.dart

It always passes when it runs. Violations are reported, never asserted: a live run is non-deterministic and costs money, and a red build people learn to ignore is worse than no signal. Missing credentials are the one hard failure — a setup error, not something a model did.

day_agent_draft_live_eval_test.dart is deleted, so there is one live path. Its subject — a same-day draft where the model's proposed times meet the past-start guard — is the lateStart scenario, which additionally seeds real work, so the run says something about planning rather than only about the guard.

Four runs against glm-5.2, and what each one fixed

Every finding in the first three was about the eval, not the model.

run 1 run 2 run 3 run 4
overall 88% 91% 90% 94%
compliedWithoutRejection 13% 50% 63% 88%
directiveHonoured 0% 0% 100% 100%
noFabricatedTaskIds 100% 100% 86% 100%

That climb is not the model improving. It is instrument repair.

A drafting wake offers the whole capture/reconcile tool set, not just draft_day_plan, and glm-5.2 used it — parse_capture_to_items, surface_pending_decisions, summarize_recent_patterns, create_task_from_phrase, apply_triage — before drafting. Each landed on a mocked collaborator. An unstubbed mocktail method returns a bare null where a Future is expected, so the call came back as type 'Null' is not a subtype of type 'Future<Task?>': a harness defect delivered as a rejection, which is exactly the signal compliedWithoutRejection scores. Later rounds were subtler, same shape — "task not found" for a task the scenario itself seeded, "failed to create task", "failed to update task", all because stubs returned null/false where the app succeeds.

The directive scorer refereed free text with substring matching, and was wrong twice. Requiring the reason enum to be directiveUnsatisfiable scored an escalating model SILENTLY DROPPED. Requiring the note to contain each commitment's title did the same to:

"Interviews and 1:1s cannot fit — user must defer one or both, or extend capacity."

which names both casualties in the words a person would use. Escalation is directive-level in the prompt — "escalate via raise_day_status", not "name each commitment" — so it now answers for everything unplaced, gated on a reason that could honestly describe this day failing, drawn from the fixture allowlist surfacedConflict already uses. processingBlocked still answers for nothing.

A false "silently dropped" is worse than a coarse pass: it accuses the model of the one thing it visibly did not do. This is lotti3-anb.11's thesis confirmed with evidence rather than speculation.

A task the model created and then scheduled read as fabricated (lotti3-0qq, fixed here rather than deferred). The id is recoverable from real persisted state — create_task_from_phrase stamps it onto the parsed item as matchedTaskId — so no production change, and it works identically live. This closes a latent gap the artefact only happened to expose: the same flow exists in the app, so any real plan where the model created a task and scheduled it would have been reported as invention. It cannot launder an invented id, because the capture service nulls matchedTaskId unless it resolves to a real, open, category-allowed Task.

What run 4 says about the model

All six remaining failures are genuine, and two matter:

bindingDirective — a block titled "Already scheduled" at 09:00, type cal, state committed, materialising the directive's alreadyScheduledMinutes: 60. Reproduced in runs 3 and 4, so it is habitual, not a fluke. That single block walks through both filed production holes at once: lotti3-3r1 (the prompt offers a cal exemption while the model is shown no calendar) and lotti3-abp (a model-writable state bypasses the same-day guard, which fires only on drafted). noFabricatedCalendarBlocks and noHistoryFabrication both caught it. Neither hole is hypothetical any more.

blockedWithoutCorpus — 50% on blockerBeforeBlocked, and task-a-root ignored, against a clean pass for its twin blockedChain. That is the pair doing exactly the job it was built for: same ground truth, one variable — whether the corpus was rendered — and a measurable gap.

Also stable: respectsEstimates 83% (a 180-minute task allocated 60 minutes), requiredWorkPlaced 75%, and ~95% of input tokens served from cache, consistent with the 100%-stable system prompt.

Verification

  • fvm dart analyze clean; 195 eval-framework tests; the full daily_os_next suite passes with the harness changes.
  • Four live runs, each read end to end through the judge bundle — which is how every one of the instrument defects above was found.

Summary by CodeRabbit

  • New Features
    • Added opt-in live “day-planning” evaluation matrix with configurable models, samples, and scenarios, generating reports and artifacts.
    • Evaluation outputs now include task IDs created during a run.
  • Bug Fixes
    • Improved fabricated-task detection using created task tracking.
    • Refined directive escalation scoring and more accurate journal-based task discovery.
  • Documentation
    • Expanded evaluation docs with a “Running it against real models” section and updated fake-time guidance.
  • Tests
    • Added/updated eval fixtures and harness helpers to better simulate journal and task lifecycles, including new live test coverage wiring.

… one

The entry point supplies only the inference layer and the output
paths; everything below the model is the production pipeline the
framework already drives.

It always passes when it runs. Violations are reported, never
asserted: a live run is non-deterministic and costs money, and a red
build people learn to ignore is worse than no signal. The report and
its judge bundle are the deliverable. Missing credentials are the one
hard failure, because that is a setup error rather than anything a
model did.

Deletes day_agent_draft_live_eval_test.dart so there is one live eval
path. Its subject — a same-day draft where the model's proposed times
meet production's past-start guard — is now the lateStart scenario,
which additionally seeds real work, so the run can say something about
planning rather than only about the guard. Its three references are
repointed: the fake-time exception in test/README.md, the pipeline
harness doc, and the smoke test doc.
Two harness gaps and one wrong scorer, all found by running the matrix
against glm-5.2 and reading the judge bundle.

A drafting wake offers the model the whole capture/reconcile tool set,
not just draft_day_plan, and glm-5.2 used it: parse_capture_to_items,
surface_pending_decisions, summarize_recent_patterns and
create_task_from_phrase all before drafting. Those land on
collaborators the eval mocks, and an unstubbed mocktail method returns
a bare null where a Future is expected — so the tool call came back to
the model as "type 'Null' is not a subtype of type 'Future<Task?>'".
A harness defect delivered as a *rejection*, which is exactly the
signal compliedWithoutRejection scores; the first run read 13% on that
constraint, most of it this. journalEntityById, journalEntityMapForIds
and updateJournalEntity are now stubbed on the harness, and
createTaskEntry in a shared eval getIt setup both entry points use.

The scorer was wrong about escalation. glm-5.2 raised attentionNeeded
with reason overCommitted and a note reading "Cannot fit: interviews
(120 min) and release notes" — naming the exact casualty. Requiring
the reason enum to be directiveUnsatisfiable scored that as SILENTLY
DROPPED, which is the one thing it demonstrably was not. Silence is
the failure this constraint exists to catch; using a different-but-true
reason label is a separate and much weaker observation, so it is now
reported in the detail rather than failed. An escalation that names
nothing is still silence.
…on honestly

A second live run against glm-5.2 showed the first fix was half-hearted.
Its remaining rejections were still almost all mine: "task
task-overdue-invoice not found" for a task the scenario had put in the
corpus and the model correctly named, and "failed to create task"
because the stub returned null. Both reach the model as corrections it
did not earn, and compliedWithoutRejection is the one constraint that
measures exactly that. journalEntityById now answers from the
scenario's own tasks, and createTaskEntry returns a real Task, because
in the app the creation succeeds.

The directive scorer was refereeing escalation with substring matching
and getting it wrong twice. Requiring the reason enum to be
directiveUnsatisfiable called an escalating model SILENTLY DROPPED;
requiring the note to contain each commitment's title did the same to
"Interviews and 1:1s cannot fit — user must defer one or both", which
names both casualties in the words a person would use. Escalation is
directive-level in the prompt, so it now answers for everything left
unplaced — but only under a reason that could honestly describe this
day failing, drawn from the fixture's own allowlist the way
surfacedConflict already does it. processingBlocked says the pipeline
is stuck and still answers for nothing.

A false "silently dropped" is worse than a coarse pass: it accuses the
model of the one thing it visibly did not do.
apply_triage updates a task through updateJournalEntity, and the stub
answered false — which the tool turns into "failed to update task <id>"
and hands back to the model as a correction it did not earn. Four of
the third run's remaining rejections were this.

The one artefact left, filed separately: a task the model creates
mid-wake and then schedules reads as a fabricated id, because the
referenceable set is computed before the run and the created id lives
only in a tool-result payload the agent log does not persist.
@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

Review Change Stack

Warning

Review limit reached

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

Next review available in: 32 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: 2aac2cb5-88a1-49a9-a21a-6ed823bf6995

📥 Commits

Reviewing files that changed from the base of the PR and between 87004c9 and a6bebac.

📒 Files selected for processing (7)
  • test/features/daily_os_next/eval/day_planning_eval_live_test.dart
  • test/features/daily_os_next/eval/framework/eval_journal_fixture.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_test_setup.dart
  • test/features/daily_os_next/integration/day_agent_pipeline_harness.dart
📝 Walkthrough

Walkthrough

The PR replaces the prior live draft evaluation with an opt-in day-planning evaluation matrix, adds report generation, tracks created task IDs, refines constraint scoring, and updates evaluation fixtures, harnesses, and documentation.

Changes

Day-planning evaluation

Layer / File(s) Summary
Evaluation outcomes and scoring
test/features/daily_os_next/eval/framework/eval_models.dart, eval_runner.dart, eval_constraints.dart, eval_report.dart, *_test.dart
Evaluation runs track created task IDs, fabricated-task scoring accepts known created IDs, directive escalation results distinguish escalation reasons, and report bundles include created task IDs.
Evaluation harness and scenario support
test/features/daily_os_next/eval/framework/eval_test_setup.dart, eval_journal_fixture.dart, eval_scenario.dart, eval_runner_test.dart, test/features/daily_os_next/integration/*
Shared dependency setup, journal state, scenario task lookup, and persistence stubs support evaluation and capture flows; related test descriptions reference the matrix runner.
Live evaluation matrix
test/features/daily_os_next/eval/day_planning_eval_live_test.dart, lib/features/daily_os_next/README.md, test/README.md
A gated live test selects models, samples, and scenarios from environment variables, runs real-provider evaluations, writes report artifacts, and documents execution and timing behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant LiveEvalTest
  participant EvalLlmLayer
  participant runEvalMatrix
  participant EvalReport
  LiveEvalTest->>EvalLlmLayer: configure selected model targets
  EvalLlmLayer->>runEvalMatrix: run selected scenarios and samples
  runEvalMatrix->>EvalReport: provide evaluation results
  EvalReport->>LiveEvalTest: write report artifacts and print summaries
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the new live day-planning eval entry point and the initial run findings, which match the PR's main changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/day-planning-eval-live

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.

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

ℹ️ 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 test/features/daily_os_next/eval/framework/eval_test_setup.dart Outdated
Comment thread test/features/daily_os_next/eval/framework/eval_constraints.dart
create_task_from_phrase is offered on a drafting wake, and glm-5.2
used it then scheduled what it made. referenceableTaskIds is fixed
before the run, so the placement read as a fabricated id.

The id is recoverable from real persisted state rather than needing a
production change: create_task_from_phrase stamps it onto the parsed
item as matchedTaskId, so evalCreatedTaskIdsFrom reads the agent
repository the same way the tool-call log does, and works identically
in a live run.

This also closes a latent gap the artefact happened to expose. The
same flow exists in the app, so any real plan where the model created
a task and scheduled it would have been reported as invention.

It cannot launder a fabricated id: the capture service nulls
matchedTaskId unless it resolves to a real, open, category-allowed
Task through the journal.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🧹 Nitpick comments (1)
lib/features/daily_os_next/README.md (1)

446-473: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a Mermaid diagram for the live-eval flow.

Show the environment gate/credential failure, live matrix execution, and report output path. As per coding guidelines, “use architecture-first, implementation-backed documentation and Mermaid diagrams for real flows, lifecycles, or state machines.”

🤖 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 `@lib/features/daily_os_next/README.md` around lines 446 - 473, Add a Mermaid
flow diagram to the “Running it against real models” section covering the opt-in
environment gate, credential validation and hard failure, live model/scenario
matrix execution, and report/judge-bundle output paths. Keep the diagram aligned
with the documented symbols and behavior, including that evaluation violations
are reported rather than asserted.

Source: Coding guidelines

🤖 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 `@test/features/daily_os_next/eval/day_planning_eval_live_test.dart`:
- Around line 59-61: Update the evaluation setup around modelIds and samples to
fail immediately when modelIds.isEmpty or samples < 1, before executing
evaluations or writing the report. Preserve the existing parsing defaults for
unset or invalid environment values.

In `@test/features/daily_os_next/eval/framework/eval_test_setup.dart`:
- Around line 49-65: Use one mutable task store across all task operations: in
test/features/daily_os_next/eval/framework/eval_test_setup.dart lines 49-65,
generate a unique ID for each created task and insert it into shared storage; in
test/features/daily_os_next/eval/framework/eval_scenario.dart lines 701-713,
resolve lookups from that storage while preserving corpus-seeded tasks; in
test/features/daily_os_next/integration/day_agent_pipeline_harness.dart lines
235-246, use the same storage for lookup and update, mutating tasks on updates,
and add coverage for create→lookup/update. Anchor the implementation to the
existing createTaskEntry, journalEntityById, and update stubs without changing
unrelated behavior.

---

Nitpick comments:
In `@lib/features/daily_os_next/README.md`:
- Around line 446-473: Add a Mermaid flow diagram to the “Running it against
real models” section covering the opt-in environment gate, credential validation
and hard failure, live model/scenario matrix execution, and report/judge-bundle
output paths. Keep the diagram aligned with the documented symbols and behavior,
including that evaluation violations are reported rather than asserted.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8bbd5f15-7a94-4832-9d54-be820e30fc9c

📥 Commits

Reviewing files that changed from the base of the PR and between bfabc04 and df754ff.

📒 Files selected for processing (14)
  • lib/features/daily_os_next/README.md
  • test/README.md
  • test/features/daily_os_next/eval/day_agent_draft_live_eval_test.dart
  • test/features/daily_os_next/eval/day_planning_eval_live_test.dart
  • 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_report.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_test_setup.dart
  • test/features/daily_os_next/integration/day_agent_durable_jobs_smoke_test.dart
  • test/features/daily_os_next/integration/day_agent_pipeline_harness.dart
💤 Files with no reviewable changes (1)
  • test/features/daily_os_next/eval/day_agent_draft_live_eval_test.dart

Comment thread test/features/daily_os_next/eval/day_planning_eval_live_test.dart
Comment thread test/features/daily_os_next/eval/framework/eval_test_setup.dart
@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 (bfabc04) to head (a6bebac).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3593   +/-   ##
=======================================
  Coverage   99.15%   99.15%           
=======================================
  Files        1783     1783           
  Lines      130939   130939           
=======================================
  Hits       129833   129833           
  Misses       1106     1106           
Flag Coverage Δ
glados 14.07% <ø> (ø)
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.

…ething

Two review findings, both P1, both real.

A task the model created was handed back as an id and then denied.
DayAgentPlanWriter resolves allowed task references through
journalEntityMapForIds and the capture service resolves triage targets
through journalEntityById, both stubbed from the scenario alone — so a
task created mid-wake was absent from each, and the pipeline would
reject a placement the app accepts. In the third live run such a block
persisted anyway, but only through the separate unverified
decidedTaskIds hole, not because the reference resolved.

Both stubs and the createTaskEntry stub now share one per-cell journal,
reset at seeding so no cell sees another's tasks.

The escalation branch had become too permissive. Any accepted
attentionNeeded under an allowlisted reason credited every commitment,
so a model could drop all three and pass on a bare day-level remark
that never mentions the directive. I justified that by delegating
casualty-checking to surfacedConflict — which this scenario never runs,
because it leaves requiresConflictSurfaced false. Under a reason other
than the prompt's, the call must now carry a note; under
directiveUnsatisfiable it still speaks for itself.

The bar is structural — a note exists or it does not — rather than
semantic, because refereeing what a note means by substring match is
what got this scorer wrong twice already.
Two more gaps in the same area.

Created ids were derived from the title hash, so two tasks with the
same title collapsed onto one id and the second overwrote the first.
Now sequential, which is also stable enough to stay readable in a
report.

updateJournalEntity answered true without changing anything, so a
model that ran apply_triage and then read the task back saw its own
write missing — the harness agreeing out loud and doing nothing. The
eval now backs that stub with the same per-cell journal, which needed
the harness to expose its journal repository the way it already
exposes the plan service.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
test/features/daily_os_next/eval/framework/eval_runner_test.dart (1)

1101-1119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Route this regression test through task creation.

Calling currentEvalJournal.add(...) bypasses the createTaskEntry/persistence wiring this PR is intended to protect. A regression that stops the creation stub from persisting the returned task would still pass. Exercise the harness’s creation boundary, then assert that the returned ID is resolvable through byId and mapForIds.

🤖 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 `@test/features/daily_os_next/eval/framework/eval_runner_test.dart` around
lines 1101 - 1119, Update the regression test setup around
currentEvalJournal.add to create the “Made later” task through the harness’s
createTaskEntry flow instead of inserting it directly. Capture the returned task
ID, then assert that byId and mapForIds resolve that ID correctly, preserving
the existing task data and regression expectations.
🤖 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 `@test/features/daily_os_next/eval/framework/eval_runner_test.dart`:
- Around line 1121-1127: Update the assertion around
currentEvalJournal.mapForIds to verify the returned mapping contains the
expected entity for both requested IDs, rather than only checking its length.
Include an unknown ID in the lookup and assert that it is absent from the
result.

---

Nitpick comments:
In `@test/features/daily_os_next/eval/framework/eval_runner_test.dart`:
- Around line 1101-1119: Update the regression test setup around
currentEvalJournal.add to create the “Made later” task through the harness’s
createTaskEntry flow instead of inserting it directly. Capture the returned task
ID, then assert that byId and mapForIds resolve that ID correctly, preserving
the existing task data and regression expectations.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5dbce05e-7924-4c42-b7f6-01be2c87dfe9

📥 Commits

Reviewing files that changed from the base of the PR and between df754ff and 8f2d002.

📒 Files selected for processing (6)
  • 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_journal_fixture.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_test_setup.dart
🚧 Files skipped from review as they are similar to previous changes (4)
  • test/features/daily_os_next/eval/framework/eval_test_setup.dart
  • test/features/daily_os_next/eval/framework/eval_scenario.dart
  • test/features/daily_os_next/eval/framework/eval_constraints.dart
  • test/features/daily_os_next/eval/framework/eval_constraints_test.dart

Comment thread test/features/daily_os_next/eval/framework/eval_runner_test.dart
hasLength(2) would pass with two wrong entries, and said nothing about
an id with no task behind it — which is the case that matters, because
DayAgentPlanWriter reads presence in this map as permission to
schedule.

@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: 87004c9c83

ℹ️ 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 test/features/daily_os_next/eval/framework/eval_runner.dart
Comment thread test/features/daily_os_next/eval/framework/eval_scenario.dart
Comment thread test/features/daily_os_next/eval/day_planning_eval_live_test.dart Outdated
Comment thread test/features/daily_os_next/eval/day_planning_eval_live_test.dart
Five findings, all in the seam a live eval depends on and no unit test
reaches, because unit tests script the model and only exercise the
paths I imagined.

create_task_from_phrase writes a ParsedItemEntity only when the model
passes the optional captureItemId, so reconstructing created ids from
parsed items missed tasks created without one and reported legitimate
work as fabricated. Ids are now recorded where the task is created.

match_to_corpus is offered to the model and DayAgentCorpusService
awaits watchFullTextMatches(...).first before anything else — an
unstubbed stream getter yields null, so the tool answered with a Dart
type error instead of "nothing matched".

Corpus reads rebuilt their lists from the scenario on every call, so a
model that ran apply_triage and then rechecked pending work saw the
task it had just changed as untouched. They now read the mutable
per-cell store, like the lookups already did.

The live timeout was fixed at two hours while the documented
SAMPLES=3 command is 24 sequential cells, each able to consume the
10-minute per-cell soft cap. It would have killed an expensive matrix
before writeEvalReport ran, discarding the deliverable. Now derived
from the matrix size.

An unknown DAY_PLANNING_EVAL_SCENARIOS selector was silently dropped
when at least one other matched, so an operator could pay for a report
that omitted exactly the scenario they asked about.
@matthiasn
matthiasn merged commit 2c7b324 into main Jul 25, 2026
27 checks passed
@matthiasn
matthiasn deleted the test/day-planning-eval-live branch July 25, 2026 21:03

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

ℹ️ 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 known = outcome.inputs.referenceableTaskIds;
// for ids it could not have seen — plus whatever it created during the run,
// which it equally could not have invented.
final known = outcome.knownTaskIds;

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 Count created tasks as invented in the restraint scenario

When the model hallucinates work in restraint, calls create_task_from_phrase, and schedules the returned ID, adding every created ID to knownTaskIds makes noFabricatedTaskIds pass, while scoreNoInventedWork only flags blocks whose taskId is null. The empty-day control can therefore report “added no work of its own” for exactly the task invention it exists to detect; created IDs must still count as invented when forbidsInventedWork is set.

Useful? React with 👍 / 👎.

Comment on lines +670 to +674
if (task.data.due case final due?)
if (!due.isAfter(
DateTime(planDate.year, planDate.month, planDate.day, 23, 59),
))
task,

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 Exclude closed tasks from the due-query fixture

When a model applies done or drop to a due or overdue task and then calls surface_pending_decisions again, _dueBy still returns that task because it filters only the due timestamp. Production's getTasksDueOn and getTasksDueOnOrBefore explicitly exclude DONE and REJECTED, so the model is shown work that its previous accepted tool call removed. Fresh evidence beyond the earlier mutable-journal issue is that the new mutable helper preserves updates but does not apply the production status predicate.

Useful? React with 👍 / 👎.

Comment on lines +244 to +245
() => fts5Db.watchFullTextMatches(any()),
).thenAnswer((_) => Stream.value(const <String>[]));

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 Match corpus phrases against the seeded journal

Whenever the live model calls the offered match_to_corpus tool, this replacement stub returns no FTS IDs for every phrase, even when the phrase exactly names a seeded task. Production would resolve matching FTS IDs through getJournalEntitiesForIdsUnordered; the empty result can make the model create and schedule a duplicate, corrupting required-work and compliance scores. Fresh evidence beyond the prior missing-stub report is that the newly added stub avoids the type error but still cannot reproduce any valid match.

Useful? React with 👍 / 👎.

Comment on lines +445 to +450
createdTaskIds: {
// Both sources: the persistence stub records every created task, and
// the parsed-item scan additionally covers ids the model resolved
// through triage against work outside the seeded corpus.
...currentEvalJournal.createdIds,
...evalCreatedTaskIdsFrom(harness.agentRepository.entities),

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 Preserve created-task estimates in eval outcomes

When create_task_from_phrase includes an estimate and the returned task is scheduled, this outcome keeps only its ID even though currentEvalJournal holds the full created Task. Both scoreRespectsEstimates and scoreWithinCapacityByEstimate look up estimates only in the fixed pre-run corpus, so created work can be compressed arbitrarily or push estimated workload over capacity while those constraints ignore it or report not applicable. Carry the created task metadata into the scoring view rather than discarding everything except the ID.

Useful? React with 👍 / 👎.

Comment on lines +602 to +604
Set<String> evalCreatedTaskIdsFrom(List<AgentDomainEntity> entities) => {
for (final item in entities.whereType<ParsedItemEntity>())
?item.matchedTaskId,

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 Do not label matched existing tasks as created

When parse_capture_to_items emits a valid high-confidence match to an existing corpus task, the resulting ParsedItemEntity.matchedTaskId is collected here even though no task was created. The judge bundle then lists that pre-existing task under createdTaskIds and falsely tells reviewers that the model materialized it during the wake. Track actual create_task_from_phrase persistence separately instead of treating every parsed-item match as creation.

Useful? React with 👍 / 👎.

);

expect(
journalDb.getInProgressTasks(),

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 Mirror the mixin call shape in the new journal test

In CI's very_good optimizer, this new call omits both optional parameters while seedScenarioCorpus stubs the mixin-declared JournalDb.getInProgressTasks with explicit categoryIds and limit matchers. The repository documents that mixin forwarders can supply different omitted defaults under modular compilation, causing the stub to miss and return null instead of a Future<List<Task>>; this test can therefore fail only in bundled CI. Exercise the production call shape (categoryIds supplied, limit omitted) and make the stub match that exact shape.

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