From 9d09fe4c5ffb699055a2c5ae489fce772af2b927 Mon Sep 17 00:00:00 2001 From: klappy Date: Mon, 20 Apr 2026 04:37:38 +0000 Subject: [PATCH] fix(challenge): restore strictly-additive invariant + DRY collapse (0.21.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes from Cursor Bugbot findings on PR #120 / #121 that should have blocked the 0.21.0 ship and didn't because the orchestrator treated Bugbot's in_progress state as non-blocking. The findings are real and the medium-severity one is a prod regression vs pre-refactor behavior. Bug #1 (medium) — Stop-word filtering silently drops canon vocab keywords: - parseCheckColumn called tokenize(m[1]) with default STOP_WORDS filter. Canon vocab includes 'from' (in source-named) and 'to' (inside 'according to'); STOP_WORDS includes both. Result: 'from' is dropped from stemmedTokens; the runtime call site applied the same filter to inputStems so 'from' is dropped there too. - Pre-refactor regex evaluator matched 'from' literally as new RegExp('\\bfrom\\b', 'i') against raw input. Inputs like 'I learned this from my colleague' passed source-named pre-refactor and fail post-refactor — the strictly-additive invariant the 0.21.0 CHANGELOG and PR description claimed is broken. - Latent landmine: any prereq whose canon vocab is entirely stop-words would have stemmedTokens.size === 0 and trigger the conservative length>=20 fallback inappropriately (false positive). Not currently exploited by canon but the structural risk is there. - Fix: pass new Set() (empty stop-words) to both tokenize() calls so canon keywords survive on both sides and shape matches. Bug #2 (low) — DRY violation in BasePrerequisite: - 0.21.0 introduced PrereqMatchVocab interface to share shape between BasePrerequisite and ChallengeTypeDef.prerequisiteOverlays[]. The inline type on prerequisiteOverlays uses '& PrereqMatchVocab' intersection; BasePrerequisite re-listed all five fields manually. Future PrereqMatchVocab additions would not propagate to BasePrerequisite — defeats the DRY purpose. - Fix: split into BasePrerequisiteCore (3 core fields) and type BasePrerequisite = BasePrerequisiteCore & PrereqMatchVocab. Verification: - Typecheck clean - governance-parser.test.mjs 105/105 pass - 2 new regression assertions in canon-tool-envelope.smoke.mjs: (10) source-named via 'from'-only canon keyword (11) source-named via 'according to' multi-word phrase - Bugbot Autofix preview diff for PR #121 used as the basis for the Bug #1 fix (cherry-picked the change locally rather than applying via Autofix UI to keep the audit trail in this branch). Process post-mortem to follow in P1.3.3 closeout ledger. --- CHANGELOG.md | 12 +++++++ package.json | 2 +- workers/package-lock.json | 4 +-- workers/package.json | 2 +- workers/src/orchestrate.ts | 40 +++++++++++++--------- workers/test/canon-tool-envelope.smoke.mjs | 30 ++++++++++++++++ 6 files changed, 70 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35f1b26..282a667 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.21.1] - 2026-04-20 + +### Fixed + +- **Strictly-additive invariant restored in `oddkit_challenge` prereq evaluator** (Cursor Bugbot finding on PR #120 / #121, medium severity). The 0.21.0 implementation called `tokenize(input)` and `tokenize(m[1])` with the default `STOP_WORDS` filter on both the input side and the parse-time vocabulary side. Canon vocab keywords that are also English stop words — notably `from` in source-named's vocabulary — were silently dropped from both `inputStems` and `stemmedTokens`. Inputs like `"I learned this from my colleague"` passed `source-named` pre-refactor (via `\bfrom\b` literal regex match) but failed post-refactor, breaking the strictly-additive invariant claimed in the 0.21.0 CHANGELOG and PR description. Fix: pass `new Set()` (empty stop-words) to both `tokenize()` calls so canon vocab survives and both sides share shape. New regression assertions in canon-tool-envelope.smoke.mjs anchor the fix at item (10) `from`-only source attribution and item (11) the `according to` multi-word case. + +- **`BasePrerequisite` collapsed to `BasePrerequisiteCore & PrereqMatchVocab` intersection** (Cursor Bugbot finding on PR #120, low severity). The 0.21.0 implementation defined `interface PrereqMatchVocab` to share shape between `BasePrerequisite` and `ChallengeTypeDef.prerequisiteOverlays[]` (DRY) but then re-listed all five fields in `BasePrerequisite` instead of using `& PrereqMatchVocab`. Fix: split into `interface BasePrerequisiteCore` (the three core fields) and `type BasePrerequisite = BasePrerequisiteCore & PrereqMatchVocab` (the intersection). Future field additions to `PrereqMatchVocab` now propagate automatically. + +### Process + +- This release exists because the 0.21.0 ship process skipped Bugbot's findings (treating in_progress as non-blocking) and skipped Sonnet 4.6 validator dispatch despite the P1.3.2 ledger explicitly warning against making smoke-only the default. Both findings landed in prod for ~15 minutes before recovery. Documented in P1.3.3 closeout ledger as a process failure to carry forward. + ## [0.21.0] - 2026-04-20 ### Changed diff --git a/package.json b/package.json index ad7066c..d4da3ba 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "oddkit", - "version": "0.21.0", + "version": "0.21.1", "description": "Agent-first CLI for ODD-governed repos. Epistemic terrain rendering with portable baseline.", "type": "module", "bin": { diff --git a/workers/package-lock.json b/workers/package-lock.json index 1afedd1..e0021e7 100644 --- a/workers/package-lock.json +++ b/workers/package-lock.json @@ -1,12 +1,12 @@ { "name": "oddkit-mcp-worker", - "version": "0.21.0", + "version": "0.21.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "oddkit-mcp-worker", - "version": "0.21.0", + "version": "0.21.1", "dependencies": { "agents": "^0.4.1", "fflate": "^0.8.2", diff --git a/workers/package.json b/workers/package.json index ba97cbc..400c8e5 100644 --- a/workers/package.json +++ b/workers/package.json @@ -1,6 +1,6 @@ { "name": "oddkit-mcp-worker", - "version": "0.21.0", + "version": "0.21.1", "private": true, "type": "module", "scripts": { diff --git a/workers/src/orchestrate.ts b/workers/src/orchestrate.ts index 664e177..96c55b5 100644 --- a/workers/src/orchestrate.ts +++ b/workers/src/orchestrate.ts @@ -102,22 +102,19 @@ interface ChallengeTypeDef { fallback: boolean; } -interface BasePrerequisite { +interface BasePrerequisiteCore { prerequisite: string; check: string; gapMessage: string; - // Per PRD D2 (P1.3.3): parse products populated at canon-fetch time. - // stemmedTokens is the stemmed form of quoted keywords in `check`; - // the four has*Check booleans flag structural-test hints detected in - // the check description. See parseCheckColumn below. These are parse - // products per klappy://canon/principles/cache-fetches-and-parses. - stemmedTokens: Set; - hasURLCheck: boolean; - hasNumericCheck: boolean; - hasProperNounCheck: boolean; - hasCitationCheck: boolean; } +// BasePrerequisite shares the PrereqMatchVocab shape (stemmedTokens + 4 +// structural-test flags) with ChallengeTypeDef.prerequisiteOverlays[] via +// intersection — defined as `& PrereqMatchVocab` rather than re-listing the +// fields, so future field additions to the shared shape propagate +// automatically. Per Bugbot finding on PR #120 (low severity). +type BasePrerequisite = BasePrerequisiteCore & PrereqMatchVocab; + /** Shared shape for the runtime match vocabulary attached to challenge * prereqs. Keeps the per-type and base-prereq structs in sync (DRY). */ interface PrereqMatchVocab { @@ -2145,7 +2142,11 @@ async function runChallengeAction( // the loop, stemmedTokens differ per prereq. Per PRD D3 (P1.3.3): stemmed // set intersection at runtime, structural tests preserved, no regex compile // per check. This is the fit-to-problem matcher per D5. - const inputStems = new Set(tokenize(input)); + // Stop-word filtering is disabled (empty Set) so this matches the parse-time + // tokenize() call in parseCheckColumn. Canon vocab includes stop-words like + // `from` (source-named) — both sides must share shape or strictly-additive + // breaks. Per Bugbot finding on PR #120 / #121. + const inputStems = new Set(tokenize(input, new Set())); const missing: string[] = []; for (const p of prereqMap.values()) { const passed = evaluatePrerequisiteCheck(inputStems, input, p); @@ -2323,10 +2324,17 @@ function parseCheckColumn(check: string): PrereqMatchVocab { let m: RegExpExecArray | null; while ((m = quotedRegex.exec(check)) !== null) { // Tokenize each quoted keyword or phrase — multi-word phrases like - // "according to" contribute multiple stems; stop-words are dropped - // by tokenize(). This preserves semantic coverage while normalizing - // morphology (problems → problem, considered → consid, etc.). - for (const stem of tokenize(m[1])) { + // "according to" contribute multiple stems. Stop-word filtering is + // disabled (empty Set) because canon vocab includes stop-word + // keywords — `from` in source-named, `to` in `according to`, etc. + // The pre-refactor regex evaluator matched these literally as + // `\bfrom\b` against raw input; dropping them here would silently + // break the strictly-additive invariant. The runtime call site uses + // the same empty stop-word set on inputStems so both sides share + // shape. Stemming still applies (problems → problem, considered → + // consid). Per Bugbot finding on PR #120 (medium severity) and + // PR #121 (carried forward). + for (const stem of tokenize(m[1], new Set())) { stemmedTokens.add(stem); } } diff --git a/workers/test/canon-tool-envelope.smoke.mjs b/workers/test/canon-tool-envelope.smoke.mjs index 1a2fcf7..a73eb3d 100644 --- a/workers/test/canon-tool-envelope.smoke.mjs +++ b/workers/test/canon-tool-envelope.smoke.mjs @@ -454,6 +454,36 @@ async function run() { `missing: ${JSON.stringify(confMissing)}`, ); + // (10) 0.21.1 regression — stop-word canon keywords must survive parse-time + // tokenization. Bugbot caught this on 0.21.0: `from` is in source-named's + // canon vocab AND in the default STOP_WORDS set, so the default-filtered + // tokenize() silently dropped it from both stemmedTokens and inputStems, + // breaking the strictly-additive invariant. Fix: pass empty Set as + // stopWords arg to both tokenize() calls. This assertion is the regression + // anchor. + const fromOnlySource = + "I learned this morning that the deploy regressed from my colleague Alex Brown — observed during last night's incident review."; + const fromMissing = await challengeMissing(fromOnlySource); + ok( + `oddkit_challenge: 0.21.1 source-named passes when input matches via stop-word canon keyword "from"`, + !includesGap(fromMissing, "no source named"), + `missing: ${JSON.stringify(fromMissing)}`, + ); + + // (11) 0.21.1 regression — verify "according to" (which contains stop-word + // "to") still passes source-named via the surviving "accord" stem. This is + // the multi-word phrase case where stop-word filtering would have dropped + // half the phrase. Pre-fix this still worked (because "accord" survives + // independently), but the assertion documents the intended behavior. + const accordingToText = + "We saw a 30% latency regression according to the Tuesday measurements I observed in the dashboard."; + const accMissing = await challengeMissing(accordingToText); + ok( + `oddkit_challenge: 0.21.1 source-named passes via stemmed "accord" from "according to"`, + !includesGap(accMissing, "no source named"), + `missing: ${JSON.stringify(accMissing)}`, + ); + // Tool 6: oddkit_gate — canon-driven, two governance surfaces. Full envelope + // governance_source + governance_uris (plural array of 2 — shape diverges // from encode's singular governance_uri, matches challenge's plural shape,