Skip to content

fix: qualityGate.algorithms nested params no longer flagged as unknown keys - #389

Merged
ranxianglei merged 2 commits into
masterfrom
2026-09-11_qualitygate-algo-config-keys
Sep 12, 2026
Merged

fix: qualityGate.algorithms nested params no longer flagged as unknown keys#389
ranxianglei merged 2 commits into
masterfrom
2026-09-11_qualitygate-algo-config-keys

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Fixes #329.

qualityGate.algorithms.<algoName>.<param> is legal config (consumed by lib/compress/quality-gate/evaluate.ts) but getConfigKeyPaths recursed into the dynamic map and every nested key failed the static allow-list → false ACP: config warning — Unknown keys on every startup.

Fix: add qualityGate.algorithms to the recursion skip list, same convention as compress.providers / messageFilters.filters (dynamic-key maps validated by their owning subsystem).

Tests: 2 new (nested params clean; unknown siblings still flagged). Suite 1209/1209; typecheck clean.

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown

📦 Built Plugin Artifact

Branch: 2026-09-11_qualitygate-algo-config-keys (6c7c740)

Option A — Install from npm PR tag (recommended)

opencode plugin opencode-acp@pr-389 --global

Each push to this PR publishes a new version under the pr-389 npm tag.

Option B — Install from GitHub

opencode plugin "github:ranxianglei/opencode-acp#2026-09-11_qualitygate-algo-config-keys" --global

Option C — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf opencode-acp-pr389.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist
  1. Restart opencode to pick up changes.

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Review of PR #389 (fixes #329) — ✅ approved, ready for human merge.

Diff cleanliness (gate 1): clean. Single commit 6bd2a64 whose parent is current master HEAD (85c6382, v1.17.0). Only the intended files change:

  • lib/config-validation.ts (+7/−1)
  • tests/config-validation.test.ts (+23)
  • devlog/2026-09-11_qualitygate-algo-config-keys/{REQ,WORKLOG}.md (new)

(The QQ-group README/devlog files that show up in a naive three-dot diff against an older base were already merged via #386 — a stale merge-base artifact, not part of this PR.)

Bug is real — reproduced. I ran the exact #329 repro config through both the pre-fix and post-fix key-walker:

Fix hits the root cause, not the symptom. qualityGate.algorithms is added to the recursion skip list at lib/config-validation.ts:88, same convention as compress.providers / messageFilters.filters / compress.modelMaxLimits|MinLimits. The top-level key stays in VALID_CONFIG_KEYS (line 71), so it's still validated at that depth; only the user-named algorithm subtree is delegated to the owning subsystem. Blast radius is exactly "fewer false warnings": getInvalidConfigKeys only feeds the non-blocking showConfigWarnings toast, and validateConfigTypes is untouched.

It's a load-bearing config path (not dead config). lib/compress/quality-gate/evaluate.ts:104 and :207 read (qg.algorithms && qg.algorithms[algoName]) ?? {}; the real param names live in context-compress-algorithms@1.3.0.

Tests: 2 new, both meaningful (import from source, non-tautological):

  • Positive — nested algo params → [] (no false warning).
  • Negative — unknown sibling qualityGate.notARealKey is still flagged while algo params pass. This is the important guard proving the skip didn't over-broaden to swallow genuinely unknown qualityGate.* keys.

Verification (ran locally on the PR tip): typecheck clean · config-validation 47/47 · full suite 1209/1209, 0 fail — matches the PR description exactly.

Backward compat: purely additive to validation logic; no persisted-state format, exported-API, or internal-tag changes. Safe.

Non-blocking notes:

  • Nit: the new tests use illustrative param names (minSummaryLength, rougeF1Threshold) rather than the real ones (layer1MinChars, …). Fine for exercising the allow-list (any nested key should pass); using the real names would just read more realistically. Optional.
  • Out of scope (already tracked in [Bug]: qualityGate.algorithms 下按算法名配置质量门参数会误报 "Unknown keys" 警告 #329, floor 5): schema/README show layer1MinRetentionPct default 1.0 which may differ from the runtime default now living in the external package. Correctly left out of this PR; worth a separate follow-up if you want them aligned.

No small fixes to apply directly — the diff is clean and correct as-is. Per policy I will not merge; please merge when ready.


中文摘要: 已验证该 bug 真实存在(用 #329 原始配置复现出被误报的 5 个键),修复把 qualityGate.algorithms 加入递归跳过名单(与 compress.providers/messageFilters.filters 同一约定),命中根因而非症状;新增的正/负两用例(尤其"未知兄弟键仍被拦截")设计到位,本地实测 typecheck 干净、全套 1209/1209 通过,diff 干净且向后兼容——可以合并。

…ests

Review follow-up on #389: the REQ.md config sample and both new tests used non-existent keys (minSummaryLength / rougeF1Threshold). rouge-recall-v1 actually consumes layer1MinChars / layer1MinRetentionPct / layer2MaxRougeF1 / layer2MaxTop20Recall (see dcp.schema.json); unknown inner keys are silently ignored at runtime, so the old sample would configure params that never take effect. No logic change — validation fix untouched. config-validation 47/47, full suite 1209/1209, typecheck clean.
@ranxianglei
ranxianglei merged commit 43e7d20 into master Sep 12, 2026
6 checks passed
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.

[Bug]: qualityGate.algorithms 下按算法名配置质量门参数会误报 "Unknown keys" 警告

1 participant