Retire the offline retrieval eval #115

Merged
kleb merged 8 commits from feature/retire-retrieval-eval into main 2026-08-26 00:59:42 +02:00
Collaborator

Summary

Retires the temporary offline retrieval evaluation now that it has done its job, plus the fixes that getting it to complete required, plus two failures found in the suite along the way.

Retire the retrieval eval

The subsystem existed to answer one question: does the V2 recall policy lose memories that V1 would have surfaced? On 125 real production cases it answered no, at identical canary recall (84.0% both) and a quarter of the harmful selections (24 to 6). V2 is now the promoted policy, so the eval is removed as designed: job runner, dataset builder, retrieval runner, metrics, encrypted state record, the session-only routes, and the settings tab.

Nothing in the recall path changes. The judge only ever ran here, offline and behind an explicit billable confirmation. buildLegacyMemoryContext stays, because v1 remains the rollback policy.

Fixes that the evaluation runs forced

  • PLANNER_AI_RATE_LIMIT makes the AI request ceiling configurable, still defaulting to 30. The interactive limit of 30 per five minutes cannot accommodate a batch job that spends one request per canary and one per judged case.
  • The judge grades one case per request. A five case batch had to return roughly 115 grade objects and the model dropped some; batching saved no tokens here, because candidate text is per case and shared by nothing.
  • The judge prompt fences the stored query and names it as data. It previously guarded candidate texts as historical data but pasted the query in raw, and a stored agent prompt carrying its own output-format instructions steered the judge into a schema-valid but empty response.
  • An unjudgeable case is skipped and counted rather than destroying a paid run, and the count is surfaced.

Failures found in the suite

  • tests/logging.test.ts was failing on main before this branch. The agent CLI is its own binary but shares logger.ts and, through it, clientAddress.ts; both validated server configuration at import, so a malformed PLANNER_LOG_LEVEL or PLANNER_TRUSTED_PROXIES aborted a CLI that never reads either. Server settings are now ignored in an agent-cli distribution, the proxy list is parsed on first use, and the server validates it explicitly at startup so fail-fast becomes a deliberate step rather than an import side effect.
  • The browser suite failed intermittently under full-suite contention, most often SettingsDialog. Seventy jsdom environments share one machine and the default five second budget is not enough for a user-event test waiting on a scheduler slot; the file passes alone. Raised to twenty seconds, which still catches a hang.

Verification

Full typecheck, type-aware oxlint, eslint, knip, oxfmt, 951 bun tests, and 224 vitest tests all pass. tests/logging.test.ts passes for the first time.

## Summary Retires the temporary offline retrieval evaluation now that it has done its job, plus the fixes that getting it to complete required, plus two failures found in the suite along the way. ### Retire the retrieval eval The subsystem existed to answer one question: does the V2 recall policy lose memories that V1 would have surfaced? On 125 real production cases it answered no, at identical canary recall (84.0% both) and a quarter of the harmful selections (24 to 6). V2 is now the promoted policy, so the eval is removed as designed: job runner, dataset builder, retrieval runner, metrics, encrypted state record, the session-only routes, and the settings tab. Nothing in the recall path changes. The judge only ever ran here, offline and behind an explicit billable confirmation. `buildLegacyMemoryContext` stays, because `v1` remains the rollback policy. ### Fixes that the evaluation runs forced - `PLANNER_AI_RATE_LIMIT` makes the AI request ceiling configurable, still defaulting to 30. The interactive limit of 30 per five minutes cannot accommodate a batch job that spends one request per canary and one per judged case. - The judge grades one case per request. A five case batch had to return roughly 115 grade objects and the model dropped some; batching saved no tokens here, because candidate text is per case and shared by nothing. - The judge prompt fences the stored query and names it as data. It previously guarded candidate texts as historical data but pasted the query in raw, and a stored agent prompt carrying its own output-format instructions steered the judge into a schema-valid but empty response. - An unjudgeable case is skipped and counted rather than destroying a paid run, and the count is surfaced. ### Failures found in the suite - `tests/logging.test.ts` was failing on `main` before this branch. The agent CLI is its own binary but shares `logger.ts` and, through it, `clientAddress.ts`; both validated server configuration at import, so a malformed `PLANNER_LOG_LEVEL` or `PLANNER_TRUSTED_PROXIES` aborted a CLI that never reads either. Server settings are now ignored in an agent-cli distribution, the proxy list is parsed on first use, and the server validates it explicitly at startup so fail-fast becomes a deliberate step rather than an import side effect. - The browser suite failed intermittently under full-suite contention, most often `SettingsDialog`. Seventy jsdom environments share one machine and the default five second budget is not enough for a user-event test waiting on a scheduler slot; the file passes alone. Raised to twenty seconds, which still catches a hang. ## Verification Full typecheck, type-aware oxlint, eslint, knip, oxfmt, 951 bun tests, and 224 vitest tests all pass. `tests/logging.test.ts` passes for the first time.
The per-account limit of 30 requests per five minutes is sized for a
person clicking through the UI. Offline batch work needs far more: a
retrieval-eval dataset refresh spends one request per canary and a full
judge run spends one per batch, so both exceed the ceiling and fail
partway. The limit now reads PLANNER_AI_RATE_LIMIT, still defaulting to
30, so an operator can raise it for the duration of such a run.
A five-case batch had to come back with a grade for every candidate of
every case in it, roughly a hundred and fifteen grade objects, and the
judge started dropping some: the full run failed at 10 of 129 cases with
an incomplete batch. Batching never saved tokens here, because candidate
text is per-case and shared by nothing, so it only ever reduced request
count for the rate limiter. One case per request keeps each response
short enough to complete.

