Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
I couldn't check this end to end: the context picker only appears for a GitHub Copilot model with a long-context price tier, and that needs real Copilot credentials. The PR's core tests (30) and TUI tests (22) pass, and typecheck passes in packages/core and packages/tui. I couldn't produce screenshots for the same reason. The picker screenshots in the description cover the new dialog; a before/after of the prompt footer and sidebar cost line would help.
Public API (adds to the API and removes nothing, but clients will see the extra variants):
Model.Infogains an optionalcontext: { default: number, long: number }in the schema, OpenAPI files and generated client.- Copilot models that have a long-context tier now list extra variants
<effort>@default/<effort>@long(for exampledefault@longandhigh@default) next to the reasoning variants. - The stored variant string can now be
effort@tier, also through--model provider/model#variant. - New TUI command
context.listwith the/context-windowslash command.
Concerns
- The combined variants show up in other clients. The TUI filters them out, but the web/desktop app lists
Object.keys(model.variants)as-is (packages/app/src/providers/models/selection.tsx). For a Copilot model with low/medium/high reasoning, the app would offer eleven choices (default@default,low@long,high@default…), and picking one there changes the context window without saying so. Can the context tier be kept out of the variant list, for example by resolvingeffort@tierin the resolver without adding every combination tomodel.variants? If not, the app needs the same filter and picker. - Default behaviour changes for Copilot users. Users who haven't chosen anything are now held to the smaller default budget (
withVariantusescontext.defaultwhen no variant is selected), so their sessions compact earlier than before. That may be intended, matching Copilot CLI, but it should be called out.
Details are in the inline comments.
5fa63e2 to
2bbfa5c
Compare
There was a problem hiding this comment.
Both concerns from my last review are resolved:
- Variants listed in other clients: Copilot models now list only their reasoning variants.
provider.tsno longer adds tier combinations or skipsVariant.resolve. The resolver, the subagent tool, the TUI and the app accepteffort@tierthrough the shared@opencode/util/model-varianthelper. - Default budget change:
models.mdxnow says Default is used unless Long is selected, and that existing sessions compact earlier.
I couldn't run it end to end, because the picker only appears for a Copilot model that advertises separate input budgets, which needs Copilot credentials. The tests nearest the change pass (core 89, TUI 33, app 10), and typecheck passes in core, tui, app and util.
Public API (backwards compatible): Model.Info gains an optional context: { default, long } in the schema, OpenAPI files and generated client. The variant reference accepted by the server, the CLI #variant syntax and the subagent tool now also accepts <effort>@default|long for models with context. There's a new TUI command, context.list (/context-window). Copilot model variant lists are unchanged.
One small point that doesn't block a merge: the new app test (preserves combined saved selections… in packages/app/src/providers/models/variant.test.ts) packs ten edge-case assertions into one case. It would be easier to follow cut down to the core cases: keeping a saved effort@long, rejecting an unknown effort, and cycling effort without changing the tier.
|
Thanks, I checked both in aa8ce27. The config plugin is back on the normal path: no generated combinations and no |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Closes #53521. Replaces #53553 with a V2 port targeting
dev, as requested by @thdxr. Related to #38164 and #52089.Type of change
What does this PR do?
Adds Default/Long context selection for Copilot models that advertise separate input budgets.
/context-windowchanges the window without changing reasoning effort. The combined selection persists through the existing variant field, such asmedium@long.The resolver applies the selected input budget and preserves configured caps. Context percentages use the window of the latest response. Copilot token-price tiers include cached input, and Copilot-only sessions display estimated AI credits. Mixed-provider sessions keep USD. These are estimates, not GitHub's billed amounts.
Includes the generated client and OpenAPI contracts, mini-mode support, and documentation for V2's
--model provider/model#variantsyntax.The model catalog lists only actual reasoning variants. Context suffixes resolve directly, and existing selections remain valid across clients. Existing sessions without a context suffix use the smaller Default budget and may compact earlier. Long is opt-in.
How did you verify your code works?
With Bun 1.4.2:
bun run checkpassed.upstream/devat25053bbdc7. The isolated baseline run had 7 passed and 3 timeout failures.Screenshots / recordings
Production context picker rendered at 60 and 120 columns:
Checklist