diff --git a/devlog/2026-09-11_qualitygate-algo-config-keys/REQ.md b/devlog/2026-09-11_qualitygate-algo-config-keys/REQ.md new file mode 100644 index 00000000..a703b790 --- /dev/null +++ b/devlog/2026-09-11_qualitygate-algo-config-keys/REQ.md @@ -0,0 +1,40 @@ +# REQ — qualityGate.algorithms nested params falsely flagged as "Unknown keys" (#329) + +## Problem + +Configuring per-algorithm quality-gate parameters triggers a false startup warning: + +```jsonc +{ + "qualityGate": { + "enabled": true, + "algorithm": "rouge-recall-v1", + "algorithms": { + "rouge-recall-v1": { + "layer1MinChars": 200, + "layer1MinRetentionPct": 2.5, + "layer2MaxRougeF1": 0.05, + "layer2MaxTop20Recall": 0.20 + } + } + } +} +``` + +`ACP: config warning — Unknown keys: qualityGate.algorithms.rouge-recall-v1...` + +The config is legal and consumed at runtime (`lib/compress/quality-gate/evaluate.ts:104` reads `qg.algorithms[algoName]`) — only the key-allowlist check is wrong. + +## Root cause + +`getConfigKeyPaths()` in `lib/config-validation.ts` recurses into `qualityGate.algorithms` because it isn't in the dynamic-key skip list (unlike `compress.providers`, `messageFilters.filters`, `compress.modelMaxLimits`). Every nested key it emits (`qualityGate.algorithms.rouge-recall-v1`, `...layer1MinChars`) fails the static allow-list lookup. + +## Fix + +Add `qualityGate.algorithms` to the recursion skip list with a comment explaining the dynamic-map convention (next path segment = user-chosen algorithm id; inner shape validated by the owning subsystem). + +## Acceptance + +- [x] New tests: nested algorithm keys → no warning; sibling unknown keys still flagged +- [x] Full suite + typecheck green +- [ ] Dual-agent review diff --git a/devlog/2026-09-11_qualitygate-algo-config-keys/WORKLOG.md b/devlog/2026-09-11_qualitygate-algo-config-keys/WORKLOG.md new file mode 100644 index 00000000..3614b495 --- /dev/null +++ b/devlog/2026-09-11_qualitygate-algo-config-keys/WORKLOG.md @@ -0,0 +1,10 @@ +# WORKLOG — qualityGate.algorithms config-key fix + +1. Reproduced from issue #329 config sample: `getInvalidConfigKeys` returned `qualityGate.algorithms.rouge-recall-v1.*` paths. +2. Located skip-list in `lib/config-validation.ts::getConfigKeyPaths`, added `qualityGate.algorithms` (+ explanatory comment). +3. Added 2 tests in `tests/config-validation.test.ts`: + - nested algorithm params → `[]` (no warning) + - unknown sibling key (`qualityGate.notARealKey`) still flagged while algorithm params pass +4. Verification: config-validation suite 47/47; full suite 1209/1209; typecheck clean. +5. Dual-agent review dispatched on the PR. +6. Review follow-up (issue #329 floor, ranxianglei): the example/test param names used non-existent keys `minSummaryLength` / `rougeF1Threshold`. Corrected to the real `rouge-recall-v1` params (`layer1MinChars`, `layer1MinRetentionPct`, `layer2MaxRougeF1`, `layer2MaxTop20Recall`) in `REQ.md` (config sample + root-cause prose) and both new tests. Unknown inner keys are silently ignored at runtime, so the old sample would have configured params that never take effect. No logic change — validation fix untouched; suite re-verified green. diff --git a/lib/config-validation.ts b/lib/config-validation.ts index 3d56f41f..dba379fa 100644 --- a/lib/config-validation.ts +++ b/lib/config-validation.ts @@ -84,8 +84,14 @@ function getConfigKeyPaths(obj: Record, prefix = ""): string[] { fullKey === "compress.modelMaxLimits" || fullKey === "compress.modelMinLimits" || fullKey === "compress.providers" || - fullKey === "messageFilters.filters" + fullKey === "messageFilters.filters" || + fullKey === "qualityGate.algorithms" ) { + // Dynamic-key maps: the next path segment is a user-chosen name + // (provider id, filter id, registered algorithm id) whose inner + // shape is validated by the owning subsystem, not by this static + // allow-list. Recursing would flag every legal entry as + // "Unknown keys" (#329). continue } diff --git a/tests/config-validation.test.ts b/tests/config-validation.test.ts index fcb93578..8a9e3e51 100644 --- a/tests/config-validation.test.ts +++ b/tests/config-validation.test.ts @@ -55,6 +55,29 @@ test("getInvalidConfigKeys does not recurse into messageFilters.filters dynamic assert.deepEqual(result, []) }) +test("getInvalidConfigKeys does not recurse into qualityGate.algorithms dynamic keys (#329)", () => { + const result = getInvalidConfigKeys({ + qualityGate: { + enabled: true, + algorithm: "rouge-recall-v1", + algorithms: { + "rouge-recall-v1": { layer1MinChars: 200, layer2MaxRougeF1: 0.3 }, + }, + }, + }) + assert.deepEqual(result, []) +}) + +test("getInvalidConfigKeys still flags unknown qualityGate keys while allowing algorithm params (#329)", () => { + const result = getInvalidConfigKeys({ + qualityGate: { + algorithms: { "rouge-recall-v1": { layer1MinChars: 200 } }, + notARealKey: true, + }, + }) + assert.deepEqual(result, ["qualityGate.notARealKey"]) +}) + test("validateConfigTypes returns empty array for valid config", () => { const result = validateConfigTypes({ enabled: true,