Skip to content

refactor(combo): move handleRoundRobinCombo into roundRobinCombo.ts - #12811

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
HouMinXi:feat/combo-round-robin-extract
Sep 7, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.51from
HouMinXi:feat/combo-round-robin-extract

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Move handleRoundRobinCombo (and resolveTargetTokenLimit) from open-sse/services/combo.ts into open-sse/services/combo/roundRobinCombo.ts.

This branch is stacked on #12746 (feat/combo-execute-target-split @ 5a5ee9be5), which is still open. GitHub will list Inner's 7 commits plus the 3 below. After #12746 lands, rebase this onto release/v3.8.51 and the Inner SHAs drop.

This PR's own commits (on top of #12746):

  1. 4d168211d — source guards (RED on ENOENT before the lift)
  2. 13219a86b — the lift
  3. changelog fragment

If you want a PR that contains only those three, wait for #12746 to merge and I will rebase. Opening now because the lift is done and reviewed against combo.ts:189/202/759.

What this PR owns (vs #12746)

After #12746 This PR
combo.ts 2164 1014 (split("\n").length)
roundRobinCombo.ts — 1199 (cap 1200)
handleRoundRobinCombo still in combo.ts leaf module

Files vs #12746 (not vs tip):

  • open-sse/services/combo.ts (−1178 net)
  • open-sse/services/combo/roundRobinCombo.ts (new)
  • tests/unit/combo/round-robin-combo.test.ts (new)
  • tests/unit/combo/combo-attempt-loop.test.ts (rrStart === -1 so Inner's unused-delay assert cannot false-green after the name leaves combo.ts)
  • changelog.d/maintenance/combo-round-robin-extract.md

Cycle

The leaf still static-imports releaseStickyPinOnFailure and clearStaleLKGP from ../combo.ts (combo.ts:189 / :202). Those helpers stay on the barrel because Inner / executeTargetAttempt take them via deps.

The barrel does not static-import the leaf. The round-robin fork is:

const { handleRoundRobinCombo } = await import("./combo/roundRobinCombo.ts");

at combo.ts:759. Node caches the module after the first load.

What did not change

Round-robin skip / sticky / semaphore / rrLoopSafetyTimer finally are copied as-is. This does not run Inner's 10 executeTarget gates on RR (breaker OPEN still does not skip an RR rotate; it only clears sticky). A later PR can add a shared targetPolicy if we want that table filled per view.

Tests

DISABLE_SQLITE_AUTO_BACKUP=true node --max-old-space-size=8192 \
  --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts \
  --import ./tests/_setup/isolateDataDir.ts --test --test-force-exit \
  tests/unit/combo/round-robin-combo.test.ts \
  tests/unit/combo/combo-attempt-loop.test.ts \
  tests/unit/combo-loop-safety-timer-leak-11804.test.ts

12/12 pass. Injection: deleting releaseStickyPinOnFailure( in the leaf turns the guard red; restore is green.

node scripts/check/check-file-size.mjs --base-ref upstream/release/v3.8.51 OK. npx eslint --no-cache on the four unique files: 0 errors.

Not in this PR

Base-red inherited

⚠️ base-red inherited: #12732 (release/v3.8.51 currently red). Failures that match that issue are not from this lift. Unique diff does not touch glm.ts, reset-aware-model-family.md, or agent-skills cli-tunnel.

@HouMinXi
HouMinXi force-pushed the feat/combo-round-robin-extract branch from 3135e5c to efee494 Compare September 6, 2026 08:12
@HouMinXi

HouMinXi commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto release/v3.8.51 tip f9a1cc8a9 (#12682 / #12691 / #12834).

3135e5ca0 → efee49419. range-diff 10 commits =. GPG G. No file overlap with the three landed commits.

@HouMinXi
HouMinXi force-pushed the feat/combo-round-robin-extract branch from efee494 to ddd6bcb Compare September 7, 2026 02:33
@HouMinXi

HouMinXi commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/release/v3.8.51 b345c7f6c (#12870 OpenCode v2 plugin).

  • efee49419ae → ddd6bcbfcc5
  • range-diff identity (eq=3 unique); all commits GPG G
  • 10 unique commit(s) ahead, 0 behind
  • --force-with-lease to HouMinXi:feat/combo-round-robin-extract
    Stacked onto rebased Inner refactor(combo): split executeTarget into gates, attempt, and loop #12746 (152bf96db). Seven Inner commits skipped as already applied; three unique RR commits range-diff =.

diegosouzapw pushed a commit that referenced this pull request Sep 7, 2026
…12746)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437, ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

Guardar os contratos com testes ANTES de levantar o bloco (`05059880` no irmão, e o `287658f5` marcando os `executeTargetGates` como lift-as-is) é o que torna um refactor deste tamanho auditável. Sem essa ordem, um extract de 3 mil linhas é indistinguível de uma reescrita.

Revalidei depois do merge da base: `combo-attempt-loop`, `execute-target-attempt`, `execute-target-gates` e `combo-loop-safety-timer-leak-11804` — 20/20 — mais typecheck:core limpo e o cap de arquivo OK.

O #12811 entra na sequência logo em seguida, com os três commits do round-robin sobre este.
round-robin-combo.test.ts is RED until handleRoundRobinCombo lives in
roundRobinCombo.ts. combo-attempt-loop inner slice falls through to EOF
when that name is gone so the unused-delay assertion cannot go green
on a truncated empty range.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Whole-function cut. combo.ts hands round-robin turns to the new file
via dynamic import so the sticky-pin helpers can stay exported from
combo.ts without a static cycle. Zero dispatch-policy change.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Signed-off-by: Minxi Hou <houminxi@gmail.com>
@diegosouzapw
diegosouzapw force-pushed the feat/combo-round-robin-extract branch from ddd6bcb to bcd6cc0 Compare September 7, 2026 12:09
@diegosouzapw
diegosouzapw merged commit ce49d96 into diegosouzapw:release/v3.8.51 Sep 7, 2026
3 of 7 checks passed
@HouMinXi
HouMinXi deleted the feat/combo-round-robin-extract branch September 7, 2026 13:27
diegosouzapw pushed a commit that referenced this pull request Sep 10, 2026
…12884)

Um `ALL_TARGETS_SKIPPED` 503 que não diz qual janela esgotou é opaco justamente no momento em que o operador mais precisa saber. Alinhar os rótulos de janela AUTH com os da API de uso fecha a outra metade: dois nomes para a mesma coisa fazem o dashboard e o erro parecerem discordar.

Revalidei sobre o tip: **6/6**, typecheck:core limpo, check-file-size OK.

**Dois consertos meus na sua branch.**

1. `typecheck:core` falhava com `TS2345` em `comboAttemptLoop.ts` (linhas 130 e 416): o `QuotaSkipTarget` declarava `connectionId?: string`, mas o `ResolvedComboTarget` carrega `string | null` para alvo não-pinado. Alarguei para `string | null` no tipo de diagnóstico em vez de estreitar o call site — o módulo só **lê** o campo e a linha 29 já narrowa com `typeof === "string"`, então null não custa nada ali. Isso apareceu porque o `comboAttemptLoop` mudou de forma no #12746/#12811, mergeados nesta mesma campanha depois que você cortou a branch.

2. O `roundRobinCombo.ts` foi de 1198 para 1205 e cruzou o teto de 1200 para arquivo novo. Congelei com justificativa: o arquivo já nasceu em 1198 quando o #12811 o levantou de dentro do `combo.ts`, e os diagnósticos em si vivem no `quotaSkipDiagnostics.ts`, sob o cap. Registrei que a próxima extração natural é o corpo do attempt loop, mas que ele acabou de ser movido e deve assentar antes de ser cortado de novo.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iegosouzapw#12746)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437, ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

Guardar os contratos com testes ANTES de levantar o bloco (`05059880` no irmão, e o `287658f5` marcando os `executeTargetGates` como lift-as-is) é o que torna um refactor deste tamanho auditável. Sem essa ordem, um extract de 3 mil linhas é indistinguível de uma reescrita.

Revalidei depois do merge da base: `combo-attempt-loop`, `execute-target-attempt`, `execute-target-gates` e `combo-loop-safety-timer-leak-11804` — 20/20 — mais typecheck:core limpo e o cap de arquivo OK.

O diegosouzapw#12811 entra na sequência logo em seguida, com os três commits do round-robin sobre este.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iegosouzapw#12811)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437, ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

**Sobre a reconstrução da branch.** Esta PR continha os 7 commits do diegosouzapw#12746 mais os 3 do round-robin. O dono escolheu mergear os dois em sequência em vez de fechar um como subsumido, então depois que o squash do diegosouzapw#12746 entrou eu reconstruí esta branch: cherry-pick de `05059880`, `54168238` e `ddd6bcbf` sobre o tip novo, e force-push. Autoria preservada — os três commits continuam seus (`Minxi Hou <houminxi@gmail.com>`), verificado com `git log --format=%an` antes do push. A PR foi de +4167/−3187 em 14 arquivos para +1281/−1182 em 5, que é o delta real do round-robin.

O `05059880` ("guard round-robin extract before the lift") é o commit que faz esse tipo de extract ser revisável: sem um teste que fixe o contrato antes do movimento, mover 1198 linhas é indistinguível de reescrever 1198 linhas.

Revalidei sobre o tip reconstruído: `round-robin-combo`, `combo-attempt-loop`, `execute-target-attempt`, `execute-target-gates` e `combo-loop-safety-timer-leak-11804` — 24/24 — com typecheck:core limpo e o cap de arquivo OK.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…iegosouzapw#12884)

Um `ALL_TARGETS_SKIPPED` 503 que não diz qual janela esgotou é opaco justamente no momento em que o operador mais precisa saber. Alinhar os rótulos de janela AUTH com os da API de uso fecha a outra metade: dois nomes para a mesma coisa fazem o dashboard e o erro parecerem discordar.

Revalidei sobre o tip: **6/6**, typecheck:core limpo, check-file-size OK.

**Dois consertos meus na sua branch.**

1. `typecheck:core` falhava com `TS2345` em `comboAttemptLoop.ts` (linhas 130 e 416): o `QuotaSkipTarget` declarava `connectionId?: string`, mas o `ResolvedComboTarget` carrega `string | null` para alvo não-pinado. Alarguei para `string | null` no tipo de diagnóstico em vez de estreitar o call site — o módulo só **lê** o campo e a linha 29 já narrowa com `typeof === "string"`, então null não custa nada ali. Isso apareceu porque o `comboAttemptLoop` mudou de forma no diegosouzapw#12746/diegosouzapw#12811, mergeados nesta mesma campanha depois que você cortou a branch.

2. O `roundRobinCombo.ts` foi de 1198 para 1205 e cruzou o teto de 1200 para arquivo novo. Congelei com justificativa: o arquivo já nasceu em 1198 quando o diegosouzapw#12811 o levantou de dentro do `combo.ts`, e os diagnósticos em si vivem no `quotaSkipDiagnostics.ts`, sob o cap. Registrei que a próxima extração natural é o corpo do attempt loop, mas que ele acabou de ser movido e deve assentar antes de ser cortado de novo.
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