fix(telemetry): a turn that never sent is not a Discord verdict

coilyco-ops commented

2026-08-15 15:40:13 +00:00

Member

Closes #802. Slices #292's first acceptance criterion, which the field it added could not meet while the classifier ran on the wrong error.

What changed

The turn event classifies a Discord verdict only where one exists. deliverOrReport marks the reply send error as it returns, and discord.turn.failed classifies through that marker instead of through the raw turn error. Everything else reports discord_failure: not_attempted.

Nothing else moved. discordFailureAttrs is untouched, so rest_error, abandoned, and no_response keep their meanings and the existing tests pinning them keep passing. discord.reply.failed and turn.reply.undelivered still classify the send and the notice at their own call sites, where the error is unwrapped and the verdict was always correct.

Why the value is a state rather than an absence

not_attempted is a distinct value because an absent discord_failure already means something else here: an image older than the field. docs/sirens-echo-indistinguishable-values.md records that exact conflation on this series, so answering it with another absence would have reproduced it.

Mutation

Classifying the turn error directly again fails four rows of TestATurnThatNeverSentIsNotADiscordVerdict, and reproduces the production reading exactly: a model stage failure reports no_response, a turn that ran out of budget reports abandoned.

Dropping the marker at deliverOrReport fails TestASendThatFailedKeepsItsVerdictAtTheTurn, which drives a rejecting turn through the seam rather than asserting about the wrapper.

What this does not do

It does not attribute the 31 replies #292 was filed on. Those predate every instrument on the series and cannot be split retroactively. 292 stays open on that criterion, which needs a decision rather than code.

It does not reduce any failure rate. It stops one series describing stage failures as delivery failures, so the delivery rate can be read at all.

ward exec gate passes on the rebased tree.

Closes #802. Slices https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/292's first acceptance criterion, which the field it added could not meet while the classifier ran on the wrong error. ## What changed **The turn event classifies a Discord verdict only where one exists.** `deliverOrReport` marks the reply send error as it returns, and `discord.turn.failed` classifies through that marker instead of through the raw turn error. Everything else reports `discord_failure: not_attempted`. **Nothing else moved.** `discordFailureAttrs` is untouched, so `rest_error`, `abandoned`, and `no_response` keep their meanings and the existing tests pinning them keep passing. `discord.reply.failed` and `turn.reply.undelivered` still classify the send and the notice at their own call sites, where the error is unwrapped and the verdict was always correct. ## Why the value is a state rather than an absence `not_attempted` is a distinct value because an absent `discord_failure` already means something else here: an image older than the field. `docs/sirens-echo-indistinguishable-values.md` records that exact conflation on this series, so answering it with another absence would have reproduced it. ## Mutation Classifying the turn error directly again fails four rows of `TestATurnThatNeverSentIsNotADiscordVerdict`, and reproduces the production reading exactly: a model stage failure reports `no_response`, a turn that ran out of budget reports `abandoned`. Dropping the marker at `deliverOrReport` fails `TestASendThatFailedKeepsItsVerdictAtTheTurn`, which drives a rejecting turn through the seam rather than asserting about the wrapper. ## What this does not do **It does not attribute the 31 replies https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/292 was filed on.** Those predate every instrument on the series and cannot be split retroactively. 292 stays open on that criterion, which needs a decision rather than code. **It does not reduce any failure rate.** It stops one series describing stage failures as delivery failures, so the delivery rate can be read at all. `ward exec gate` passes on the rebased tree.

coilyco-ops added 1 commit

2026-08-15 15:40:13 +00:00

fix(telemetry): a turn that never sent is not a Discord verdict

ci / image-build (pull_request) Successful in 24s

Details

ci / test (pull_request) Failing after 30s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

feb86b8514

`discord.turn.failed` fires when the turn returned any error, and it
classified that error the way a failed send is classified. A model stage
failure is neither a context error nor a rejection, so it fell through to the
catch-all and reported `no_response`, the value that means the gateway never
answered.

Thirteen of fourteen classified rows in one 24 hour window were turns that
failed at a stage and never reached a send. That is why every sample on the
series read `no_response` and not one ever read `rest_error`.

The reply send is now marked as it returns, and the turn event classifies a
Discord verdict only where one exists. Everything else reports
`discord_failure: not_attempted`, so the two populations split in a group-by
instead of merging into the catch-all.

`discord.reply.failed` is unchanged and remains the event that fires when a
send actually failed.

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-15 15:40:13 +00:00

coilyco-ops referenced this pull request

2026-08-15 15:41:02 +00:00

