fix: cap compaction and completion max_tokens to provider limits - #12
Merged
Conversation
Per-agent progress previously jumped through fixed stage values and capped at 90 for the entire finalizing stream. Each model delta now creeps toward the stage ceiling with a minimum step, the aggregate line shows Finalizing once every delegated agent is terminal but the result has not arrived, and narrow-width member rows pad the state token so the task column no longer shifts between phases.
Compaction requests sent max_tokens equal to the model's full context window whenever maxOutputSize was not configured, which strict OpenAI-compatible providers reject with 400 invalid_request_error (e.g. "Invalid max_tokens value, the valid range of max_tokens is [1, 393216]"). Cap compaction output at 128k by default and size chat-completions caps to the remaining context window.
…e preflight Add Homebrew install detection with an update helper and activation step so brew-managed CLIs restart onto the new version after /update, and update the doctor, preflight, and docs accordingly.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis pull request adds restart-based Homebrew updates, durable update state, context-aware completion-token caps, and event-driven Dynamic Workflow progress. It also updates diagnostics, TUI messages, tests, documentation, and release changesets. ChangesUpdate lifecycle
Completion budget
Dynamic Workflow progress
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Preflight
participant UpdateHelper
participant Homebrew
participant InstallState
CLI->>Preflight: request Homebrew update
Preflight->>InstallState: record preparation job
Preflight->>UpdateHelper: launch detached preparation
UpdateHelper->>Homebrew: prepare and verify artifact
Homebrew-->>UpdateHelper: return prepared metadata
UpdateHelper->>InstallState: persist pending update
CLI->>InstallState: load pending update at startup
CLI->>Homebrew: activate verified update
Homebrew-->>CLI: return activated executable
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
apps/pythinker-code/src/cli/update/preflight.ts (1)
571-583: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNarrow the
operationparameter to the helper's accepted values.
operationis typed asstring. Only'prepare-homebrew'is ever passed, and the helper validates the value withPrepareHomebrewArgsSchema. A literal union would make a typo a compile error instead of a runtime helper rejection.♻️ Proposed change
function updateHelperCommand( - operation: string, + operation: 'prepare-homebrew', jobId: string, version: string, requestedBy: UpdateRequestOrigin, ): SpawnCommand {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/cli/update/preflight.ts` around lines 571 - 583, Update the operation parameter of updateHelperCommand to a literal type containing the helper’s accepted operation value, currently 'prepare-homebrew', so invalid operations fail at compile time while preserving the existing command construction.apps/pythinker-code/test/cli/update/preflight.test.ts (1)
488-497: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact helper argument array, not a subset.
Both tests use
expect.arrayContaining. That matcher ignores order and position.runUpdateHelperdestructures the arguments positionally withconst [, jobId, requestedVersion, requestedBy] = parsed.data, so order is part of the contract. A reordered or shifted argument list would still pass these assertions.Assert the full array so the positional contract is covered.
♻️ Proposed change for the preflight test
expect(mocks.spawn).toHaveBeenCalledWith( process.execPath, - expect.arrayContaining([ - '__update_helper', - 'prepare-homebrew', - '0.5.0', - 'automatic', - ]), + [ + process.argv[1], + '__update_helper', + 'prepare-homebrew', + expect.any(String), + '0.5.0', + 'automatic', + ], expect.objectContaining({ detached: true, stdio: 'ignore' }), );♻️ Proposed change for the manual-update test
expect(mocks.spawn).toHaveBeenCalledWith( process.execPath, - expect.arrayContaining(['prepare-homebrew', '0.5.0', 'manual']), + [ + process.argv[1], + '__update_helper', + 'prepare-homebrew', + expect.any(String), + '0.5.0', + 'manual', + ], expect.objectContaining({ detached: true, stdio: 'ignore' }), );Also applies to: 1547-1551
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/test/cli/update/preflight.test.ts` around lines 488 - 497, Update both spawn assertions in the preflight and manual-update tests to use an exact array matcher for the helper arguments instead of expect.arrayContaining. Preserve and verify the expected positional order used by runUpdateHelper’s parsed.data destructuring, while keeping the existing spawn options assertion unchanged.apps/pythinker-code/src/cli/update/install-state.ts (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace both
z.string().uuid()calls withz.uuid(). Zod 4 deprecates the method form.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/cli/update/install-state.ts` at line 30, Update both UUID schema definitions in the install-state validation to use Zod 4’s standalone z.uuid() form instead of the deprecated z.string().uuid() method, while preserving their existing optionality and validation behavior.apps/pythinker-code/src/cli/update/activation.ts (1)
108-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
pending.sourceinstead of the hardcoded literal.Line 119 writes
source: 'homebrew'.pending.sourcealready carries that value and is the literal'homebrew'inUpdatePreparedHomebrew, so behavior is identical today. Reading it frompendingkeeps the record correct if a second prepared source is added later.♻️ Proposed change
active: { version: pending.version, - source: 'homebrew', + source: pending.source, operation: 'activate',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/cli/update/activation.ts` around lines 108 - 134, Update the activatingState construction in the activation flow to set active.source from pending.source instead of the hardcoded 'homebrew' literal, while leaving the rest of the state transition unchanged.apps/pythinker-code/test/cli/update/update-helper.test.ts (1)
18-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one module specifier style for the mocked module.
Line 18 mocks
'../../../src/cli/update/homebrew'while line 20 imports the actual module through'#/cli/update/homebrew'. Both resolve to the same file today, so the mock applies. Using the alias in both places removes the dependence on that resolution equivalence.♻️ Proposed change
-vi.mock('../../../src/cli/update/homebrew', async () => { +vi.mock('`#/cli/update/homebrew`', async () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/test/cli/update/update-helper.test.ts` around lines 18 - 21, Update the vi.mock declaration in the update-helper test to use the same '`#/cli/update/homebrew`' module specifier already passed to vi.importActual, keeping both references consistent and preserving the existing mock behavior.Source: Coding guidelines
apps/pythinker-code/src/cli/update/homebrew.ts (1)
90-127: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to
brewinvocations.
runHomebrewCommandwaits forclosewith no time bound.brew update,brew fetch, andbrew upgradecan stall on network or lock contention. The detached helper holdsactivestate for the job while it waits, so a stalled child blocks later prepare attempts until the state is cleared elsewhere.Add an optional timeout and kill the child when it expires.
♻️ Proposed timeout support
export interface HomebrewCommandOptions { readonly capture?: boolean; readonly inheritOutput?: boolean; readonly env?: NodeJS.ProcessEnv; readonly logFile?: string; + readonly timeoutMs?: number; }const child = spawn('brew', [...args], { cwd: homedir(), env: { ...process.env, ...options.env }, stdio: ['ignore', 'pipe', 'pipe'], + timeout: options.timeoutMs, + killSignal: 'SIGTERM', });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/cli/update/homebrew.ts` around lines 90 - 127, Update runHomebrewCommand to accept an optional timeout, start a timer for each spawned brew child, and terminate the child when the timeout expires so the promise settles and active job state is released. Clear the timer when the child exits, while preserving existing stdout, stderr, and normal completion handling.apps/pythinker-code/src/cli/update/update-helper.ts (1)
15-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
z.uuid()for the UUID schema. The top-level Zod 4 format matches the RFC 4122 v4 IDs generated byrandomUUID().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/cli/update/update-helper.ts` around lines 15 - 20, Update PrepareHomebrewArgsSchema to replace the UUID string validation with the top-level z.uuid() schema, while preserving the existing tuple order and other argument validations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/pythinker-code/src/cli/update/activation.ts`:
- Around line 137-151: Update the PreparedHomebrewUpdateInvalidError branch in
the activation flow to preserve the existing cumulative prepare-failure attempt
count instead of resetting attempts to 1. Reuse the prior lastFailure state when
it represents the same pending version, increment its prepare attempts, and
persist the incremented value in the invalidation result so repeated invalid
artifacts can reach AUTO_INSTALL_FAILURE_PROMPT_THRESHOLD.
- Around line 100-106: Update the failure branch in the activation flow around
activationAttempts so reaching ACTIVATION_FAILURE_LIMIT persists the terminal
pending-update state as pending: null before returning the failed status.
Preserve pending.version, the failure message, and any existing lastFailure
value needed by preflight fallback handling.
In `@apps/pythinker-code/src/cli/update/install-lock.ts`:
- Around line 127-147: Update tryAcquireUpdateInstallLock’s publication path
around link(stagedPath, filePath) to handle data directories that do not support
hard links without rethrowing an avoidable filesystem error. Preserve atomic
no-overwrite lock publication and do not replace it with direct open(filePath,
'wx'); either validate/document the hard-link requirement or implement an
alternative portable protocol, while retaining cleanup of stagedPath.
In `@apps/pythinker-code/src/cli/update/preflight.ts`:
- Around line 611-623: In the preparation state created around startedState,
preserve the existing verified pending update instead of resetting pending to
null when a new job begins. Keep the success path’s existing behavior of
replacing pending with the newly prepared update, and ensure retained pending
state remains compatible with active preparation without emitting a misleading
notice.
- Around line 640-651: Update failureAttemptsFor to match both version and
operation, then pass the relevant operation at every caller. Ensure each failure
writer supplies its own operation when calculating attempts and recording
lastFailure, so prepare, install, and activate failures maintain independent
counters.
In
`@apps/pythinker-code/src/tui/components/dialogs/update-preference-selector.ts`:
- Line 7: Update the description in the preference selector to use
source-neutral wording that applies to all installation sources; remove the
Homebrew-specific and next-launch installation claim while preserving the
automatic-update meaning.
In `@apps/pythinker-code/test/cli/update/update-helper.test.ts`:
- Around line 125-244: Update the detached-process test beginning “finishes
preparation in a detached process after its parent exits” to use
it.skipIf(process.platform === 'win32') and retain the existing 12_000 timeout
argument, so the test is skipped on Windows while preserving its current
behavior elsewhere.
In `@docs/guides/getting-started.md`:
- Line 64: Update the “Upgrade” description to match the command reference:
state that running `pythinker upgrade` checks immediately and offers to install
the update now for global npm, pnpm, yarn, bun, and macOS/Linux native
installations, while Homebrew and Windows native installations print the update
command instead. Correct the subject so the CLI, rather than the reader,
presents the option.
---
Nitpick comments:
In `@apps/pythinker-code/src/cli/update/activation.ts`:
- Around line 108-134: Update the activatingState construction in the activation
flow to set active.source from pending.source instead of the hardcoded
'homebrew' literal, while leaving the rest of the state transition unchanged.
In `@apps/pythinker-code/src/cli/update/homebrew.ts`:
- Around line 90-127: Update runHomebrewCommand to accept an optional timeout,
start a timer for each spawned brew child, and terminate the child when the
timeout expires so the promise settles and active job state is released. Clear
the timer when the child exits, while preserving existing stdout, stderr, and
normal completion handling.
In `@apps/pythinker-code/src/cli/update/install-state.ts`:
- Line 30: Update both UUID schema definitions in the install-state validation
to use Zod 4’s standalone z.uuid() form instead of the deprecated
z.string().uuid() method, while preserving their existing optionality and
validation behavior.
In `@apps/pythinker-code/src/cli/update/preflight.ts`:
- Around line 571-583: Update the operation parameter of updateHelperCommand to
a literal type containing the helper’s accepted operation value, currently
'prepare-homebrew', so invalid operations fail at compile time while preserving
the existing command construction.
In `@apps/pythinker-code/src/cli/update/update-helper.ts`:
- Around line 15-20: Update PrepareHomebrewArgsSchema to replace the UUID string
validation with the top-level z.uuid() schema, while preserving the existing
tuple order and other argument validations.
In `@apps/pythinker-code/test/cli/update/preflight.test.ts`:
- Around line 488-497: Update both spawn assertions in the preflight and
manual-update tests to use an exact array matcher for the helper arguments
instead of expect.arrayContaining. Preserve and verify the expected positional
order used by runUpdateHelper’s parsed.data destructuring, while keeping the
existing spawn options assertion unchanged.
In `@apps/pythinker-code/test/cli/update/update-helper.test.ts`:
- Around line 18-21: Update the vi.mock declaration in the update-helper test to
use the same '`#/cli/update/homebrew`' module specifier already passed to
vi.importActual, keeping both references consistent and preserving the existing
mock behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bcc013b1-dc28-4b52-973e-d5e39686292e
📒 Files selected for processing (43)
.changeset/compaction-max-tokens-cap.md.changeset/dynamic-workflow-progress-stall.md.changeset/homebrew-restart-updates.mdapps/pythinker-code/src/cli/sub/doctor.tsapps/pythinker-code/src/cli/update/activation.tsapps/pythinker-code/src/cli/update/homebrew.tsapps/pythinker-code/src/cli/update/install-lock.tsapps/pythinker-code/src/cli/update/install-state.tsapps/pythinker-code/src/cli/update/preflight.tsapps/pythinker-code/src/cli/update/types.tsapps/pythinker-code/src/cli/update/update-helper.tsapps/pythinker-code/src/constant/app.tsapps/pythinker-code/src/main.tsapps/pythinker-code/src/tui/commands/info.tsapps/pythinker-code/src/tui/components/dialogs/update-preference-selector.tsapps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.tsapps/pythinker-code/src/tui/constant/rendering.tsapps/pythinker-code/src/utils/paths.tsapps/pythinker-code/src/utils/persistence.tsapps/pythinker-code/test/cli/doctor.test.tsapps/pythinker-code/test/cli/main.test.tsapps/pythinker-code/test/cli/update/activation.test.tsapps/pythinker-code/test/cli/update/cache.test.tsapps/pythinker-code/test/cli/update/preflight.test.tsapps/pythinker-code/test/cli/update/update-helper.test.tsapps/pythinker-code/test/tui/components/dialogs/choice-picker.test.tsapps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.tsdocs/configuration/config-files.mddocs/configuration/data-locations.mddocs/configuration/env-vars.mddocs/guides/getting-started.mddocs/reference/pythinker-command.mdpackages/agent-core/src/agent/compaction/full.tspackages/agent-core/src/agent/index.tspackages/agent-core/src/agent/turn/kosong-llm.tspackages/agent-core/src/utils/completion-budget.tspackages/agent-core/test/agent/compaction/full.test.tspackages/agent-core/test/agent/config-state.test.tspackages/kosong/src/provider.tspackages/kosong/src/providers/openai-legacy.tspackages/kosong/src/providers/pythinker.tspackages/kosong/test/openai-legacy.test.tspackages/kosong/test/pythinker.test.ts
- Centralize the remaining-context-window clamp in computeCompletionBudgetCap
so every provider benefits, and pass usedContextTokens from compaction —
the near-full request the cap exists for. Remove the per-provider
MaxCompletionTokensOptions duplication from kosong.
- Clear the pending update record once the activation failure limit is
reached so startup and /update stop reporting a dead update forever.
- Keep lastFailure across a successful prepare so invalid-artifact
activations accumulate toward the failure threshold.
- Retain a verified pending update while a newer preparation runs.
- Scope failure-attempt increments by operation; record install failures
with operation: 'install'.
- Return whether background prepare/install actually started and map a
refused start to 'in-progress' instead of claiming 'started'.
- Make writeJsonFile fsync opt-in ({ durable: true }, install.json only).
- Dedupe formatErrorMessage into cli/update/format-error.ts; normalize
appendLog chunks instead of duplicated branches; z.uuid(); source-neutral
update-preference wording; align pythinker upgrade docs; skip the
POSIX-only detached-helper test on Windows; exact helper spawn asserts.
Member
Author
|
Review findings addressed in 1ea5543. Nitpicks: fixed — 'prepare-homebrew' literal type, exact helper spawn-argument asserts, z.uuid() (both files), active.source from pending.source, alias-style vi.mock specifier. Skipped — brew invocation timeout: advisory; a stalled brew holds only the per-job active record, which the existing freshness check (hasFreshActiveInstall) already expires, and adding kill-timer plumbing to the helper isn't warranted without a report. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/pythinker-code/src/utils/persistence.ts (1)
76-89: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDo not fail a completed write when the directory close rejects.
Line 76 completes the atomic rename. After that point the write has succeeded.
directory.sync()already tolerates failure, butdirectory.close()on line 86 can still reject (for exampleEIO). That rejection reaches the outer catch,unlink(tmpPath)finds nothing, andwriteJsonFilerejects for a write that is already durable on disk. Callers such aswriteUpdateInstallStatethen record a spurious update failure.Swallow the close rejection to keep the post-rename path non-fatal.
♻️ Proposed fix
const directory = await open(dirname(filePath), 'r').catch(() => null); if (directory !== null) { try { await directory.sync().catch(() => {}); } finally { - await directory.close(); + await directory.close().catch(() => {}); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/utils/persistence.ts` around lines 76 - 89, Update the directory cleanup in the durable branch of writeJsonFile so directory.close() rejection is swallowed, just like directory.sync(). Keep the completed rename non-fatal after await rename(tmpPath, filePath), while preserving the existing close execution in the finally block.apps/pythinker-code/test/cli/update/preflight.test.ts (1)
1580-1615: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the retained
pendingupdate during a new preparation.
startBackgroundHomebrewPreparationkeeps an existing verifiedpendingwhen it starts a newer preparation. It relies on the...freshStatespread instartedState. No test asserts that behavior. A future change that setspending: nullin the started state would remove an installable update without failing any test.Add a case that starts preparation for
0.6.0with a verifiedpendingat0.5.0and asserts the persisted started state still carries thatpending.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/test/cli/update/preflight.test.ts` around lines 1580 - 1615, Add a test for startBackgroundHomebrewPreparation that begins preparing version 0.6.0 while install state contains a verified pending update for 0.5.0, then assert the persisted started state retains that pending update. Reuse the existing pending-state and mock helpers, and verify writeUpdateInstallState receives the retained pending value.apps/pythinker-code/src/cli/update/preflight.ts (1)
954-969: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReport the ready update even when the promotion write fails.
The prepared update stays installable on restart.
requestedByis ownership metadata only. IfwriteUpdateInstallStatefails, this path returnscheck-failedand hides an update that is already verified and ready. The user then sees an error instead of the restart instruction.Treat the promotion write as best effort and keep returning the ready result.
♻️ Proposed fix
const pending = installState.pending; if (pending.requestedBy === 'automatic') { - try { - await writeUpdateInstallState({ - ...installState, - pending: { ...pending, requestedBy: 'manual' }, - }); - } catch (error) { - return { status: 'check-failed', message: formatErrorMessage(error) }; - } + // Ownership metadata only: the prepared update stays installable even + // when this write fails, so do not report a check failure. + await writeUpdateInstallState({ + ...installState, + pending: { ...pending, requestedBy: 'manual' }, + }).catch(() => {}); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/cli/update/preflight.ts` around lines 954 - 969, Update the automatic-promotion block in the preflight flow so failures from writeUpdateInstallState do not return check-failed. Treat the requestedBy promotion as best effort, preserve the error handling only as appropriate for non-blocking reporting, and always continue to return the existing in-progress ready-to-install result for the verified pending update.apps/pythinker-code/test/cli/update/update-helper.test.ts (1)
125-146: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winTest source and version mismatches separately.
This fixture changes both
active.sourceandactive.version. The test passes ifownsPrepareJobvalidates only one field. Add one case with the matching source and a wrong version. Add one case with the matching version and a wrong source. This preserves the helper ownership contract before it changes install state.As per path instructions, “Tests must be able to fail.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/test/cli/update/update-helper.test.ts` around lines 125 - 146, Split the test around runUpdateHelper into two independent cases: one with the expected source and mismatched version, and another with the expected version and mismatched source. Keep each case asserting that prepareHomebrewUpdate is not called and the active state remains unchanged, so ownsPrepareJob must validate both fields.Source: Path instructions
packages/kosong/src/providers/openai-legacy.ts (1)
655-657: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd provider-level Vitest coverage for the changed bounds.
The supplied tests cover the central budget helper but not the concrete provider behavior.
- packages/kosong/src/providers/openai-legacy.ts#L655-L657: test both token-parameter branches and the 128K and minimum-one boundaries.
- packages/kosong/src/providers/pythinker.ts#L554-L555: test negative, zero, and positive completion-token values.
As per path instructions, new behavior in published library code should have Vitest coverage; extend existing suites.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kosong/src/providers/openai-legacy.ts` around lines 655 - 657, Extend the existing Vitest provider suites to cover OpenAILegacyChatProvider.withMaxCompletionTokens, testing both token-parameter branches at the 128K ceiling and minimum-one boundary; also cover Pythinker’s completion-token handling for negative, zero, and positive values. Apply changes in packages/kosong/src/providers/openai-legacy.ts lines 655-657 and packages/kosong/src/providers/pythinker.ts lines 554-555, with the implementation sites requiring no direct changes.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/agent-core/test/agent/compaction/full.test.ts`:
- Around line 1911-1913: Update the assertions around
compactionMaxCompletionTokens in the full compaction test to derive tokensBefore
with estimateTokensForMessages and assert the exact expected value Math.max(1,
maxContextTokens - tokensBefore). Replace the compile-time as number cast with a
runtime type guard that verifies the provider parameter before using it.
In `@packages/kosong/src/providers/openai-legacy.ts`:
- Around line 655-657: Preserve the exported ChatProvider contract by restoring
a deprecated compatibility overload for withMaxCompletionTokens that accepts
MaxCompletionTokensOptions, while retaining the numeric overload and existing
behavior. Ensure the options form still applies context-aware clamping using
usedContextTokens and maxContextTokens before delegating through
withGenerationKwargs, including for JavaScript callers.
- Around line 655-657: Preserve backward compatibility for
withMaxCompletionTokens in OpenAILegacyChatProvider at
packages/kosong/src/providers/openai-legacy.ts:655-657 and PythinkerChatProvider
at packages/kosong/src/providers/pythinker.ts:554-555 by restoring a deprecated
overload accepting MaxCompletionTokensOptions and retaining its runtime
context-aware clamping; alternatively, explicitly version and document the
breaking API change, including updating the package version from 0.4.6.
---
Nitpick comments:
In `@apps/pythinker-code/src/cli/update/preflight.ts`:
- Around line 954-969: Update the automatic-promotion block in the preflight
flow so failures from writeUpdateInstallState do not return check-failed. Treat
the requestedBy promotion as best effort, preserve the error handling only as
appropriate for non-blocking reporting, and always continue to return the
existing in-progress ready-to-install result for the verified pending update.
In `@apps/pythinker-code/src/utils/persistence.ts`:
- Around line 76-89: Update the directory cleanup in the durable branch of
writeJsonFile so directory.close() rejection is swallowed, just like
directory.sync(). Keep the completed rename non-fatal after await
rename(tmpPath, filePath), while preserving the existing close execution in the
finally block.
In `@apps/pythinker-code/test/cli/update/preflight.test.ts`:
- Around line 1580-1615: Add a test for startBackgroundHomebrewPreparation that
begins preparing version 0.6.0 while install state contains a verified pending
update for 0.5.0, then assert the persisted started state retains that pending
update. Reuse the existing pending-state and mock helpers, and verify
writeUpdateInstallState receives the retained pending value.
In `@apps/pythinker-code/test/cli/update/update-helper.test.ts`:
- Around line 125-146: Split the test around runUpdateHelper into two
independent cases: one with the expected source and mismatched version, and
another with the expected version and mismatched source. Keep each case
asserting that prepareHomebrewUpdate is not called and the active state remains
unchanged, so ownsPrepareJob must validate both fields.
In `@packages/kosong/src/providers/openai-legacy.ts`:
- Around line 655-657: Extend the existing Vitest provider suites to cover
OpenAILegacyChatProvider.withMaxCompletionTokens, testing both token-parameter
branches at the 128K ceiling and minimum-one boundary; also cover Pythinker’s
completion-token handling for negative, zero, and positive values. Apply changes
in packages/kosong/src/providers/openai-legacy.ts lines 655-657 and
packages/kosong/src/providers/pythinker.ts lines 554-555, with the
implementation sites requiring no direct changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d87ae561-5dcb-4081-8288-53d1b83297e7
📒 Files selected for processing (21)
apps/pythinker-code/src/cli/update/activation.tsapps/pythinker-code/src/cli/update/format-error.tsapps/pythinker-code/src/cli/update/homebrew.tsapps/pythinker-code/src/cli/update/install-state.tsapps/pythinker-code/src/cli/update/preflight.tsapps/pythinker-code/src/cli/update/update-helper.tsapps/pythinker-code/src/tui/components/dialogs/update-preference-selector.tsapps/pythinker-code/src/utils/persistence.tsapps/pythinker-code/test/cli/update/activation.test.tsapps/pythinker-code/test/cli/update/preflight.test.tsapps/pythinker-code/test/cli/update/update-helper.test.tsapps/pythinker-code/test/tui/components/dialogs/choice-picker.test.tsdocs/guides/getting-started.mdpackages/agent-core/src/agent/compaction/full.tspackages/agent-core/src/utils/completion-budget.tspackages/agent-core/test/agent/compaction/full.test.tspackages/agent-core/test/utils/completion-budget.test.tspackages/kosong/src/provider.tspackages/kosong/src/providers/openai-legacy.tspackages/kosong/src/providers/pythinker.tspackages/kosong/test/openai-legacy.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/kosong/test/openai-legacy.test.ts
- apps/pythinker-code/src/cli/update/install-state.ts
- docs/guides/getting-started.md
- apps/pythinker-code/src/tui/components/dialogs/update-preference-selector.ts
- apps/pythinker-code/test/cli/update/activation.test.ts
- packages/kosong/src/provider.ts
- apps/pythinker-code/test/tui/components/dialogs/choice-picker.test.ts
- apps/pythinker-code/src/cli/update/update-helper.ts
- packages/agent-core/src/agent/compaction/full.ts
- apps/pythinker-code/src/cli/update/activation.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/agent-core/test/agent/compaction/full.test.ts (1)
1912-1913: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the runtime type guard out of the test body.
vitest(no-conditional-in-test)flags this branch. Extract the guard into a helper outside the test so the test remains branch-free. Keep runtime validation; do not replace it with a type assertion.This addresses the reported oxlint warning for conditionals in tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/agent-core/test/agent/compaction/full.test.ts` around lines 1912 - 1913, Extract the typeof cap runtime validation from the test body into a helper defined outside the test, preserving the existing TypeError message and validation behavior. Update the test to call the helper so its body contains no conditional branch or type assertion.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/agent-core/test/agent/compaction/full.test.ts`:
- Around line 1912-1913: Extract the typeof cap runtime validation from the test
body into a helper defined outside the test, preserving the existing TypeError
message and validation behavior. Update the test to call the helper so its body
contains no conditional branch or type assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d79d866c-7f95-4b3f-bf99-f078f8cb8066
📒 Files selected for processing (1)
packages/agent-core/test/agent/compaction/full.test.ts
Merged
elkaix
pushed a commit
that referenced
this pull request
Aug 4, 2026
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @pythoughts/pythinker-code@0.7.0 ### Minor Changes - [#12](#12) [`02f7f8d`](02f7f8d) - Prepare verified Homebrew updates in the background and install them automatically on the next interactive launch. ### Patch Changes - [#12](#12) [`02f7f8d`](02f7f8d) - Fix context compaction failing with provider "Invalid max_tokens" errors by capping requested completion tokens to the remaining context window and a safe output ceiling instead of the full context window size. - [#12](#12) [`02f7f8d`](02f7f8d) - Fix Dynamic Workflow progress sticking at 90% during long streaming, show a Finalizing state once all delegated agents finish, and fix member row alignment at narrow widths. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This was referenced Aug 4, 2026
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
1 participant
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.
Related Issue
No issue filed — problem explained directly.
Problem
Context compaction fails with a provider 400 whenever the model alias has no
maxOutputSizeconfigured:The compaction request resolved its completion budget without
maxOutputSize, so the cap fell back to the model's full context window and was sent asmax_tokens. Providers that enforceinput + max_tokens <= context window(Moonshot/Console gateways, DeepSeek, Groq, and also Anthropic/OpenAI/Google under the same conditions) reject the request outright. The class of bug is provider-agnostic; compaction just hits it first because its prompt is the largest.What changed
Port of the two upstream fixes for this (kimi-code
794db5553and76c643bcb), adapted to pythinker:maxOutputSize ?? min(contextWindow, 128k)instead of falling through to the context window (packages/agent-core/src/agent/compaction/full.ts).withMaxCompletionTokensgained optional context: OpenAI-legacy and Pythinker chat-completions providers clamp the cap to the remaining context window (maxContextTokens - usedContextTokens), and OpenAI-legacy additionally to a 128k ceiling (packages/kosong).KosongLLMso per-request caps shrink as the context fills.Also includes a second commit: restart Homebrew-managed installs after
/update(helper + activation step, doctor/preflight updates, docs). Both changes carry their own changeset; happy to split into a separate PR if preferred.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.Summary by CodeRabbit