PR #5: fixes verified, one minor finding
F61-F63 are fixed at 421c043. No new merge blocker. One nonblocking P2 remains in exceptional evaluation accounting.
1 open P2; 3 findings fixed. No GitHub review comments posted.
+1158 −46 421c043Reviewed on 23 September 2026 at 421c04337163ce18eeb93f5fd7cc30649c0ef93c. Incremental scope: f3fd677..421c043, +236 −13. PR base remains main. PR #5 has not been merged.
Architecture and complexity
The context projection and shared workflow-fault helper are appropriate changes. They use existing frame and workflow contracts without another service or a new routing layer. The outer evaluation fallback is the one unnecessary second representation of a failed run: it lacks the runner state needed for accurate metrics. F64 is a localized correction, not a reason to redesign the package. The read-only Jev scope is otherwise ready; this review does not establish real-model semantic quality.
Verification
Typecheck and all 379 repository tests pass: 323 evals, 40 journal-file and 16 perception-jev. The 133-case prior independent run has 130 passes and the same three inherited decoder failures. Six additional round-2 probes have five passes and one failure supporting F64. Combined: 135 passed, 4 failed, 139 tests. Independent probe typecheck and git diff --check pass. No live model call was made.
All four callback controls pass: specs, reduce, stop and resolveStatus. The context isolation control passes. The failing new case uses a custom in-process backend, not ordinary model JSON through the Jev wrapper.
Review JSONReview textRound-2 probesRound-2 resultsPrior probes rerun379 repository testsRound-1 archive
Round-1 assessment and verification, historical
PR #5 needs changes
PR #4 is merged. PR #5 rebased cleanly, but the model request omits the context needed to answer its questions.
3 open findings: 1 P1 and 2 P2. No GitHub review comments posted.
+930 −41 f3fd677Reviewed 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.
- The Jev backend validates specs, sends a detached request, validates client output and returns normalized answers.
- Evaluation runs the existing workflow against frames and aggregates outcomes, failures and latency.
- The scripted client matches decision ID sets, with one-shot frame-bound scripts and reusable unbound templates.
- The package does not call the kernel, authorities, executors or journal. The real SDK host wrapper and semantic interpreter remain out of scope.
Round 2 changes
F61-F63 verified fixed. One new nonblocking P2 concerns accounting in the evaluator fallback.
Assembled semantic context now reaches the client as a detached frozen projection. Ordinary callback exceptions are recorded inside the runner; an outer catch synthesizes a fallback run.
F61 passes N1 and the nested isolation control. F62 passes N5 and N6. F63 passes N7 and all four callback controls.
Files changed at round 1
| File | Change |
|---|---|
| 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.ts | modified |
| packages/perception/src/validation.ts | modified |
| pnpm-lock.yaml | +22 |
Issues: 1 open, 3 fixed
| ID | Priority | Finding |
|---|---|---|
| F64 · open | P2 | Preserve accounting when the evaluation fallback catches a runner exception |
| F61 · fixed | P1 | Send the materialized context to the model client |
| F62 · fixed | P2 | Count backend attempts and explicit error results accurately |
| F63 · fixed | P2 | Preserve 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.
Round-1 data flow, historical
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
Round-2 review comment
Round-1 comments, fixed
Fixed in 421c043 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
Fixed in 421c043 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
Fixed in 421c043 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: });
P2 F64. Preserve accounting when the evaluation fallback catches a runner exceptionpackages/perception-jev/src/evaluation.ts:91-116logiccorrectness
The new outer catch creates a fresh WorkflowRun with calls and latency set to zero, empty traces, and a workflow-error reason. An exception escaping the runner does not establish that no backend call occurred or that the workflow caused it. This fallback now returns invented accounting as a completed evaluation result.
The independent reproduction uses the public PerceptionBackend interface. Its first response has an answers getter that throws during decoding; the second frame succeeds. Both backend calls happen, but the evaluation reports calls=1, backendFailures=0 and workflowFailures=1. The getter escaping the shared decoder is an inherited limitation already documented in round 1. The new defect is the fallback turning that exception into an inaccurate, apparently complete metrics report.
This is a nonblocking P2 for PR #5. The normal Jev wrapper catches this response-access failure at its backend boundary, and the four ordinary workflow callback paths now retain their accounting correctly. Preserve the available run state at the runner boundary and attribute decoder exceptions there, or represent unknown accounting explicitly instead of supplying zero. Avoid adding a second counter system in the evaluator.
Fix direction. Keep exception containment where the runner owns accumulated state. Record a decoder failure at the backend-output boundary and preserve counts, traces and known latency. Do not silently classify every escaping exception as a workflow failure with zero calls.
Prompt for AI agents