Batch size joins the judge configuration fingerprint, since how many
cases share a request changes what the judge sees while grading one.
The judge prompt guarded candidate texts as historical data but dropped
the stored query in raw, and this corpus is full of agent prompts that
carry their own output-format instructions. A query telling the model to
answer in some other shape gets a schema-valid but empty cases array
back, which reads as an incomplete batch. The query is now delimited and
named as data alongside the candidates, and the prompt asks explicitly
for one entry per case and one grade per candidate.

An incomplete answer no longer destroys the run either. It retries, then
leaves the case unjudged and counts it, and the count travels through
the judgment record to the settings view, so a partial judgment is
visible rather than silent. The failure logs candidate and grade counts,
which separates an answer that came back in the wrong shape from one
that ran out of room.
It existed to answer one question: does the V2 recall policy lose
memories that V1 would have surfaced. On 125 real cases it answered no,
at identical canary recall and a quarter of the harmful selections, and
V2 is now the promoted policy. The subsystem was built to be deleted
once it had said that, so it goes: the job runner, dataset builder,
retrieval runner, metrics, encrypted state record, the session-only
routes, and the settings tab.

The judge only ever ran here, offline and behind an explicit billable
confirmation. Nothing in the recall path changes, and
buildLegacyMemoryContext stays because v1 remains the rollback policy.
The agent CLI is its own binary but shares logger.ts, and through it
clientAddress.ts. Both validated server configuration while being
imported, so PLANNER_LOG_LEVEL or PLANNER_TRUSTED_PROXIES being
malformed aborted the CLI at startup over settings it never reads. The
log level, retention, and slow-request budgets are now ignored outright
in an agent-cli distribution rather than validated, and the trusted
proxy list is parsed on first use.

The server still refuses to start on a malformed proxy list: it
validates explicitly once the runtime mode is set, so fail-fast moves
from a side effect of importing a module to a deliberate startup step.

Fixes the failing isolation assertion in tests/logging.test.ts.
Seventy jsdom environments share one machine, so a user-event driven
test can spend seconds waiting for a scheduler slot that it gets
instantly when the file runs alone. The default five second budget
turned that into failures that moved between runs; SettingsDialog was
the one that lost most often. Twenty seconds still catches a hang.
Collaborator

kReview review

Verdict: no findings

No findings to address in the reviewed diff.

Overall risk is low; no concrete correctness, security, or data-retention defect is established by the visible changes.

Reviewed by kReview at 8e2fc506e1. This comment is conservative and based only on the PR diff, metadata, and supplied repository context.

Est. cost ~$4.07 total (2.6M in / 8.9k out) · this run ~$0.32 (65.4k in / 383 out) / gpt-5.6-sol.

<!-- codex-forgejo-review --> <!-- codex-forgejo-review-head:8e2fc506e14908a5b8fd21055fdba3971afa33e2 --> ## kReview review **Verdict:** no findings No findings to address in the reviewed diff. Overall risk is low; no concrete correctness, security, or data-retention defect is established by the visible changes. _Reviewed by kReview at `8e2fc506e1`. This comment is conservative and based only on the PR diff, metadata, and supplied repository context._ _Est. cost ~$4.07 total (2.6M in / 8.9k out) · this run ~$0.32 (65.4k in / 383 out) / gpt-5.6-sol._ <!-- codex-forgejo-review-state:eyJoZWFkU2hhIjoiOGUyZmM1MDZlMTQ5MDhhNWI4ZmQyMTA1NWZkYmEzOTcxYWZhMzNlMiIsInN1bW1hcnkiOiJPdmVyYWxsIHJpc2sgaXMgbG93OyBubyBjb25jcmV0ZSBjb3JyZWN0bmVzcywgc2VjdXJpdHksIG9yIGRhdGEtcmV0ZW50aW9uIGRlZmVjdCBpcyBlc3RhYmxpc2hlZCBieSB0aGUgdmlzaWJsZSBjaGFuZ2VzLiIsImZpbmRpbmdzIjpbXSwiY3VtdWxhdGl2ZUNvc3QiOnsidXNkIjo0LjA2OTU2MiwiaW5wdXRUb2tlbnMiOjI2NDIwMTYsIm91dHB1dFRva2VucyI6ODg3MiwibW9kZWwiOiJncHQtNS42LXNvbCJ9fQ== -->
Removing the subsystem removed every path that could read or delete the
record it kept per user, and that record holds the stored prompts the
dataset was built from. It is blind-indexed, so no operator query can
reach it either: only code holding the user's keys can address it. Left
alone the data would simply outlive the feature.

Startup now deletes the record and its offline-recovery file once per
user, guarded so a failure logs rather than blocking a boot, and
idempotent so later starts are no-ops. The module can go once every
deployment has started a build containing it.
The sweep had a single outer catch, so a user whose keys fail to load or
whose delete transaction errors ended the loop and left the private
evaluation data of every account after them in place. Each account is
now isolated: a failure is reported and the sweep continues.

The failure report also had to change shape. The log allowlist keeps
only the fields it names and drops every key matching /error/i outright,
so the previous message would have arrived empty; it now reports through
failureClass and code, and the success count through count.
kleb merged commit 3cb9503806 into main 2026-08-26 00:59:42 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
knopersikcuo/Planner!115
No description provided.