Repository navigation
Conversation
|
CI on this head ( Own-red (fixed, this SHA):
Inherited on
Unique files do not include glm.ts, changelog leading “- ”, or the
This contribution is intended for maintainer review and merge. |
c90553b to
bb94055
Compare
HouMinXi
changed the title
4413bfa to
64df9ed
Compare
64df9ed to
91d0372
Compare
|
Pushed
Local |
|
Holding this one out of today's batch deliberately, and it is not a judgement on the work. The other 20 PRs of yours merged today. Two of them rewrite the same function this decomposes: #13069 routes the non-streaming leg through The reason to give this its own round rather than carry it in a 21-PR batch: a behaviour-preserving decomposition of the hottest path in the codebase is exactly the change where the batch's economics stop working. The consolidated model runs focused tests per PR and leans on attribution when something goes red, and a refactor that touches every block of What it needs from here:
On the 252 lines over the Thanks for the other twenty. The live evidence on them — the X500 logs, the 36-minute outage write-up, the real |
032344c to
24a466a
Compare
|
Careful, well-executed decomposition — chatCore.ts goes from 6145 to 2752 lines (55% cut), and |
|
Thanks for taking this on — Holding it for now, for reasons that are about timing rather than the work itself:
What would make it mergeable: rebase onto the current The tip already carries |
|
@diegosouzapw I do want this merged; the earlier contributor-only / do-not-merge wording was my mistake, not a request to withhold the work. I have corrected the description and my earlier comment. Your current-base equivalence requirement is reasonable: an old conflict resolution is not evidence of a behavior-preserving split. I will not call the old tests proof for the new tip. The next revision needs block-by-block equivalence against the current source, preserved test discovery, and the broader adjacent suite before it is ready. |
24a466a to
131e14e
Compare
|
Precisa ser refeita sobre o tip atual. Não forcei a resolução, e explico por quê. Tentei o merge e abortei sem commitar nada. Os números: o tip avançou Resolver significaria decidir, para cada um dos 50 hunks de 9 PRs já mergeadas, em qual dos 6 módulos ele passa a morar, e reconstruir a fiação. Isso é reexecutar a extração, não resolver um conflito — e é trabalho seu, não meu, porque é o desenho da separação. Dois agravantes fecharam a decisão:
Sugestão: rebase/refazimento da extração sobre o tip atual, e mergear antes as PRs cirúrgicas do grupo — que é a ordem que o próprio plan-file sugere. O trabalho de separação continua valendo; só precisa partir de uma base que não seja de 9 commits atrás. |
|
Pinned the Claude OAuth raw passthrough wiring from #14053 onto this split so the three execute sites do not live only in the pre-split barrel.
Tests: This is not the tip re-extract. The PR is still CONFLICTING against current |
|
Will not hand-resolve the eight conflict regions. You are right that that would be re-running the extraction, not merging. Plan from here: re-derive leaves on current #12498 / #13448 / #13556 / #13069 / #13050 are already on the tip this re-extract starts from. |
9f4108c to
1de9b94
Compare
|
Re-extracted on current What is in this branch now:
Not in this branch: stream materialize, request prelude, and the Claude OAuth raw passthrough wiring that had leaked into the old head. Tests: 33/33 on the send + recovery + pipeline files. |
HouMinXi
changed the title
1de9b94 to
7c9e8ae
Compare
|
Title and body now match the branch. This PR is the sender leaf plus the recovery policy table. Stream completion, request prelude, cache, and translation stay in Head |
7c9e8ae to
80225fa
Compare
… file Review flagged that the probe's regex splice could mangle the source in ways the assertions would not notice. Checking it, that was already happening: the replacement wrapped the assignment in `void 0 && (providerUrl = ...` and left the parenthesis unbalanced, so the probe carried one parse error. That matters because a file which does not parse can report 2454 for reasons unrelated to the branch under test -- so the self-check could have been passing on a broken probe rather than on working flow analysis, which is the exact false negative it was added to rule out. The probe now comments the assignment out, leaving a file that parses cleanly, and two assertions pin what the old code only assumed: the probe has zero parse diagnostics, and it carries exactly one fewer assignment than the leaf. Measured after the change: 0 parse errors, 6 assignments down to 5, still two TS2454. All four failure shapes turn it red -- a probe that does not parse, a probe that drops nothing, a renumbered diagnostic, and a real gapped branch. Suite is 65 green, typecheck:core and lint clean. Signed-off-by: Minxi Hou <houminxi@gmail.com>
…ding The comment quoted the malformed splice inline, backticks and all. Encoding that line into the review pipeline's JSON payload failed mid-string, which surfaced as a parse error and a mismatched excerpt rather than anything about the test. Same explanation, described in words instead of quoting the broken expression. No behaviour change; the file's other inline code references round-trip fine, so they stay as they are. Suite is 65 green, typecheck:core and lint clean. Signed-off-by: Minxi Hou <houminxi@gmail.com>
Review pointed out the probe's line regex would only comment out the first line of a multi-line assignment. Reproduced it: reshaping one assignment across three lines left the guard green, because the regex simply matched a different single-line assignment instead and probed that one. The branch whose shape changed was never exercised. The probe targets now come from the AST, so a statement spanning several lines is removed whole, and every assignment is probed rather than just whichever the regex happened to hit first. Probing all of them surfaced something the single-probe version could not: not every removal is a compile error. Dropping the assignment in the retry branch leaves an earlier one reaching the same read, so the compiler sees a value and says nothing -- the fault there is a stale URL, not an unassigned one, which definite-assignment analysis does not model. Requiring every probe to fire would have been asserting something untrue about the code, so the check requires that at least one fires, which is what makes a clean result meaningful. Re-verified after loosening: a gapped branch is red, a multi-line assignment left intact is green, the same assignment gapped is red, and renumbering the diagnostic is red. Suite is 65 green, typecheck:core and lint clean. Signed-off-by: Minxi Hou <houminxi@gmail.com>
…arry-out The streaming leg needs eight values carried back because it can fall through and the barrel keeps using what it rebound. The non-streaming leg rebinds the same kinds of values -- currentModel, translatedBody, finalBody -- and carries none of them, which is only safe because the barrel tail-calls it: the result is returned directly and nothing in that block runs afterwards. Nothing recorded that. Adding work after the return would give the second leg the same stale-value defect this suite just fixed in the first one, and the suite would stay green. The test parses the barrel and asserts the shape: exactly one callsite, the result flowing to a return through nothing but await, and no statements after it. Each of those turning false is a real change in the contract, not a formatting difference, so it fails with a message naming the shape it lost. Verified against three ways the property can break: a statement after the return, capturing the result into a local instead of returning it, and a second callsite appearing. Suite is 66 green, typecheck:core and lint clean. Signed-off-by: Minxi Hou <houminxi@gmail.com>
…ds at split modules Re-point structural source guards in chatcore-hierarchical-admission.test.ts and chatcore-stream-error-result.test.ts at the split leaf modules: - Point wire-send loop and concurrency gate assertions at executeProviderRequest.ts - Point sendProviderAttempt assertions at streamingLeg.ts and nonStreamingLeg.ts - Point non-streaming semaphore try/catch assertions at nonStreamingLeg.ts - Assert that barrel chatCore.ts delegates and passes executeProviderRequest to both legs Signed-off-by: Minxi Hou <houminxi@gmail.com>
…eChatCore Extract the inline applyProviderFailureClassification logic (cooldowns, terminal statuses, lockouts, quota state updates, and response telemetry) into a dedicated leaf module open-sse/handlers/chatCore/providerFailureClassification.ts. The chatCore barrel now instantiates the classifier via createProviderFailureClassifier, reducing chatCore.ts from 2534 lines to 2211 lines (-323 lines). Signed-off-by: Minxi Hou <houminxi@gmail.com>
…dleChatCore Extract streaming success response header assembly, transform stream selection, pipeline chaining, and completion callback wiring into open-sse/handlers/chatCore/streamingSuccessResponse.ts. - Move streaming Response construction, transform stream creation (Responses API translation, general translation, or passthrough), codex turn-state provenance, and completion/failure callback registration to a dedicated leaf helper. - Pass dependencies explicitly via StreamingSuccessResponseDeps. - chatCore barrel delegates streaming completion directly to the leaf, reducing handleChatCore tail complexity and shrinking the barrel from 2211 to 2011 lines. - Add unit characterization test with defect injection verification. Signed-off-by: Minxi Hou <houminxi@gmail.com>
… into leaf module Lifts semantic cache check (temp=0 early return), reasoning input policy, body sanitization, memory/skills injection, proactive context compression, and output token budget enforcement out of handleChatCore into open-sse/handlers/chatCore/cacheAndCompress.ts. - Replaces inline Phases 9.1 through 9.4 with a typed runCacheAndCompress delegate - Reduces chatCore.ts barrel from 2011 to 1785 lines (-226 lines, cumulative -71.2%) - Passes all 685 unit tests with verified AST/runtime defect injections Signed-off-by: Minxi Hou <houminxi@gmail.com>
…eaf module Lifts request translation (format conversion, system prompt injection, tool normalization, message stripping), stream controller setup, executor resolution, credential binding, Claude passthrough handling, and request deduplication hashing out of handleChatCore into open-sse/handlers/chatCore/translateAndDedup.ts. - Replaces inline translation and dedup block with runTranslateAndDedup delegate - Drops chatCore.ts barrel from 1785 to 993 lines (-792 lines, cumulative -84.0%) - Adds tests/unit/chatcore/translate-and-dedup.test.ts verifying structural seams - Passes all 685 unit tests with verified runtime defect injections Signed-off-by: Minxi Hou <houminxi@gmail.com>
80225fa to
65512fb
Compare
HouMinXi
changed the title
|
Re-extracted on current What is in this branch now:
|
|
@diegosouzapw this one is super nice |
|
Re-extracted onto the current release/v3.8.51 tip as #14725: each leaf diffs empty against its tip region (imports/exports and deps/carry wiring aside), one clean commit, 209 chatcore-referencing test files show zero new failures against the tip baseline. Two wiring seams the suite caught after the lifts (translatedBody snapshot vs live binding; test re-anchors) are fixed and covered. This PR can stay open for history; the tip-based version is the one to review. |
Re-extraction of cheaper-inference#13065 onto the current release/v3.8.51 tip (a41ded2). Each leaf is lifted out of handleChatCore unchanged apart from imports/exports and the deps/carry wiring, so every leaf diffs empty against the tip implementation except the wiring: - executeProviderRequest.ts — one provider send (admission, body preparation, executor call, retry ladder) - nonStreamingResponse.ts — the !stream leg - streamingResponse.ts — the stream leg up to readiness - streamingTail.ts — readiness, translation pipeline, tail finalization While replaying, the 18 upstream hunks that landed inside the lifted regions were folded into the leaves so behaviour stays identical to the tip: getExecutorClientHeaders() call sites, metered-budget cost wrapping, the stream-readiness fallback hook, formatted FLUSH_EMPTY_RETRY verdict logging, captureStreamReasoningForReplay, videoTranscriptSensitive flags, and the connection-id provenance fields on the streaming response headers. The hard-lease inventory guard moves its FLUSH_EMPTY_RETRY expectation to the leaf's new home. Signed-off-by: Minxi Hou <houminxi@gmail.com>
diegosouzapw
added
the
deferred-v3.8.52
diegosouzapw
changed the title
The overflow fail-fast gate (finalEstimatedInputTokens >= finalContextLimit) moved from chatCore.ts into chatCore/cacheAndCompress.ts with the response-path decomposition, but the cheaper-inference#12793 regression guard's barrelSource still read chatCore.ts, silently asserting against a file that no longer contains the gate's logic. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…ingLeg streamingLeg.ts was 1311 lines, over the 1200-line cap for new files (config/quality/file-size-baseline.json). Lift the self-contained 401/403 credential-refresh-and-retry block into its own leaf, mirroring the deps-bag pattern already used across chatCore's other extractions. No behavior change: the block moves verbatim, taking its declared-but-unused-until-later state (upstreamErrorParsed, parsedStatusCode, parsedMessage, parsedRetryAfterMs, upstreamErrorBody) along as return values instead of shared closure locals. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
|
Re-homed to |
diegosouzapw
removed
the
deferred-v3.8.52
diegosouzapw
changed the title
|
Closing this. release/v3.8.52 already absorbed a different chatCore split through #14725 (streamingResponse, streamingTail, nonStreamingResponse). This branch is 865 commits behind that base and its leaf set no longer matches the tree. The remaining 3.8.53 decomposition will continue as smaller slices on the current release/v3.8.52 baseline: credential refresh, provider failure classification, post-translation request finalization, and pre-request checks. |
Summary
Re-extracted
handleChatCoredecomposition directly on currentrelease/v3.8.51tip (7a921299c5) instead of merging conflicting intermediate branches.chatCore.tsdrops from 6,200+ lines down to 992 lines by isolating discrete responsibilities into leaf modules underopen-sse/handlers/chatCore/.What changed
providerExecutionPipeline.ts: Provider execution pipeline and credential retry loop.recoveryPolicy.ts: Recovery decision table and error classification.streamingLeg.ts: SSE stream dispatch, lifecycle hooks, and abort listeners.nonStreamingLeg.ts: Non-streaming response collection and payload forwarding.materializeStreamSuccess.ts: Stream completion, usage calculation, and analytics persistence.cacheAndCompress.ts: Semantic cache lookup, cache hit construction, and context compression.translateAndDedup.ts: Request translation, tool normalization, stream controller setup, and request deduplication.chatCore.ts: Retains top-level routing orchestration and target retry dispatch (down to 992 lines).Verification
npm run typecheck:core: clean (0 errors)npm run check:any-budget:t11: passRelated to the earlier extract discussion on this PR. #12339 stays open. #12696 stays open.