PR #5 needs changes

PR #4 is merged. PR #5 rebased cleanly, but the model request omits the context needed to answer its questions.

Changes requested

3 open findings: 1 P1 and 2 P2. No GitHub review comments posted.

+930 −41 f3fd677

Reviewed on 23 September 2026 at f3fd677b378518161518769af0f99803b18ccb1d, based on bfbb2b02334fee04163e12910c8ec515afaecc82. PR #4 merged as bfbb2b0. PR #5 rebased without conflicts and was pushed with an explicit lease. Range-diff confirms the rebased commit is patch-identical; GitHub now lists main as its base.

Architecture and complexity

The package boundary is sensible. It wraps an injected client, keeps policy in Circe and exposes a read-only evaluation path. I found no production call into the kernel or an executor in this package. A scripted client is useful as a protocol double, and leaving the real interpreter out of scope is honest.

The main architectural gap is information flow. Frame identity is useful for tracing, but it is not model evidence. The sole client request needs the bounded semantic content already present in the assembled frame. This can be fixed without another service, global frame registry or language-specific rules.

The evaluation wrapper is small and appropriately placed, but its failure accounting and per-frame error handling need to match its stated purpose. Correct those contracts before using it to judge a model. The static import test is a useful guard against accidental coupling, not a sandbox for arbitrary injected client code.

Verification

Repository typecheck passes. All 375 repository tests pass: 323 evals, 40 journal-file and 12 perception-jev tests. All 122 prior independent review probes also pass on PR #5. The new nine-case suite has seven failures and two passing controls; four failures support the three PR-specific findings. Three failures concern inherited accessor behavior, confirmed separately by two baseline tests. The independent suite typechecks.

N8 verifies detached Jev answers and ambiguous-status gating. N9 verifies ordinary two-frame metrics and latency. No live model call was made; the injected-client seam and workflow behavior were exercised directly.

Review JSONReview textReproductionsNew resultsInherited-case baseline check122 prior probes375 repository tests

Walkthrough

PR #5 introduces an injected Jev decision client, a perception backend wrapper, a read-only evaluation loop and a scripted model double. It also changes the shared perception decoder and adds latency to workflow results.

Files changed
FileChange
packages/perception-jev/src/backend.ts+135
packages/perception-jev/src/evaluation.ts+111
packages/perception-jev/src/scripted-client.ts+53
packages/perception-jev/src/perception-jev.test.ts+450
packages/perception-jev/src/index.ts+3
packages/perception-jev/package.json+26
packages/perception-jev/tsconfig.json+8
packages/perception/src/orchestrator.tsmodified
packages/perception/src/validation.tsmodified
pnpm-lock.yaml+22
Issues
IDPriorityFinding
F61P1Send the materialized context to the model client
F62P2Count backend attempts and explicit error results accurately
F63P2Preserve evaluation results when a workflow callback fails
Inherited decoder limitations, excluded from new findings

N2-N4 expose limitations in the claimed one-read decoder: status is read repeatedly, an accessor can change AMBIGUOUS to RESOLVED, and a throwing answer getter escapes workflow failure recording. Separate tests against the merged PR4 code reproduce the same workflow failures. They are inherited limitations, not new PR5 regressions, and are excluded from actionable counts. The new analyzePerceptionResult API still does not establish the advertised full-envelope one-read guarantee. The Jev wrapper itself snapshots status once, so its ordinary status path passes N8. These are custom in-process accessor cases, not ordinary parsed model JSON.

Data flow

Assembled frame + decision specs -> Jev request -> injected client -> decode -> workflow policy -> evaluation metrics. The current request drops semantic frame content.

