Skip to content

fix(sse): rotation attribution diagnostics (skipped accounts, serving id, request correlation) - #14223

Merged
diegosouzapw merged 10 commits into
cheaper-inference:release/v3.8.51from
maxmad64bis:fix/n109-n111-n112-observ-attrib
Sep 22, 2026
Merged

diegosouzapw merged 10 commits into
cheaper-inference:release/v3.8.51from
maxmad64bis:fix/n109-n111-n112-observ-attrib

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #13866

Summary

When rotation passes over cooling-down accounts, traffic concentrates with no trace of why, and proxy log rows can't be tied to the request or the serving account. This names skipped accounts, exposes per-account rotation state, and carries the serving account plus the request correlation on proxy log rows, all behind a new off-by-default flag with no selection change.

Related Issues

No linked issue — operator observability follow-up, no upstream issue covers skipped-account attribution.

Validation

  • Change type: provider
  • Focused tests and category gates from the golden path
  • npm run lint — ESLint 0 on the touched files; the full eslint . run is red on the base (🔴 Release branch not green: release/v3.8.51 #13866), unrelated to this diff.
  • Reconciled with the current active release base; focused checks rerun afterward — rev-list --count HEAD..upstream/release/v3.8.51 = 0, gates re-run after the last amend.
  • Production-code changes include a new or updated automated test in this PR — 6 new test files (17 tests, RED-then-GREEN).

Tests Added Or Updated

  • New: rotation-attribution-snapshot (4), rotation-attribution-flag (2), rotation-attribution-skipped-lines (2), proxy-logger-attribution (4), migration-182-183-proxy-logs-attribution (3), resilience-connections-rotation (2); updated: feature-flags-settings (counter 74→75). Neighbors green (resilience 22, 8 suites 27).

Coverage Notes

  • Three additive paths (skip log, rotation state, two journal columns) each covered by its own test file; migrations covered up-only with anti-cross-talk; no existing coverage removed.

Reviewer Notes

  • New off-by-default flag ROTATION_ATTRIBUTION (fail-safe false, no restart) with two additive nullable journal columns (migrations 182/183, no index) and a read-only loopback-gated route extension. Local gates: typecheck:core 0, ESLint 0 on owned files, C4 re-run at push time.
  • File-size rebaseline at this head (303fcb2103): _rebaseline_2026_09_20_14223_rotation_attribution_growth pins frozen at gate counts (chat.ts 2561, chatHelpers.ts 1257, proxyFetch.ts 1294, opencode.ts 1276); gate green locally. Counters use the gate measure (split("\n").length, one more than wc -l).
  • CI reds are inherited from the red base (🔴 Release branch not green: release/v3.8.51 #13866): ESLint ✖ lines cite files outside this diff (git diff $mb empty on each), same 9 jobs on third-party fix(sse): treat antigravity empty completions with a normal stop as valid 200s (#14160) #14243. Non-blocking.

@maxmad64bis
maxmad64bis force-pushed the fix/n109-n111-n112-observ-attrib branch from 802e369 to 2c90662 Compare September 19, 2026 23:29
@maxmad64bis
maxmad64bis marked this pull request as ready for review September 19, 2026 23:36
… id, request correlation)

When rotation passes over cooling-down accounts, the operator sees traffic
concentrate with no trace of why, and proxy log rows cannot be tied to the
request or the serving account (all anonymous traffic reads noauth).
Name skipped accounts (masked id, one info line each), expose per-account
state on the resilience connections endpoint, and carry the masked serving
account plus the request correlation id on proxy log rows behind a new
off-by-default flag; selection unchanged.
Verified: 17 new tests RED-then-GREEN, neighbors green, typecheck core 0.
@maxmad64bis
maxmad64bis force-pushed the fix/n109-n111-n112-observ-attrib branch from 2c90662 to 303fcb2 Compare September 20, 2026 10:39
Conflicts:
- config/quality/file-size-baseline.json: kept both sides' _rebaseline notes;
  collapsed the duplicated frozen keys and re-measured opencode.ts on the
  reconciled tree (1339).
- open-sse/executors/opencode.ts: took the release shape (egressPacing /
  park-and-resume / try-finally slot release) and re-applied the attribution
  delta on top of it.
- tests/unit/feature-flags-settings.test.ts: 76 definitions on the reconciled
  tree (tip's new flag + ROTATION_ATTRIBUTION).
The branch forked when 182 was still free; release/v3.8.51 has shipped
182_request_cost_ledger_and_key_quota.sql since. Two files under the same
numeric prefix make getMigrationFiles() throw 'Migration version collision
detected' on the first database touch, so a fresh install cannot boot.

- 182_proxy_logs_rotation_account.sql -> 183_proxy_logs_rotation_account.sql
- 183_proxy_logs_correlation_id.sql   -> 184_proxy_logs_correlation_id.sql
- isSchemaAlreadyApplied: case "182"/"183" -> "183"/"184" (the arms are keyed
  by version only, so on the tip they would have answered for a different
  migration and skipped it)
- tests/unit/migration-182-183-proxy-logs-attribution.test.ts renamed to
  migration-183-184-proxy-logs-attribution.test.ts, with the copied SQL
  filenames, the ledger expectations and the case names moved to 183/184
…-attrib

Conflicts:
- open-sse/executors/opencode.ts: cheaper-inference#14464 replaced the free-tier refusal exit
  with handleLoopFreeTierRefusal(); kept the release shape and moved the
  attribution flush to just before that call.
- config/quality/file-size-baseline.json: kept both sides' _rebaseline notes;
  re-measured on the reconciled tree — opencode.ts 1386, proxyFetch.ts 1287
  (the tip shrank it to 1268 in the cheaper-inference#14218 Prettier pass).
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Maintainer note — pushed a reconciliation commit on top of your work (your commits are untouched).

Migrations renumbered 182/183 → 183/184. When this branch forked, 182 was still free; release/v3.8.51 has since shipped 182_request_cost_ledger_and_key_quota.sql (#13610). Two files under the same numeric prefix make getMigrationFiles() throw Migration version collision detected on the first database touch, so a fresh install would not boot. Renamed:

  • src/lib/db/migrations/182_proxy_logs_rotation_account.sql → 183_proxy_logs_rotation_account.sql
  • src/lib/db/migrations/183_proxy_logs_correlation_id.sql → 184_proxy_logs_correlation_id.sql

Everything keyed to those numbers moved with them:

  • src/lib/db/migrationRunner.ts: the isSchemaAlreadyApplied arms case "182" / case "183" → case "183" / case "184". These are keyed by version only, so left at 182 the rotation_account check would have answered for 182_request_cost_ledger_and_key_quota and skipped it.
  • tests/unit/migration-182-183-proxy-logs-attribution.test.ts → tests/unit/migration-183-184-proxy-logs-attribution.test.ts, with the copied SQL filenames, the _omniroute_migrations ledger expectations and the case names updated.
  • The _rebaseline_2026_09_20_14223_... note in config/quality/file-size-baseline.json re-measured on the reconciled tree.

The existing tests/unit/migration-135-numbering-collision.test.ts is the end-to-end proof: its first case, "a fresh install applies all real on-disk migrations without a version collision", runs every real on-disk migration against a fresh temp DATA_DIR. It passes on this branch, and re-adding a duplicate 182_proxy_logs_rotation_account.sql makes it fail with Migration version collision detected: version=182 → [proxy_logs_rotation_account, request_cost_ledger_and_key_quota].

Also reconciled against the current tip while merging release/v3.8.51 in:

  • open-sse/executors/opencode.ts: fix(opencode): complete refused tool subsets with previously accepted names #14464 replaced the free-tier-refusal exit with handleLoopFreeTierRefusal(). Kept the release shape and moved the skipped-account flush to just before that call.
  • tests/unit/feature-flags-settings.test.ts: the expected count is now 76 (the registry gained one flag on the tip after the fork, plus ROTATION_ATTRIBUTION); counted from FEATURE_FLAG_DEFINITIONS, not guessed. ROTATION_ATTRIBUTION stays defaultValue: "false".
  • config/quality/file-size-baseline.json: both sides' notes kept; ceilings re-measured on the reconciled tree — opencode.ts 1386, proxyFetch.ts 1287.

Focused tests green: 82/82 across the six attribution/migration/resilience/feature-flag files plus the collision guard. Two failures reproduce unchanged on the pure tip (30a83ee) and are not from this PR: check:open-sse-typecheck (open-sse/executors/auggie.ts TS2769/TS18047) and tests/unit/opencode-executor.test.ts → "omits accept header when stream is false".

… exits too

The two `runParkAndReplay` returns in the 429 arm were the only rotation
exits that left the request without its cooldown-skip diagnostics.
…ormatted + sync flag counts

The two earlier commits on this branch were made with --no-verify, so
lint-staged never ran: chat.ts, types/resilience.ts (two declarations on
one line) and the rotation test were left off-format. Also brings
docs/reference/FEATURE_FLAGS.md's catalog counts back in line with the
registry (76 flags, Network 18) after ROTATION_ATTRIBUTION was added.
… on the reconciled tip

opencode.ts 1386->1324 and chat.ts 2561->2559 were measured before cheaper-inference#14149
moved the MuseSpark block out of the executor and before Prettier reflowed
chat.ts; pin both to the gate's own count on this tree.
# Conflicts:
#	open-sse/executors/opencode.ts
#	tests/unit/feature-flags-settings.test.ts
…ter the cheaper-inference#14353/cheaper-inference#14213 merge

cheaper-inference#14353 moved the opencode member list off the shared instance, so the
rotation-attribution snapshot/skip enumeration now walks the per-request
list; opencode.ts settles at 1318. The registry gained
FLUSH_EMPTY_RETRY_ENABLED from cheaper-inference#14213, taking the catalog to 77 flags
(Network 19).
@diegosouzapw
diegosouzapw merged commit 6668859 into cheaper-inference:release/v3.8.51 Sep 22, 2026
7 of 16 checks passed
diegosouzapw pushed a commit that referenced this pull request Sep 22, 2026
Merged — closes #14068. Thank you, @RaviTharuma.

Collapsing "this alias is not in the live catalog" into the generic `availability` bucket meant an unknown model got reported to the operator as "no credentials available" — a diagnosis pointing at the wrong subsystem entirely. Giving it its own `model_not_in_catalog` reason, allowlisted and grouped in `diagnostics.skippedTargets` on `ALL_TARGETS_SKIPPED`, is the difference between a five-minute fix and an afternoon of checking keys. Deleting the earlier `src/lib/model-not-found.ts` scaffolding and its unused helper test in the same PR, rather than leaving it orphaned, is the right call too.

Validation before merge: boarded onto the current release tip together with the other seven PRs of your batch — merged clean, no conflict against the tip or any sibling. `combo-skipped-targets-summary` + `combo-decision-trace` 16 pass / 0 fail, `typecheck:core` clean, `check:changelog-integrity` OK.

One thing I added on your branch: this PR was the only one in the batch to trip `check:file-size`, because both files it threads the new reason through were already frozen **exactly at their measured size with zero headroom** — `src/sse/handlers/chat.ts` had been tightened to 2559 by #14223 two days ago, and `roundRobinCombo.ts` sat at 1261. Your own growth is +1 and +2 respectively (the `errorType === "model_not_found"` early return, and the strict `available !== true` pre-check plus the sticky-target expression that Prettier reflows over three lines at printWidth 100). That is irreducible call-site plumbing, not a quality regression, so it earned a documented rebaseline rather than a bounce: annotation key `_rebaseline_2026_09_22_14069_model_not_in_catalog`, with both ceilings set to the values measured on the reconciled tree *after* the commit. Structural shrink of both god-files stays tracked in #3501.
diegosouzapw added a commit that referenced this pull request Sep 24, 2026
…s lossy policy; drain 12 base-red files

#14529 moved lossy engines off the header-less path in the runtime but not in
deriveEffectivePreviewPlan(), so the dashboard preview showed rtk -> caveman
while requests ran session-dedup -> lite (#12063 again). The preview now
applies downgradeUnrequestedLossy(); the #12063 test pins preview == runtime
for the profile, engines-map and safe-profile cases (fails on the old code).

Test guards realigned to merged design changes, each traced to its commit:
- #14529: compression suites opt in with allow-lossy where they test the
  operator plan, and assert the header-less downgrade; default seed is
  session-dedup + lite; the #10503 probe ignores the loopback dashboard
  telemetry POST that stacked compression emits (the context gate still
  rejects locally — verified with logging).
- #14530: breaker profile asserted on the connection breaker; the
  provider-wide cooldown case drives the network-error path.
- #14370: @opencode/plugin (stable OpenCode 2.x contract, npm maintainer
  thdxr) added to the dependency allowlist.
- #14213: the opt-in empty-turn retry credential site is inventoried as
  class A (dispatch goes through assertManagedLeaseFence).
- #14223 / #14329: flag count 77, quota registry 13 (muse-code).

Refs #14496
@maxmad64bis
maxmad64bis deleted the fix/n109-n111-n112-observ-attrib branch September 24, 2026 21:13
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.

2 participants