🤖 feat(workflows): evaluation adapter, step admission schema and evaluation model default (L3a) - #4324
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: ba77f82e48
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3846af8324
ℹ️ 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".
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3846af8324
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 786ff21362
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f1d0fbcfa
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abdd1ad5e3
ℹ️ 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".
abdd1ad to
cb3ef0a
Compare
cb3ef0a to
d570f62
Compare
…tion model default (L3a) Adds the headless pieces the workflow `evaluate()` step (L3b) wires into WorkflowRunner, without touching the runner yet: - `WorkflowStepRecordSchema.evaluation` (persisted admission) and a new `evaluation` run event with a finite failure identity (`EvaluationStepFailureReason`/`Code`); exhaustive UI switches extended. - `evaluationDefaults.model` app-config field with load/save normalization, `getConfig` projection and `config.updateEvaluationDefaults`. - `WorkflowEvaluationAdapter`: selection precedence (per-call > CLI override > Settings default; persisted selection + fingerprint check on later attempts), lazy dispatch through `runEvaluationToOutcome`, and a never-throwing headless usage write tagged `workflow_evaluation`. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$244.23`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=244.23 -->
…and never price unknown counts as zero Codex round 1 on the L3a adapter: - `requestAnalyticsIngest?` hook (same as AgentStatusService) called when `recordHeadlessUsage` returns a row; headless runs have no stream-end. - Usage with an unknown input or output count is skipped with the fixed code `evaluation-ledger-skipped-unknown-usage` instead of being recorded with the missing component priced at zero; a missing total is derived. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$244.23`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=244.23 -->
…ct blank per-call models Codex round 3 on the L3a adapter: - `projectWorkflowRun` takes an evaluation step's title from the `evaluation` event like it does for agent-step/task events. - `WorkflowEvaluateSpecSchema.model` rejects blank strings so a blank per-call model cannot shadow the CLI/Settings default; the adapter asserts the same invariant before applying precedence. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$244.23`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=244.23 -->
Codex round 4 on L3a: the `evaluation` run event requires a non-empty title when present, so `WorkflowEvaluateSpecSchema.title` now refuses blank strings up front (like `id` and `model`) instead of letting the event append fail after admission. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$244.23`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=244.23 -->
…ge row does not land Codex round 5 on L3a: `recordHeadlessUsage` swallows its own write failures and resolves `undefined` (also its tombstoned-workspace value), so the adapter's catch never ran in production. The adapter now logs `evaluation-ledger-not-recorded` with run/step-digest/attempt context whenever no row was recorded and wakes analytics ingestion only when one was; the catch stays as a defensive backstop. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$244.23`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=244.23 -->
…ing.evaluationModel Main landed #4307 (auto model routing) with its own `autoModelRouting.evaluationModel` classifier setting while this stack was in review. The two settings stay independent by design (different defaults, admission and billing semantics); say so at the schema so neither grows a fallback onto the other. _Generated with `xum` • Model: `coder:anthropic/claude-fable-5-1` • Thinking: `xhigh` • Cost: `$244.23`_ <!-- mux-attribution: model=coder:anthropic/claude-fable-5-1 thinking=xhigh costs=244.23 -->
d570f62 to
febb058
Compare
1 participant
Summary
Layer L3a of the workflow
evaluate()plan (stack: L1 SDK pins → L2a schemas → L2b resolver +EvaluationService→ L3a adapter + persistence schemas + config → L3b runnerevaluate()+ CLI → L4 Settings/timeline → L5 TypeSafe → L6 example). Adds the headless pieces L3b wires intoWorkflowRunner, without touching the runner, the sandbox API, or any construction site yet: the persisted admission on step records and anevaluationrun event, theevaluationDefaults.modelapp-config field, andWorkflowEvaluationAdapter(selection, dispatch, accounting). Nothing user-visible changes in this PR.Implementation
src/common/orpc/schemas/workflow.ts):WorkflowStepRecordSchema.evaluation?: EvaluationAdmissionSchema(the L2a admission; written atomically with thestartedrecord and carried on every later record so the latest-wins merge always retains it) and a newevaluationevent (started|completed|failed|cached, display data only;title/modelString/responseModelIdare untrusted display metadata per the plan's security model). Failure identity is a finite allowlist:EvaluationStepFailureReason/Codeinsrc/common/types/evaluation.ts= the L2 service reasons +deadline,unauthorized,admission-mismatch,admission-missing,attempts-exhausted(+ codesno-model,unsupported-provider|route,unknown-model). The exhaustiveWorkflowRunEventswitches inWorkflowRunToolCall.tsxandprojectWorkflowRun.tsgain the new case (label, detail = usage, tone, step-bearing).EvaluationDefaultsSchema { model? }on disk,ProjectsConfig.evaluationDefaults, load/save normalization (trimmed; blank clears the block so no stale{}lingers),getClientConfig()/config.getConfigprojection, andconfig.updateEvaluationDefaults({ model })(router +Config.updateEvaluationDefaults), mirroringupdateHeartbeatDefaultPrompt. The L4 Settings card reads/writes this.WorkflowEvaluationAdapter(src/node/services/workflows/WorkflowEvaluationAdapter.ts), constructed from{ evaluationService, aiService, sessionUsageService, config, workspaceId, evaluationModelOverride? }:resolveSelection(spec, persisted?): first attempt = per-callmodel> CLI--evaluation-model> Settings default (none ⇒invalid-input/no-model, no resolver call); later attempts use the persisted selection only and refuse a changedconfigFingerprint(admission-mismatch); every call re-reads credentials so revocation surfaces asunauthorized. Resolver rejections map through an exhaustiveRecord.dispatch(pinned, call, { runtimeAbortSignal, attemptDeadlineAt })→EvaluationOutcomevia L2b'srunEvaluationToOutcome; the service call is built insideEffect.suspendso an already-aborted or already-expired attempt constructs nothing.recordUsage(pinned, result, { runId, stepDigest, attempt }): onerecordHeadlessUsage(..., { analyticsSource: "workflow_evaluation", metadataModel })withnullcounts dropped and only object-shaped usage metadata forwarded; never throws.recordHeadlessUsageswallows its own write failures (logs the cause, resolvesundefined— the same value as for a removal-tombstoned workspace), so the adapter logs the fixed codeevaluation-ledger-not-recordedwith run/step-digest/attempt context whenever no row landed, wakes analytics ingestion only when one did, and keeps a defensiveevaluation-ledger-failedcatch for an unexpected throw (digest/number fields only, no error text). Usage with an unknown input or output count is skipped (evaluation-ledger-skipped-unknown-usage) rather than priced as zero.Validation
WorkflowEvaluationAdapter.test.ts(fake resolver/service/ledger/config): precedence per-call > override > default; no model ⇒no-modelwith zero resolver calls; all four resolver rejections mapped; persisted selection wins over a changed default and override; fingerprint mismatch ⇒admission-mismatch; revocation on resume ⇒unauthorized; blank override rejected at construction; dispatch passes the pinned instance and provider options and classifies success/typed failure (429); already-fired abort ⇒interruptedand elapsed deadline ⇒deadlinewith zero service calls; ledger row shape + analytics ingest wake; unknown input/output ⇒ no row +skippedcode (missing total derived); ledgerundefined⇒not-recordedcode with step context and no ingest; an unexpected throw ⇒failedcode and the thrown text never reaches the log line (sentinel); blank per-call model refused with zero resolver calls.schemas/workflow.test.ts: admission-bearing and legacy step records both parse;evaluationevent lifecycle round-trips; a free-textreasonis rejected.appConfigOnDisk.test.ts+tests/ipc/config/evaluationDefaults.test.ts(trim, clear on blank/null,getConfigprojection through the real oRPC router).make static-checkgreen;bun test src/node/services/workflows src/common/orpc/schemas src/browser/features/RightSidebar/Workflows src/node/services/evaluation518/0.Follow-ups
evaluationevents (Codex round 5, thread, deferred):getWorkflowDisplayRowsinWorkflowRunToolCall.tsxcoalesces task/workflow/patch lifecycles into one row per step but currently renders eachevaluationevent as its own row (and a terminal event withouttitlefalls back to the step id). No runner emits these events until L3b, so nothing is user-visible yet; the plan's L4 layer (timeline rendering) owns this and will coalesce bystepId/inputHash/attempt, keeping thestartedtitle and showing the latest status.EvaluationServicerejects the output (invalid-output), the response's usage is dropped with the error, so that spend is not ledgered. The plan's v1 contract records usage only for completed attempts; carrying sanitized usage onEvaluationError→EvaluationOutcome→ a post-recordStepFailedledger write is a cross-layer change (L2b error contract + L3b runner) to be delivered as its own stacked follow-up.Rebase note (2026-09-22)
Rebased onto
mainafter #4307 landed (see #4322 for the two-factories decision). #4307 also addedautoModelRouting.evaluationModel(the auto-router's classifier, defaulting to TypeSafe's Jev). It is independent of this PR'sevaluationDefaults.model(explicit workflow evaluator with its own admission and billing semantics); the schema comment says so and neither setting falls back to the other. Unifying the two Settings surfaces, if ever, is L4/L5 territory.Risks
Low. Additive schema fields (optional; existing records parse unchanged — the accepted downgrade scope is that a previous release cannot read runs containing the new event, which no run can contain until L3b exists), an additive config field, and a new class with no callers yet. The only edits to existing behavior are the new
case "evaluation"branches in two UI switches.Generated with
xum• Model:coder:anthropic/claude-fable-5-1• Thinking:xhigh• Cost:$244.23