Prompt for all review comments · Ctrl+`
Prompt to re-run review · Ctrl+Space
Review comments


P1 F61. Send the materialized context to the model clientpackages/perception-jev/src/backend.ts:92-104logiccorrectness
The backend reduces ContextFrame to four identity fields and coverage flags before calling its only injected client. The utterance, candidate attributes, relations, events, interaction state and capability semantics never reach that interface. Decision specs describe the bounded questions; they do not necessarily contain the evidence needed to answer them.
N1 assembles a real frame for “continue the payment worker” with two materialized candidates. The request contains the generic question “Which candidate does the phrase refer to?” and option IDs A, B and none, but neither the user's phrase nor either candidate's title. This is the same valid spec shape the retained integration tests use. An SDK wrapper implementing only complete(request) cannot infer the intended answer from those IDs and hashes.
Leaving a real interpreter out of scope is reasonable, but the model seam must carry its inputs. Requiring each host to secretly fetch frames by ID or embed all context into every question creates an undocumented second path. Pass an immutable, detached, bounded frame projection with the evidence already assembled by Circe.
Fix direction. Include the utterance and the assembled semantic context in JevDecisionRequest, preserving frame identity and retrieval completeness. Detach and freeze the full request. Keep the client read-only; do not add store access or language rules.
Probes: N1
92:       const request: JevDecisionRequest = {
93:         model: options.model,
94:         frame: {
95:           id: frame.id,
96:           hash: frame.hash,
97:           interactionId: frame.interactionId,
98:           observationId: frame.observationId,
99:         },
100:         // Detached and frozen: the client cannot rewrite the policy inputs
101:         // the orchestrator validated and will evaluate.
102:         decisions: deepFreeze(structuredClone(specs)),
103:         coverage: deepFreeze(structuredClone(frame.assembly.completeness)),
104:       };
Prompt for AI agents


P2 F62. Count backend attempts and explicit error results accuratelypackages/perception-jev/src/evaluation.ts:88-99logiccorrectness
The new evaluation metrics copy run.calls and classify failures only from run.failures. run.calls increments after a successful backend return, so a client invocation that throws is not counted. A well-formed result whose status is ERROR also creates no DecisionFailure, so backendFailures stays zero.
N5 invokes the Jev client twice, with one transport failure and one success. The evaluation returns two runs and one backend failure but reports only one call. N6 returns an explicit model ERROR result; errorRuns becomes one while backendFailures is zero. These are ordinary failure paths, not malformed JavaScript objects.
The harness is meant to support model evaluation before enabling execution. Its call counts and backend failure totals currently understate failures and usage. Count attempted calls at the invocation boundary, and represent explicit backend ERROR statuses consistently without double counting thrown or malformed responses.
Fix direction. Count every attempted backend call, including exceptions, and attribute explicit backend ERROR results as backend failures. Keep run counts separate from failure-event counts and document latency semantics for unsuccessful calls.
Probes: N5, N6
88:     metrics.calls += run.calls;
89:     metrics.errors += run.errors.length;
90:     metrics.totalLatencyMs += run.latencyMs;
91:     metrics.runStatuses[run.status] = (metrics.runStatuses[run.status] ?? 0) + 1;
92:     if (run.status === "ERROR") metrics.errorRuns += 1;
93:     for (const failure of run.failures) {
94:       if (failure.reason === "backend-error" || failure.reason === "malformed-backend-output") {
95:         metrics.backendFailures += 1;
96:       } else {
97:         metrics.workflowFailures += 1;
98:       }
99:     }
Prompt for AI agents


P2 F63. Preserve evaluation results when a workflow callback failspackages/perception-jev/src/evaluation.ts:78-86logicstability
The evaluation loop awaits each workflow without an exception boundary. The workflow runner records some structural faults, but exceptions from specs, reduce, stop or resolveStatus propagate. A callback error therefore escapes the new batch evaluation API instead of contributing a workflow failure.
N7 evaluates two frames with a reducer that throws only for the first. runReadOnlyEvaluation rejects immediately, returns no metrics or runs, and never evaluates the second frame. A successful backend response is available, so this is a workflow fault rather than a model failure.
For a read-only corpus evaluation, one faulty frame should remain visible in the report instead of discarding the batch. Record a failed run with the correct attribution and retain completed runs while continuing the remaining frames. If a deliberate fail-fast mode is wanted, make it explicit and preserve the accumulated diagnostic output.
Fix direction. Convert workflow callback exceptions into structured per-run workflow failures, then continue the evaluation batch. Preserve frame/run identity and distinguish workflow errors from backend errors.
Probes: N7
78:   for (const [index, frame] of input.frames.entries()) {
79:     const run = await runPerceptionWorkflow({
80:       backend: input.backend,
81:       frame,
82:       workflow: input.workflow,
83:       policy: input.policy,
84:       now: input.now,
85:       runId: runIdFor(frame, index),
86:     });
Prompt for AI agents