Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions devlog/2026-09-11_qualitygate-algo-config-keys/REQ.md
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions devlog/2026-09-11_qualitygate-algo-config-keys/WORKLOG.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 7 additions & 1 deletion lib/config-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,14 @@ function getConfigKeyPaths(obj: Record<string, any>, 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
}

Expand Down
23 changes: 23 additions & 0 deletions tests/config-validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading