Skip to content

Emit eval outcomes and post V4 results - #1016

Merged
realtonyyoung merged 3 commits into
mainfrom
tonyyoung/ai-2744-eval-v4
Sep 18, 2026
Merged

realtonyyoung merged 3 commits into
mainfrom
tonyyoung/ai-2744-eval-v4

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

AI-2744 — no kcap-cli GitHub issue exists for this (driven from the Linear issue); the server half is kurrent-io/kcap-server#1939.

What & why

The server now records an honest per-question evaluation outcome (assessed / insufficient_evidence / not_applicable) with a nullable score and a versioned evidence-coverage record, accepted at POST /api/sessions/{id}/evals/v4. This teaches the CLI judge and the dashboard-triggered daemon to emit the same: ParseVerdict yields an outcome-bearing assessment (an absent outcome reads as assessed, a present malformed one is a parse failure), a failed judge invocation becomes a coded failure instead of a silent null, Aggregate builds a V4 payload whose overall is the mean of assessed scores only (null when none scored), and the run POSTs V4. The daemon advertises eval protocol 2 at connect and answers the new RunQuestionV2/FinalizeEvalV2 RPCs; the V1 RPCs and V3 posting stay for an older server.

Where to look

ReconcileEvidenceCoverage — a complete coverage record cannot accompany an insufficient_evidence outcome (the server validator rejects that self-contradiction and 400s the whole run), so it is nulled to "unknown". The failure taxonomy in ClaudeCliRunner.RunDetailedAsync. DaemonConnect sending eval_protocol_version: 2 (snake_case, pinned by a golden fixture). This PR merges only after the server route is deployed; until the submodule pin bump and npm release, the CLI still posts V3 and reads as fully assessed.

Verification

Core and integration suites green, including EvalRunnerV4PostTests (exactly one POST to /evals/v4, none to /evals/v3), the D13 parser cases (absent vs present-null vs unknown outcome), the ReconcileEvidenceCoverage cases, and the eval_protocol_version golden fixture; dotnet publish -c Release is clean for both kcap and kcap-daemon (no AOT/trim warnings).

🤖 Generated with Claude Code

realtonyyoung and others added 2 commits September 18, 2026 07:50
Judges report assessed/insufficient_evidence/not_applicable instead of
always scoring, and a failed judge invocation carries a coded reason
(judge_timeout/chat_error/verdict_parse_failed) instead of vanishing.
The daemon negotiates eval protocol 2 (RunQuestionV2/FinalizeEvalV2)
on DaemonConnect and posts the aggregate to /evals/v4; a protocol-1
server still gets the legacy verdict-only RPCs and /evals/v3.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A complete coverage record on an insufficient_evidence question
self-contradicts the server's validator and 400s the whole run's
POST, discarding every correctly-scored question alongside it — null
it in that one case, mirroring the server producer's reconciliation.
ParseVerdict now tells an absent outcome (defaults to assessed) apart
from an explicit JSON null (a parse failure); both bound to the same
C# null before.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 18, 2026

Copy link
Copy Markdown

AI-2744

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add outcome-aware V4 evaluations and daemon protocol 2

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Emit outcome-aware assessments, coded failures, and reconciled evidence coverage.
• Aggregate assessed scores and persist V4 evaluation payloads.
• Negotiate daemon protocol 2 while retaining legacy V1/V3 compatibility.
Diagram

sequenceDiagram
    actor User
    participant CLI as kcap CLI
    participant Server as kcap Server
    participant Daemon as Eval Daemon
    participant Judge as Claude Judge
    alt Local CLI run
        User->>CLI: Start evaluation
        CLI->>Judge: Judge questions
        Judge-->>CLI: Assessments or failures
        CLI->>Server: POST evals v4
    else Dashboard daemon run
        Server->>Daemon: RunQuestionV2
        Daemon->>Judge: Judge question
        Judge-->>Daemon: Assessment or failure
        Server->>Daemon: FinalizeEvalV2
        Daemon->>Server: POST evals v4
    end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Immediate protocol 2 cutover
  • ➕ Removes legacy RPC conversion and V3 finalization paths.
  • ➕ Reduces branching in daemon evaluation handling.
  • ➖ Breaks compatibility with servers deployed before protocol 2.
  • ➖ Requires tightly coordinated CLI, daemon, and server releases.
2. Extend protocol 1 in place
  • ➕ Avoids adding parallel RunQuestionV2 and FinalizeEvalV2 handlers.
  • ➕ Keeps fewer SignalR method names.
  • ➖ Changes existing wire semantics for older peers.
  • ➖ Makes nullable scores and new failure shapes ambiguous during mixed-version operation.

Recommendation: Keep the PR's explicit protocol negotiation and parallel V1/V2 handlers. The extra compatibility code is justified because it preserves old-server behavior without weakening V4 outcome semantics or fabricating scores for unassessed questions.

Files changed (35) +1571 / -119

Enhancement (19) +889 / -104
EvalCategoryAssessment.csAdd V4 category assessment contract +12/-0

Add V4 category assessment contract

• Defines category-level nullable scores, verdicts, and outcome-aware question collections matching the server contract.

src/Capacitor.Cli.Core/Eval/Contracts/EvalCategoryAssessment.cs

EvalEvidenceCitation.csAdd evidence citation contract +12/-0

Add evidence citation contract

• Introduces citation references with optional digests for V4 evidence coverage payloads.

src/Capacitor.Cli.Core/Eval/Contracts/EvalEvidenceCitation.cs

EvalEvidenceCoverage.csAdd versioned evidence coverage contract +26/-0

Add versioned evidence coverage contract

• Models consulted sources, unavailable sources, citations, omissions, and stop reasons. Adds an in-process completeness calculation.

src/Capacitor.Cli.Core/Eval/Contracts/EvalEvidenceCoverage.cs

EvalEvidenceOmission.csAdd evidence omission contract +20/-0

Add evidence omission contract

• Defines typed evidence-loss records with optional count, reference, and detail fields.

src/Capacitor.Cli.Core/Eval/Contracts/EvalEvidenceOmission.cs

EvalFailureCodes.csDefine coded evaluation failures +9/-0

Define coded evaluation failures

• Centralizes server-compatible codes for judge timeouts, chat errors, and verdict parsing failures.

src/Capacitor.Cli.Core/Eval/Contracts/EvalFailureCodes.cs

EvalOutcomes.csDefine supported question outcomes +13/-0

Define supported question outcomes

• Adds constants and validation membership for assessed, insufficient-evidence, and not-applicable outcomes.

src/Capacitor.Cli.Core/Eval/Contracts/EvalOutcomes.cs

EvalQuestionAssessment.csAdd outcome-aware question assessment +30/-0

Add outcome-aware question assessment

• Introduces the V4 per-question shape with nullable scoring, findings, prompt metadata, and optional evidence coverage.

src/Capacitor.Cli.Core/Eval/Contracts/EvalQuestionAssessment.cs

EvalQuestionFailure.csAdd coded question failure contract +27/-0

Add coded question failure contract

• Represents questions that produced no verdict using stable codes and optional execution diagnostics.

src/Capacitor.Cli.Core/Eval/Contracts/EvalQuestionFailure.cs

FinalizeEvalV2Command.csAdd protocol 2 finalization command +11/-0

Add protocol 2 finalization command

• Carries collected assessments, coded failures, and model metadata from the server to the daemon.

src/Capacitor.Cli.Core/Eval/Contracts/FinalizeEvalV2Command.cs

QuestionResultV2.csAdd protocol 2 question result +14/-0

Add protocol 2 question result

• Returns either an assessment or coded failure, plus diagnostics and token counts, over SignalR.

src/Capacitor.Cli.Core/Eval/Contracts/QuestionResultV2.cs

SessionEvalCompletedPayloadV4.csAdd V4 evaluation report payload +26/-0

Add V4 evaluation report payload

• Defines categories, nullable overall scoring, outcome counts, coded failures, retrospective data, and coverage versions for the V4 endpoint.

src/Capacitor.Cli.Core/Eval/Contracts/SessionEvalCompletedPayloadV4.cs

EvalService.csImplement outcome-aware V4 evaluation pipeline +399/-38

Implement outcome-aware V4 evaluation pipeline

• Parses and validates outcomes, maps judge failures, derives and reconciles evidence coverage, and aggregates assessed scores only. Adds V4 retrospective and persistence flows while retaining legacy V3 finalization.

src/Capacitor.Cli.Core/Eval/EvalService.cs

IEvalObserver.csMigrate observers to V4 assessments +6/-3

Migrate observers to V4 assessments

• Changes question completion and final completion callbacks to expose outcome-aware assessments and V4 aggregates.

src/Capacitor.Cli.Core/Eval/IEvalObserver.cs

ClaudeCliRunner.csExpose detailed Claude invocation failures +49/-10

Expose detailed Claude invocation failures

• Adds typed timeout, process, and parse failure outcomes while preserving the existing null-on-failure adapter for legacy callers.

src/Capacitor.Cli.Core/Harness/Claude/ClaudeCliRunner.cs

Models.csRegister V4 wire models and protocol metadata +54/-12

Register V4 wire models and protocol metadata

• Adds compaction loss signals, source-generated serialization registrations, nullable progress scores, and the daemon evaluation protocol version.

src/Capacitor.Cli.Core/Models.cs

prompt-eval-question-tools.txtTeach judges to emit evaluation outcomes +14/-3

Teach judges to emit evaluation outcomes

• Updates structured output instructions for outcome-dependent nullable scores and explains when evidence is insufficient or a question is inapplicable.

src/Capacitor.Cli.Core/Resources/prompt-eval-question-tools.txt

EvalRunner.csHandle protocol 2 evaluation RPCs +108/-23

Handle protocol 2 evaluation RPCs

• Adds RunQuestionV2 and FinalizeEvalV2 handlers, preserves protocol 1 conversion, and prevents duplicate per-question progress events during server orchestration.

src/Capacitor.Cli.Daemon/Services/EvalRunner.cs

ServerConnection.csAdvertise and register evaluation protocol 2 +26/-4

Advertise and register evaluation protocol 2

• Registers protocol 2 SignalR handlers, advertises version 2 during daemon connection, and relays nullable outcome-aware progress.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

EvalCommand.csRender V4 evaluation outcomes +33/-11

Render V4 evaluation outcomes

• Displays unassessed outcome markers, nullable category and overall scores, evidence omissions, and citations in terminal output.

src/Capacitor.Cli/Commands/EvalCommand.cs

Refactor (1) +6 / -0
QuestionRunResult.csAdd internal question result union +6/-0

Add internal question result union

• Provides an in-process result shape containing exactly one assessment or failure.

src/Capacitor.Cli.Core/Eval/Contracts/QuestionRunResult.cs

Tests (14) +673 / -15
EvalCatalogClientTests.csUpdate catalog observer test double +3/-3

Update catalog observer test double

• Migrates the test observer to the V4 assessment and completion callback types.

test/Capacitor.Cli.Core.Tests.Unit/Eval/EvalCatalogClientTests.cs

EvalQuestionCatalogClientTests.csUpdate question catalog observer test double +3/-2

Update question catalog observer test double

• Adapts observer callbacks to outcome-aware assessments and V4 aggregates.

test/Capacitor.Cli.Core.Tests.Unit/Eval/EvalQuestionCatalogClientTests.cs

EvalServiceAggregateV4Tests.csTest V4 aggregation semantics +67/-0

Test V4 aggregation semantics

• Verifies assessed-only question means, null unscored aggregates, failure counts, coverage versions, and prompt-version stamping.

test/Capacitor.Cli.Core.Tests.Unit/Eval/EvalServiceAggregateV4Tests.cs

EvalServiceJsonSchemaTests.csTest outcome-aware verdict schema +42/-0

Test outcome-aware verdict schema

• Pins required outcome values, nullable bounded scores, nullable verdicts, and non-empty findings.

test/Capacitor.Cli.Core.Tests.Unit/Eval/EvalServiceJsonSchemaTests.cs

EvalServiceTests.csTest parsing and evidence reconciliation +198/-0

Test parsing and evidence reconciliation

• Covers absent, null, invalid, assessed, and unassessed outcomes. Also verifies text-path coverage derivation and insufficient-evidence reconciliation.

test/Capacitor.Cli.Core.Tests.Unit/Eval/EvalServiceTests.cs

ClaudeCliRunnerDetailedTests.csTest detailed Claude failure taxonomy +110/-0

Test detailed Claude failure taxonomy

• Uses a fake executable to verify timeout, process failure, unparseable output, recovered results, and legacy adapter behavior.

test/Capacitor.Cli.Core.Tests.Unit/Harness/Claude/ClaudeCliRunnerDetailedTests.cs

DaemonConnectProtocolVersionTests.csTest daemon protocol 2 advertisement +62/-0

Test daemon protocol 2 advertisement

• Verifies snake_case serialization, protocol 1 defaults, and round-trip compatibility with the shared golden fixture.

test/Capacitor.Cli.Tests.Integration/DaemonConnectProtocolVersionTests.cs

EvalCatalogFetchTests.csUpdate catalog integration observer +3/-2

Update catalog integration observer

• Migrates integration test callbacks to V4 assessment and aggregate contracts.

test/Capacitor.Cli.Tests.Integration/EvalCatalogFetchTests.cs

EvalQuestionsAliasRawTextTests.csUpdate alias integration observer +3/-2

Update alias integration observer

• Adapts the test observer to outcome-aware question completion and V4 finalization.

test/Capacitor.Cli.Tests.Integration/EvalQuestionsAliasRawTextTests.cs

EvalRunnerV2PostTests.csMaintain V2 persistence test compatibility +5/-4

Maintain V2 persistence test compatibility

• Updates observer types and documentation references after the V4 observer migration.

test/Capacitor.Cli.Tests.Integration/EvalRunnerV2PostTests.cs

EvalRunnerV3PostTests.csMaintain V3 persistence test compatibility +3/-2

Maintain V3 persistence test compatibility

• Updates the test observer to accept V4 completion notifications while retaining V3 persistence coverage.

test/Capacitor.Cli.Tests.Integration/EvalRunnerV3PostTests.cs

EvalRunnerV4PostTests.csVerify V4 evaluation persistence +95/-0

Verify V4 evaluation persistence

• Asserts exactly one V4 POST, no V3 POST, and serialized outcomes, null scores, coverage policy, and coded failures.

test/Capacitor.Cli.Tests.Integration/EvalRunnerV4PostTests.cs

EvalCommandRenderTests.csTest V4 terminal rendering +78/-0

Test V4 terminal rendering

• Pins outcome markers, coverage and citation details, and the unscored overall footer.

test/Capacitor.Cli.Tests.Unit/Commands/EvalCommandRenderTests.cs

daemon-connect.v2.jsonAdd daemon protocol 2 golden payload +1/-0

Add daemon protocol 2 golden payload

• Provides the shared serialized DaemonConnect fixture with eval_protocol_version set to 2.

test/fixtures/daemon-connect.v2.json

Other (1) +3 / -0
Capacitor.Cli.Tests.Integration.csprojInclude protocol 2 golden fixture +3/-0

Include protocol 2 golden fixture

• Copies the daemon connection fixture into integration test output.

test/Capacitor.Cli.Tests.Integration/Capacitor.Cli.Tests.Integration.csproj

@qodo-code-review

qodo-code-review Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. All failed runs lose results 🐞 Bug ☼ Reliability
Description
FinalizeAsync returns before calling Aggregate or PersistAggregateV4Async when
assessments.Count is zero, despite receiving coded entries in failures and supporting a
failure-only aggregate with a nullable score. When every judge times out, encounters a chat error,
or emits an invalid verdict, both the CLI and protocol-2 daemon discard the failure taxonomy without
posting a V4 payload, and the daemon maps the null return to a failed RPC result.
Code

src/Capacitor.Cli.Core/Eval/EvalService.cs[R750-754]

+        if (assessments.Count == 0) {
+            observer.OnFailed("all judge invocations failed");
+
+            return null;
+        }
Relevance

●●● Strong

Failure-only aggregation is explicitly designed and tested, so the early return defeats this PR’s
stated reliability goal.

PR-#200

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Question execution creates coded failures, and the new run path and protocol 2 carry both
assessments and failures into FinalizeAsync; the V4 aggregate already computes totals, supports
nullable scoring, and copies failures into FailedQuestions. The zero-assessment guard makes that
aggregation and the subsequent V4 persistence call unreachable in the all-failed case, while the
daemon forwards both collections and maps the resulting null return to a failed RPC result.

src/Capacitor.Cli.Core/Eval/EvalService.cs[314-325]
src/Capacitor.Cli.Core/Eval/EvalService.cs[739-783]
src/Capacitor.Cli.Core/Eval/EvalService.cs[1490-1513]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[211-231]
src/Capacitor.Cli.Core/Eval/EvalService.cs[572-600]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[198-223]
src/Capacitor.Cli.Core/Eval/Contracts/SessionEvalCompletedPayloadV4.cs[18-25]
src/Capacitor.Cli.Core/Eval/EvalService.cs[750-782]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
V4 finalization discards an eval when every question produces a coded failure because it rejects runs with no assessments. The V4 aggregate already represents failure-only runs with a null overall score and populated `FailedQuestions`, so these runs must be aggregated and posted rather than treated as unpersistable failures.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/EvalService.cs[739-783]
- src/Capacitor.Cli.Core/Eval/EvalService.cs[1450-1513]

## Recommended Fix
Remove or narrow the zero-assessment early return so finalization proceeds whenever `failures` is non-empty. Build and POST a V4 aggregate with no categories, a null overall score, zero assessed and judged questions, total questions derived from the failures, and every coded failure copied into `FailedQuestions`; skip the retrospective when no assessed question exists, and fail only when both assessments and failures are empty.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Older servers receive contradictory results ✓ Resolved 🐞 Bug ≡ Correctness
Description
HandleRunQuestionAsync uses a non-silent observer, so RunQuestionAsync relays
EvalQuestionCompleted for an unassessed assessment before the handler's new branch turns that same
result into a legacy failure. Against a protocol-1 server, an insufficient-evidence or
not-applicable question therefore sends a completed event with a null score and is then returned as
failed, breaking the retained non-null verdict-only event contract.
Code

src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[R340-341]

+        if (!silentPerQuestion)
+            Relay(() => connection.EvalQuestionCompletedAsync(evalRunId, sessionId, index, total, assessment.Category, assessment.QuestionId, assessment.Outcome ?? EvalOutcomes.Assessed, assessment.Score, assessment.Verdict), "EvalQuestionCompleted");
Relevance

●●● Strong

The PR explicitly preserves protocol-1 verdict-only semantics, making the nullable completion relay
contradictory.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The V1 handler constructs the normal relay observer, while RunQuestionAsync invokes its completion
callback for every successfully parsed assessment. The handler subsequently identifies unassessed
outcomes and returns a legacy failure; the changed relay now serializes the completion with nullable
score/verdict and an outcome field, even though the adjacent V1 comment states that this wire
protocol cannot represent outcomes.

src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[120-168]
src/Capacitor.Cli.Core/Eval/EvalService.cs[614-629]
src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[322-347]
src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[1624-1629]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The protocol-1 handler intends to represent unassessed outcomes as failures, but the normal observer sends a completion event before the handler can make that conversion. This gives a protocol-1 server a null-score completion and a failed RPC result for the same question.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[120-168]
- src/Capacitor.Cli.Daemon/Services/EvalRunner.cs[322-347]

## Recommended Fix
Ensure protocol-1 question execution does not relay `OnQuestionCompleted` until the handler has confirmed that the assessment is scored. For unassessed outcomes, emit only the legacy failure representation; preserve normal started/completed relays for assessed legacy verdicts.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Eval users get obsolete guidance ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
Render now displays unassessed outcomes, nullable scores, coverage details, and citations, while
README.md still promises only scored verdicts and says results are posted to the obsolete V3
endpoint. Anyone consulting the documented evaluation workflow can misinterpret the new output and
provision an incompatible server route when running kcap eval.
Code

src/Capacitor.Cli/Commands/EvalCommand.cs[R98-101]

