Skip to content

fix(config): name the thresholds that are actually in force - #653

Open
Mukller wants to merge 1 commit into
Tarquinen:devfrom
Mukller:fix/config-warning-effective-thresholds
Open

Mukller wants to merge 1 commit into
Tarquinen:devfrom
Mukller:fix/config-warning-effective-thresholds

Conversation

@Mukller

@Mukller Mukller commented Oct 3, 2026

Copy link
Copy Markdown

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 /
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.

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.
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.

1 participant