Echo: 31 replies in 24h were ready and never reached Discord, with no stage failure logged #292

coilyco-ops added 1 commit

2026-08-15 16:05:20 +00:00

Merge remote-tracking branch 'origin/main' into pr803

ci / image-build (pull_request) Successful in 26s

Details

ci / test (pull_request) Successful in 41s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

f2bc65e00e

coilyco-ops referenced this pull request

2026-08-15 16:07:02 +00:00

TestRefreshIsIdempotentUntilTheNextTurn failed in CI on an unreachable fixture server, and Open discards the reason so nothing says why #809

coilyco-ops commented

2026-08-15 16:07:43 +00:00

Author

Member

CI is green. Run 20623: test success, image-build success.

What was red, and why it was not this change

Run 20603 failed one test:

--- FAIL: TestRefreshIsIdempotentUntilTheNextTurn (0.00s)
    mcp_refresh_test.go:129: Open: no configured MCP server is reachable

That is the MCP roster path. This branch touches agent.go (deliverOrReport), replyfailure.go, replyfailure_test.go, and two docs, and nothing it changes is reachable from MCPProvider.Open. I read the diff before concluding that rather than inferring it from the file list.

What I did

Merged current main into the branch rather than rebasing, so another session's branch is not rewritten and no commit is discarded. The merge picks up #805's pyproject.toml hook excludes. ward gate green on the merged tree - build, policy-check, vet, test, test-skips, pre-commit - and CI green on the push.

I did not just rerun until it passed

The single test at -count=50, the package six times, the package eight times at GOMAXPROCS=2 -parallel 2, and the package twelve times at GOMAXPROCS=2 -parallel 16 -race. All green, no race warnings. So it is a real intermittent that I could not reproduce, not a break this branch introduced and not something the merge fixed.

The reason I could not chase it further is worth its own ticket, and is #809: Open collects each unreachable server by name only and discards the error, so the failure cannot say whether it was connection-refused, a backoff, or a failed listing. readyLocked produces a specific error on every branch and none survives. Same shape as #795. That issue asks for the reason to be carried first, so the next occurrence is self-diagnosing instead of costing another investigation.

Merge stays director-gated per the repository's pull-request-and-merge workflow.

CI is green. Run 20623: `test` success, `image-build` success. ## What was red, and why it was not this change Run 20603 failed one test: ``` --- FAIL: TestRefreshIsIdempotentUntilTheNextTurn (0.00s) mcp_refresh_test.go:129: Open: no configured MCP server is reachable ``` That is the MCP roster path. This branch touches `agent.go` (`deliverOrReport`), `replyfailure.go`, `replyfailure_test.go`, and two docs, and nothing it changes is reachable from `MCPProvider.Open`. I read the diff before concluding that rather than inferring it from the file list. ## What I did Merged current `main` into the branch rather than rebasing, so another session's branch is not rewritten and no commit is discarded. The merge picks up #805's `pyproject.toml` hook excludes. `ward gate` green on the merged tree - build, policy-check, vet, test, test-skips, pre-commit - and CI green on the push. ## I did not just rerun until it passed The single test at `-count=50`, the package six times, the package eight times at `GOMAXPROCS=2 -parallel 2`, and the package twelve times at `GOMAXPROCS=2 -parallel 16 -race`. All green, no race warnings. So it is a real intermittent that I could not reproduce, not a break this branch introduced and not something the merge fixed. The reason I could not chase it further is worth its own ticket, and is #809: `Open` collects each unreachable server **by name only** and discards the error, so the failure cannot say whether it was connection-refused, a backoff, or a failed listing. `readyLocked` produces a specific error on every branch and none survives. Same shape as #795. That issue asks for the reason to be carried first, so the next occurrence is self-diagnosing instead of costing another investigation. Merge stays director-gated per the repository's `pull-request-and-merge` workflow.

coilysiren merged commit cfd3ba2ee9 into main

2026-08-15 16:19:37 +00:00

coilysiren referenced this pull request from a commit

2026-08-15 16:19:37 +00:00

Merge pull request 'fix(telemetry): a turn that never sent is not a Discord verdict' (#803) from aos/claude/aw85-turn-failure-classify into main

coilyco-ops referenced this pull request from a commit

2026-08-15 16:33:23 +00:00

fix(ci): unbreak main, red from two semantic merge collisions

coilyco-ops referenced this pull request

2026-08-15 16:33:41 +00:00

Unbreak main, red from two semantic merge collisions #813

coilyco-ops referenced this pull request