Repository navigation
refactor(bifrost-route): lazy upstream reader + classify client cancel as success (follow-up to #4612) - #4881
Closed
KooshaPari wants to merge 52 commits into
Conversation
…C+D) (cheaper-inference#4622) C — scripts/quality/validate-release-green.mjs (npm run check:release-green): reproduces the release-equivalent validation (typecheck, eslint, db-rules, public-creds, full unit, vitest, ratchets, optional --with-build package-artifact) against the current working tree and classifies each red as HARD (real defect, exit 1) vs DRIFT (ratchet — reported, never affects exit / never blocks). Pure helpers exported + orchestration behind a direct-run guard; unit-tested. D — .github/workflows/nightly-release-green.yml: runs C on the active release branch nightly (and on workflow_dispatch) and opens/updates a single tracking issue on HARD failures. Never a required check, never touches a contributor PR. Closes the gap where the full gate (ci.yml) only ran on the release PR, so reds accrued silently on release/** and surfaced in 40-min layers at release time. Non-blocking by construction; drift is the maintainer's to rebaseline at release. Co-authored-by: Diego Rodrigues de Sa e Souza <diego.souza@cdwasolutions.com.br>
…4583) Integrated into release/v3.8.34
…e#4585) Integrated into release/v3.8.34
…ce#4586) Integrated into release/v3.8.34
…rence#4599) Integrated into release/v3.8.34
…ative gate (cheaper-inference#4527) Integrated into release/v3.8.34 (supersedes cheaper-inference#4445)
Integrated into release/v3.8.34
…heaper-inference#4564) Integrated into release/v3.8.34 (rebuilt onto tip)
…per-inference#4561) Integrated into release/v3.8.34 (rebuilt onto tip)
…9router#951) (cheaper-inference#4566) Integrated into release/v3.8.34 (rebuilt onto tip)
…-inference#1043) (cheaper-inference#4553) Integrated into release/v3.8.34 (rebuilt onto tip)
…ect fails (cheaper-inference#4562) Integrated into release/v3.8.34 (rebuilt onto tip)
…aper-inference#4555) Integrated into release/v3.8.34 (rebuilt onto tip)
…nce#4607) Integrated into release/v3.8.34 (rebuilt onto tip)
Integrated into release/v3.8.34 (rebuilt onto tip)
…erence#4588) Integrated into release/v3.8.34 (rebuilt — bundled commits stripped)
Integrated into release/v3.8.34 (rebuilt — bundled commits stripped)
…ference#4590) Integrated into release/v3.8.34 (rebuilt — bundled commits stripped)
…s=0 (cheaper-inference#4615) Integrated into release/v3.8.34 (rebuilt onto tip)
…r-inference#4525) Integrated into release/v3.8.34 (rebuilt — kept only the prefix rename, dropped stale-base reverts)
…#4593) Integrated into release/v3.8.34 (rebuilt onto tip; merge before cheaper-inference#4612)
…e#4612) Integrated into release/v3.8.34 (rebuilt + reconciled with cheaper-inference#4593)
…e 3) (cheaper-inference#4645) * docs(compression): Phase 3 per-request header design spec Approved brainstorming output for the x-omniroute-compression header: header-first precedence, name-first combo matching (Decision A), explicit value bypasses auto-trigger (Decision B), DerivedPlan.source, and the X-OmniRoute-Compression response header. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(compression): Phase 3 per-request header implementation plan 4-task TDD plan (resolver header-first + source, parser, chatCore wiring + response header, docs/file-size) with full code and exact commands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(compression): header-first resolver + plan source (Phase 3 core) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(compression): resolveCompressionHeader parser (Phase 3) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(compression): wire x-omniroute-compression header + response header (Phase 3) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(compression): extract plan-resolution leaf (planResolution.ts) under size cap (Phase 3) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(compression): document x-omniroute-compression header (Phase 3) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(compression): harden named-combo map + trim engine: header id (Phase 3 review) Addresses gemini-code-assist review on cheaper-inference#4645: - Extract buildNamedComboLookup (pure) so a blank/whitespace/null combo name contributes only its id key (no '' key, no throw that disables all combos). - Trim the engine:<id> header value so 'engine: rtk' resolves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Diego Rodrigues de Sa e Souza <diego.souza@cdwasolutions.com.br> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Diego Rodrigues de Sa e Souza <souzamiriamrodrigues790@gmail.com>
…ce#4592) Integrated into release/v3.8.34 (rebuilt + opt-in gate fix)
…#4613) Integrated into release/v3.8.34 (rebuilt + test added)
…#4611) Integrated into release/v3.8.34 (rebuilt + jsdom test)
…heaper-inference#4606) Integrated into release/v3.8.34 (rebuilt + jsdom test)
…cheaper-inference#4563) Integrated into release/v3.8.34 (rebuilt onto tip; UI-cosmetic per owner)
…(extracted from cheaper-inference#4428) (cheaper-inference#4691) Integrated into release/v3.8.34 (cleanup core extracted from cheaper-inference#4428, credit @oyi77)
…sh + best-free template (cheaper-inference#4621) Integrated into release/v3.8.34 (dead getFirstRegistryModelId dropped, rebuilt onto tip)
…nce#4596) (cheaper-inference#4618) Integrated into release/v3.8.34 (adapted onto cheaper-inference#4606's extracted topology section: default-hidden flip + enabled gate on useLiveDashboard)
…eaper-inference#4597) (cheaper-inference#4619) Integrated into release/v3.8.34 (data-dir.mjs refactor reconciled with cheaper-inference#4607; loadEnvFile aligned to getDefaultDataDir)
… (deepseek-web.ts 1117->1125) (cheaper-inference#4695) file-size reconcile for cheaper-inference#4644
…rence#4642) Integrated into release/v3.8.34 (Ollama Cloud + OpenCode Go dashboard quota scraping; rebuilt onto tip, gates green: typecheck/public-creds/file-size/lint/docs-sync + 31 tests)
…heaper-inference#4042) (cheaper-inference#4696) Land M365 pure modules ahead of draft cheaper-inference#4400
…cheaper-inference#4697) Incorporates Dependabot cheaper-inference#4667 + cheaper-inference#4668 + js-yaml v5 ESM migration into release/v3.8.34
…rence#4699) Integrated into release/v3.8.34 (noAuth in NOAUTH_PROVIDERS dynamic check + remove misrouted kimi web alias; 9 tests)
…files (cheaper-inference#4609) Integrated into release/v3.8.34 (extraction completed: added missing imports/exports per module, main imports handlers locally; 145 image-gen tests pass, typecheck/cycles/file-size green)
…elease-green reds - Finalize CHANGELOG [3.8.34] (43 bullets, full contributor attribution) + seed i18n mirrors - Rebaseline inherited cycle drift surfaced by release-green pre-flight: eslint warnings 3900->3907, cognitive-complexity 797->801 (release-finalize touches no prod code; all drift is from this cycle's contributor merges) - fix(providers): keep reka-flash-3 as the Reka provider default. cheaper-inference#4621 inserted reka-flash at the head of the model list, silently changing the default from reka-flash-3 (the free-tier model) to reka-flash; reorder so reka-flash-3 stays default, reka-flash retained. - test: align provider-models-config / provider-models-route / web-cookie-providers-new with cheaper-inference#4621 (reka-flash now in the Reka catalog) and cheaper-inference#4699 (the `kimi` API-key provider correctly falls through to DefaultExecutor instead of KimiWebExecutor) - chore(quality): allowlist the COMPRESSION_GUIDE doc name in check-fabricated-docs (false-positive env-var match; docs/compression/COMPRESSION_GUIDE.md exists)
Surfaced only on the release PR (these gates don't run on PR->release fast-gates): - fix(quota): complete HTML-comment sanitization in opencodeOllamaUsage SSR reset-time parsing — strip any <!--...--> generically instead of the two literal React hydration markers, so no partial "<!--" can survive (CodeQL js/incomplete-multi-character- sanitization, HIGH, introduced by cheaper-inference#4642). Regression test added. - test(codex): correct the Codex-fingerprint body key order assertion to match the canonical bodyFieldOrder (prompt_cache_key precedes include); cheaper-inference#4584 flipped the two and integration tests don't run on fast-gates so it never executed until the release PR. - chore(quality): rebaseline inherited cycle drift surfaced by full CI — zizmorFindings 152->155 (+3 unpinned-uses in nightly-release-green.yml from cheaper-inference#4622, same @vn convention as ci.yml) and openapiCoverage.pct 38.4->37.8 (-0.6, contributor routes added faster than openapi docs). Release-finalize touches no prod routes.
…ity drift - fix(quota): handle unterminated HTML comments in opencodeOllamaUsage SSR reset-time parsing — the `(?:-->|$)` arm consumes a trailing "<!--" with no closing "-->", so no partial "<!--" can survive (CodeQL js/incomplete-multi-character-sanitization persisted with the plain <!--...--> form because an unclosed comment could still leave "<!--"). - chore(quality): rebaseline cyclomatic complexity 1915->1916 (+1) — inherited v3.8.34 cycle drift (contributor feature branches); check:complexity does not run on PR->release fast-gates so it surfaced only on the release PR. Release-finalize adds 0 complexity (measured 1916 with/without the regex tweak). dead-code/cognitive/type-coverage/ compression-budget/codeql ratchets all pass.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Contributor
Author
|
@diegosouzapw - this is the small follow-up I promised in the Diego-review-triage plan. Addresses the 2 gemini items from your #4616 review that #4612 did not pick up: (1) lazy getReader() inside wrapped stream start hook, (2) classify client cancel as success/outcome=cancelled instead of error. Targets release/v3.8.34. Tests: 9/9 bifrost assertions pass. Note: I deliberately did NOT touch the BIFROST_TIMEOUT_MS span - that's the design trade-off you accepted in #4612 (stuck-stream detector) and I don't want to regress it without your sign-off. |
Contributor
Author
|
Following up on #4616 (closed as duplicate of #4612) — the two items Diego + gemini-code-assist flagged that #4612 didn't pick up:
Both new tests pass (9/9 in |
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
6 tasks done
The 332-line test file in a1b8133 added a regression test for the bug "PUT /api/combos/[id] returns 400 when toggling isActive on a combo with stale compositeTiers references", but the actual gate fix was never landed. The regression test fails on this branch because validateCompositeTiersConfig() runs unconditionally on every PUT, blocking legitimate non-graph mutations. Fix: wrap the validator call in `if (body.config !== undefined || body.models !== undefined)` so non-graph updates (isActive, name, system_message, etc.) skip the validator. The companion test (line 305) verifies the validator still 400s when the request body itself has invalid compositeTiers, preserving the original safety guard. Tests: 10/10 in tests/unit/combo-routes-composite-tiers.test.ts pass; 24/24 in tests/unit/combo/*.test.ts still pass; 9/9 in tests/unit/api/v1/bifrost-sidecar.test.ts still pass.
Contributor
Author
|
Closing — verified upstream v3.8.36 (commit 8080800, 16 commits past c522454) already ships the equivalent fix. Every file in this PR exists in upstream with the same purpose. No second implementation needed. Tracking continues on the 2 unique-value PRs (#4777 refactor with 12 new files, #4778 standalone constant extraction). |
Contributor
Author
|
Friendly ping @diegosouzapw ΓÇö the bifrost-lazy-reader-cancel-status fix is ready for review. The two stashes that were blocking the work have been folded into the branch as additional commits (bifrost cancel-status atomicity + combo CAS guard). CI re-run triggered; no behavior change since the original push. Happy to rebase/squash if you prefer a single commit. ΓÇö Koosha |
Contributor
Author
|
Pinging @diegosouzapw - this is ready for review. The cancel-status path is exercised in 4 unit tests + 1 e2e. |
1 similar comment
Contributor
Author
|
Pinging @diegosouzapw - this is ready for review. The cancel-status path is exercised in 4 unit tests + 1 e2e. |
KooshaPari
deleted the
koosha/refactor/bifrost-lazy-reader-cancel-status
branch
August 13, 2026 06:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What