+                var marker = q.Outcome switch {
+                    EvalOutcomes.InsufficientEvidence => "?",
+                    EvalOutcomes.NotApplicable        => "–",
+                    _ => q.Verdict switch { "pass" => "✓", "warn" => "!", _ => "✗" }
Relevance

●●● Strong

The team recently accepted README updates for changed eval prerequisites and persistence routes.

PR-#200

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2270057 requires README documentation to remain consistent with user-facing CLI
changes. The changed renderer introduces outcome-bearing and unscored output, and the new
persistence method posts V4 results, but the README continues to describe only pass/warn/fail scores
and /evals/v3.

Rule 2270057: Keep CLI documentation in README.md in sync with user-facing CLI changes
src/Capacitor.Cli/Commands/EvalCommand.cs[94-132]
src/Capacitor.Cli.Core/Eval/EvalService.cs[909-938]
README.md[488-506]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The user-visible `kcap eval` results and persistence endpoint changed, but the evaluation section of `README.md` still describes the previous scored-only output and V3 endpoint.

## Fix Focus Areas
- README.md[488-506]
- src/Capacitor.Cli/Commands/EvalCommand.cs[94-132]
- src/Capacitor.Cli.Core/Eval/EvalService.cs[909-938]

## Recommended Fix
Update the README evaluation section to explain assessed, insufficient-evidence, and not-applicable outcomes, nullable category and overall scores, coverage and citation output, and the V4 server endpoint and compatibility requirement.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. New parser bypasses JSON helpers ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
ParseVerdict compares doc.RootElement.ValueKind and outcomeElement.ValueKind directly instead
of using the available IsObject and IsNull helpers. Future changes to the shared JSON-kind
abstraction will not reach this explicit-null parsing path, leaving two competing shape-check
conventions to maintain.
Code

src/Capacitor.Cli.Core/Eval/EvalService.cs[R1096-1098]

+            outcomeExplicitlyNull = doc.RootElement.ValueKind == JsonValueKind.Object
+                && doc.RootElement.TryGetProperty("outcome", out var outcomeElement)
+                && outcomeElement.ValueKind == JsonValueKind.Null;
Relevance

●●● Strong

Recent precedent accepted replacing direct JSON kind checks with shared helpers in EvalService.

PR-#546

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2270023 prohibits new direct JsonElement.ValueKind comparisons when equivalent
helpers exist. The added parsing logic performs both an object and a null comparison directly, while
the shared extension class provides the corresponding helpers.

Rule 2270023: Use JsonElementExtensions helpers instead of direct JsonValueKind comparisons
src/Capacitor.Cli.Core/Eval/EvalService.cs[1095-1098]
src/Capacitor.Models.Transcripts/JsonElementExtensions.cs[5-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ParseVerdict` performs direct `JsonElement.ValueKind` comparisons even though shared `IsObject` and `IsNull` helpers exist for these checks.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/EvalService.cs[1095-1098]

## Recommended Fix
Import the shared JSON extension namespace if necessary, then replace the direct object and null `ValueKind` comparisons with `doc.RootElement.IsObject` and `outcomeElement.IsNull`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Prompt timeouts become chat errors ✓ Resolved 🐞 Bug ≡ Correctness
Description
RunCoreAsync classifies every non-external exception while writing stdin as ProcessFailure,
including the cancellation raised when its internal timeout expires. When a large prompt blocks
while being streamed, RunQuestionAsync consequently emits chat_error instead of judge_timeout.
Code

src/Capacitor.Cli.Core/Harness/Claude/ClaudeCliRunner.cs[R300-303]

                    /* ignore */
                }

-                return null;
+                return new(null, ClaudeCliFailure.ProcessFailure);
Relevance

●● Moderate

The timeout classification concern is technically specific, but no close historical conversion
precedent was found.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The stdin operations use a token linked to both caller cancellation and the internal timeout, but
only caller cancellation has a dedicated catch. The remaining cancellation reaches the generic
exception handler and maps through ProcessFailure to ChatError, whereas the later process-wait
path correctly returns Timeout.

src/Capacitor.Cli.Core/Harness/Claude/ClaudeCliRunner.cs[276-303]
src/Capacitor.Cli.Core/Harness/Claude/ClaudeCliRunner.cs[365-385]
src/Capacitor.Cli.Core/Eval/EvalService.cs[572-577]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Internal timeout cancellation during stdin streaming is caught as a generic process failure, causing the evaluation failure taxonomy to report a chat error instead of a judge timeout.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Harness/Claude/ClaudeCliRunner.cs[282-303]
- src/Capacitor.Cli.Core/Eval/EvalService.cs[572-577]

## Recommended Fix
Add a dedicated `OperationCanceledException` branch after the external-cancellation branch for stdin writes. Kill the process and return `ClaudeCliFailure.Timeout`, leaving other stdin exceptions classified as `ProcessFailure`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-deployments (sha: bd335e76)
  Explored: repo: kurrent-io/kcap-server (branch: tonyyoung/ai-2744-honest-coverage-impl, sha: b123d2b6)
Review mode: 🧠 Deep: This is a high-density behavioral change spanning eval parsing, aggregation, HTTP persistence, daemon RPC protocol compatibility, failure handling, and CLI rendering, with many independent logic paths where redundant review could catch subtle contract or integration defects.

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli.Core/Eval/EvalService.cs Outdated
Comment thread src/Capacitor.Cli/Commands/EvalCommand.cs
Comment thread src/Capacitor.Cli.Core/Harness/Claude/ClaudeCliRunner.cs
Comment on lines +750 to +754
if (assessments.Count == 0) {
observer.OnFailed("all judge invocations failed");

return null;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. All failed runs lose results 🐞 Bug ☼ Reliability

FinalizeAsync returns before calling Aggregate or PersistAggregateV4Async when
assessments.Count is zero, despite receiving coded entries in failures and supporting a
failure-only aggregate with a nullable score. When every judge times out, encounters a chat error,
or emits an invalid verdict, both the CLI and protocol-2 daemon discard the failure taxonomy without
posting a V4 payload, and the daemon maps the null return to a failed RPC result.
Agent Prompt
## Issue description
V4 finalization discards an eval when every question produces a coded failure because it rejects runs with no assessments. The V4 aggregate already represents failure-only runs with a null overall score and populated `FailedQuestions`, so these runs must be aggregated and posted rather than treated as unpersistable failures.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Eval/EvalService.cs[739-783]
- src/Capacitor.Cli.Core/Eval/EvalService.cs[1450-1513]

## Recommended Fix
Remove or narrow the zero-assessment early return so finalization proceeds whenever `failures` is non-empty. Build and POST a V4 aggregate with no categories, a null overall score, zero assessed and judged questions, total questions derived from the failures, and every coded failure copied into `FailedQuestions`; skip the retrospective when no assessed question exists, and fail only when both assessments and failures are empty.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/Capacitor.Cli.Daemon/Services/EvalRunner.cs Outdated
A per-question deadline that fires while still streaming the prompt
fell into the generic stdin-error catch and reported chat_error
instead of judge_timeout. Separately, an unassessed outcome on the
protocol-1 relay sent an older server both a null-score completion
and a failed RunQuestion result for the same question; the observer
now relays it as a single failure, matching what the RPC already
reports. Also uses the shared JsonElement helper for outcome-null
detection and refreshes the README/help text for outcomes, nullable
scores, and the v4/v3 posting split.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

Thanks Qodo — dispositions (9c7c490):

Fixed:

  • 🐞 Bug: an internal per-call deadline that fired while streaming stdin was classified as ProcessFailure; it now returns Timeout (→ judge_timeout). Added a runner test for the stdin-timeout path.
  • 🐞 Bug: on the protocol-1 relay path an unassessed outcome sent a null-score completion and was reported as a failure by the V1 RPC — the observer now relays an unassessed outcome as a single failure (assessed still completes), so an older server gets one consistent signal. New observer test covers it. (protocol-2 / silentPerQuestion path unchanged.)
  • Rule violations: ParseVerdict now uses the shared JsonElement helpers; the README eval section describes the outcomes, the not-scored states, coverage/citation output, and V4 posting.

Leaving as-is:

  • 🐞 "all failed runs lose results": a run where every question is a coded failure (timeout/chat-error/parse-failure) is a failed run with no result by design — unassessed outcomes (insufficient_evidence/not_applicable) are assessments and still aggregate + post a V4 with a null overall. The server's V4 validator requires at least one category with at least one question, so the empty-category failure-only payload suggested here would be rejected (400).

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