Repository navigation
Conversation
Closes Tarquinen#631 ## Problem The report describes a day of work lost to a config typo: `modelMaxLimits` / `modelMinLimits` placed at the top level and per-model entries written as `compress["provider/model"]`, neither of which is in the schema. The plugin printed one `Unknown keys` line, then ran on the silent fallback of 100% of `model.limit.context`, so nudges fired at ~30k instead of the intended ~76k. Two things made that undiagnosable: 1. **The warning is emitted before the merge.** `showConfigWarnings` was called per config layer, *before* `mergeLayer` ran for that layer, so the effective values simply did not exist yet. This is why the warning could not name them. 2. **The thresholds are never logged anywhere.** `DCP initialized` logged only `strategies`. Since the failure mode of a schema mismatch is *wrong thresholds* rather than an error, and `getConfig` runs once at startup, there was no way to see what the plugin had decided without attaching a debugger. ## Change - `showConfigWarnings` now takes collected per-layer diagnostics and the **merged** config, and is called once at the end of `getConfig`. The toast therefore lists the thresholds that are in force, not the ones the offending layer tried to set. It also says the config is read once and OpenCode must be restarted, which was the other half of the report. - `describeEffectiveThresholds(config)` renders absolute limits as tokens and percentage limits as `"35%" of the model context window`. Percentages cannot be evaluated until the host reports the window, so the configured form is what the user needs to see. - `DCP initialized` now logs `thresholds` and the restart note, so the resolved values are in the debug log from the first line. ## Where the helper lives `describeEffectiveThresholds` moved to a new `lib/thresholds.ts`. `config.ts` imports `jsonc-parser/lib/esm/main.js`, whose published ESM entry has no named `parse` export — that is why `tsup.config.ts` already bundles `jsonc-parser`, and why no test could ever import `config.ts` at runtime (every existing test uses `import type`, which is erased). A pure formatting helper has no business pulling in a JSON parser, so this makes it testable at all. ## Verification `tests/config-warning.test.ts`, seven cases. All fail on the current implementation (the file cannot even load, since the helper does not exist): - absolute thresholds render as tokens - percentage thresholds say what they are relative to - a mixed configuration reports each form separately - the description uses the real schema keys and not the misplaced ones - `DCP initialized` logs the thresholds and the restart note - the warning is emitted *after* the merge and is no longer emitted per layer - the helper stays importable without the JSONC parser Full suite 131 passing, `tsc --noEmit` clean, `tsup` + declarations build, `prettier --check` clean. `verify-package.mjs` still passes its file, package.json and import-graph checks.
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.
fix(config): name the thresholds that are actually in force
Closes #631
Problem
The report describes a day of work lost to a config typo:
modelMaxLimits/modelMinLimitsplaced at the top level and per-model entries written ascompress["provider/model"], neither of which is in the schema. The pluginprinted one
Unknown keysline, then ran on the silent fallback of 100% ofmodel.limit.context, so nudges fired at ~30k instead of the intended ~76k.Two things made that undiagnosable:
The warning is emitted before the merge.
showConfigWarningswas called perconfig layer, before
mergeLayerran for that layer, so the effective valuessimply did not exist yet. This is why the warning could not name them.
The thresholds are never logged anywhere.
DCP initializedlogged onlystrategies. Since the failure mode of a schema mismatch is wrong thresholdsrather than an error, and
getConfigruns once at startup, there was no way tosee what the plugin had decided without attaching a debugger.
Change
showConfigWarningsnow takes collected per-layer diagnostics and the mergedconfig, and is called once at the end of
getConfig. The toast therefore liststhe thresholds that are in force, not the ones the offending layer tried to set.
It also says the config is read once and OpenCode must be restarted, which was
the other half of the report.
describeEffectiveThresholds(config)renders absolute limits as tokens andpercentage limits as
"35%" of the model context window. Percentages cannot beevaluated until the host reports the window, so the configured form is what the
user needs to see.
DCP initializednow logsthresholdsand the restart note, so the resolvedvalues are in the debug log from the first line.
Where the helper lives
describeEffectiveThresholdsmoved to a newlib/thresholds.ts.config.tsimports
jsonc-parser/lib/esm/main.js, whose published ESM entry has no namedparseexport - that is whytsup.config.tsalready bundlesjsonc-parser, andwhy no test could ever import
config.tsat runtime (every existing test usesimport type, which is erased). A pure formatting helper has no business pullingin a JSON parser, so this makes it testable at all.
Verification
tests/config-warning.test.ts, seven cases. All fail on the current implementation(the file cannot even load, since the helper does not exist):
DCP initializedlogs the thresholds and the restart noteFull suite 131 passing,
tsc --noEmitclean,tsup+ declarations build,prettier --checkclean.verify-package.mjsstill passes its file,package.json and import-graph checks.