From 434f466288e74845b8d97cf092a39b5c3ab5682c Mon Sep 17 00:00:00 2001 From: root Date: Thu, 30 Apr 2026 22:58:02 -0500 Subject: [PATCH] matrix: roleNormalize allowlist for non-plural-s tokens (scrum role_gate_v1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3-lineage scrum review of the role-gate work (commits 7f2f112..0331288) ran Opus 4.7 / Kimi K2.6 / Qwen3-coder via scripts/scrum_review.sh. All three flagged the same edge case: the homegrown plural-stripper in roleNormalize would collapse non-plural-s tokens like "Sales" → "Sale", "Logistics" → "Logistic", "Operations" → "Operation". In a staffing domain those are real role names; the silent normalization would have caused false role-equality matches and re-opened the cross-role bleed for those clusters. Fix: - nonPluralSWords allowlist for known staffing-domain non-plural-s tokens (sales, logistics, operations, facilities, premises, news, physics, economics, mathematics, analytics). - Last-word-only stripping ("Sales Associate" stays whole; only "Associates" head noun is plural-checked). - -ss ending check so "Press Operator", "Boss" don't lose their s. - strings.ToLower + strings.TrimSpace replace the homegrown rune- loop ASCII normalizer (Opus INFO — minor cleanup, folded in). Tests: - TestRoleNormalize_NonPluralS: 18 cases covering the allowlist, -ss ending, real plurals (Operators → Operator, Boxes → Box), multi-word real plurals (Forklift Operators → forklift operator), whitespace/case tolerance. - TestRoleEqual_NonPluralS: gate-level pairing — proves equal- shape allowlisted tokens compare equal AND that "Sales" ≠ "Sale" (the original bug shape). - Existing TestRoleEqual_PluralAndCase still green (refactor preserved behavior). Other scrum findings dispositioned (not actioned): - Opus WARN on empty-role fail-open semantics: documented backward-compat behavior; production path closes via opt-in LLM extractor (real_004). - Opus INFO on unsynchronized package-global cache map: harness is single-goroutine; add sync.Mutex when/if it parallelizes. - Opus INFO on parallel constructor (NewPlaybookEntryWithRole vs optional arg): API smell only, both forms preserved. - Kimi 2 BLOCKs (NewPlaybookEntryWithRole missing, ApplyPlaybookBoost signature breakage): FALSE positives. Pre-push smoke chain green on 0331288, both symbols + all call sites compile clean. Matches feedback_cross_lineage_review.md's documented Kimi truncation behavior — Kimi BLOCKs warrant trace verification before action. Disposition (local): reports/scrum/_evidence/2026-04-30/verdicts/role_gate_v1_disposition.md. Co-Authored-By: Claude Opus 4.7 (1M context) --- STATE_OF_PLAY.md | 1 + internal/matrix/playbook.go | 91 +++++++++++++++++++++----------- internal/matrix/playbook_test.go | 74 ++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 32 deletions(-) diff --git a/STATE_OF_PLAY.md b/STATE_OF_PLAY.md index 189cdf7..b142fa2 100644 --- a/STATE_OF_PLAY.md +++ b/STATE_OF_PLAY.md @@ -265,6 +265,7 @@ The list is intentionally short. Items move to closed when the work demands them | (fix) | Cross-role gate: `Role` on `PlaybookEntry`, `QueryRole` on `SearchRequest`, gate fires in both `ApplyPlaybookBoost` + `InjectPlaybookMisses`. `roleEqual` handles case + plural. Backward-compat: empty role on either side = gate disabled (preserves lift suite + free-form callers). 5 new unit tests use exact real_001 distance + role values. Re-run real_002: bleed closed (Q#5 Pickers, Q#10 CNC Operator stay at cold-pass top-1; same-role lifts still fire). Closes OPEN #1. Findings: `reports/reality-tests/real_002_findings.md`. | | (probe) | Reality test real_003: 40 queries (10 fill_events rows × 4 styles — `need` / `client_first` / `looking` / `shorthand`). Confirmed shorthand-vs-shorthand bleed is real: CNC Operator shorthand recording leaked `w-2404` onto Forklift Operator shorthand query (both empty role, gate disabled). Extended `extractRoleFromNeed` to handle `client_first` + `looking` patterns; shorthand stays empty (regex can't separate role from city without anchor). Re-run real_003b: bleed closed across all 4 styles in this dataset. 10 new sub-tests in `scripts/playbook_lift/main_test.go` lock the patterns + the documented shorthand limitation. Findings: `reports/reality-tests/real_003_findings.md`. | | (fix) | LLM-based role extractor (real_004): `roleExtractor` struct with regex-first → qwen2.5 format=json fallback → per-process cache. Opt-in via `-llm-role-extract` flag + `LLM_ROLE_EXTRACT=1` env. Off-by-default preserves real_003b shipping config. 8 new tests including `TestRoleExtractor_ClosesCrossRoleShorthandBleed` — the load-bearing witness pairing with the matrix-side `TestInjectPlaybookMisses_RoleGateRejectsCrossRole` to prove the extraction-layer + gate-layer compose correctly on the exact real_003 failure mode. Findings: `reports/reality-tests/real_004_findings.md`. | +| (scrum) | 3-lineage scrum review on `7f2f112..0331288` (Opus + Kimi + Qwen3-coder via `scripts/scrum_review.sh`). Convergent finding (3/3): `roleNormalize` plural-stripper mangled non-plural-s tokens (Sales → Sale, Logistics → Logistic). **Fixed**: `nonPluralSWords` allowlist + `-ss` ending check + `strings.ToLower`/`TrimSpace` cleanup. New tests `TestRoleNormalize_NonPluralS` + `TestRoleEqual_NonPluralS` lock the edge cases. Kimi 2 BLOCKs were false positives (model-truncation artifacts per `feedback_cross_lineage_review.md`). Disposition: `reports/scrum/_evidence/2026-04-30/verdicts/role_gate_v1_disposition.md` (local). | Plus on Rust side (`8de94eb`, `3d06868`): qwen2.5 → qwen3.5:latest backport in active defaults; distillation acceptance reports regenerated (run_hash refresh, reproducibility property still holds). diff --git a/internal/matrix/playbook.go b/internal/matrix/playbook.go index 14432f9..5a9f6d9 100644 --- a/internal/matrix/playbook.go +++ b/internal/matrix/playbook.go @@ -30,6 +30,7 @@ import ( "encoding/json" "errors" "sort" + "strings" "time" ) @@ -219,42 +220,68 @@ func roleEqual(a, b string) bool { return roleNormalize(a) == roleNormalize(b) } -// roleNormalize lowercases + trims + strips a single trailing "s" -// (or "es" after "x"/"s"/"ch"/"sh") to collapse common staffing -// plurals without pulling in a real stemmer. +// nonPluralSWords is the allowlist of staffing-domain tokens that end +// in 's' but are NOT plurals — naïve trailing-'s' stripping would +// mangle these into wrong tokens ("Sales" → "Sale", "Logistics" → +// "Logistic"). Per scrum review role_gate_v1 (Opus + Kimi + Qwen +// convergent finding 2026-04-30): expand this list as new false- +// positives surface from production traffic. +var nonPluralSWords = map[string]bool{ + "sales": true, + "logistics": true, + "operations": true, + "facilities": true, + "premises": true, + "news": true, + "physics": true, + "economics": true, + "mathematics":true, + "analytics": true, +} + +// roleNormalize lowercases + trims + best-effort-singularizes the +// last word so "Forklift Operators" matches "Forklift Operator" but +// "Sales Associate" doesn't lose its "s". +// +// Only the LAST word is plural-checked because staffing-domain role +// names follow English head-noun-last conventions ("Sales Associate" +// → head is "Associate"; "Forklift Operators" → head is "Operators"). +// +// Singularization rules (in order): +// 1. Last word ∈ nonPluralSWords → return as-is (e.g. "Logistics +// Coordinator" stays "logistics coordinator"). +// 2. Last word ends in "ss" → return as-is ("Press Operator" stays; +// "Boss" stays). +// 3. Last word ends in "es" after "x"/"s"/"ch"/"sh" → strip "es" +// ("Boxes" → "Box", "Dishes" → "Dish"). +// 4. Last word ends in "s" → strip "s" ("Operators" → "Operator"). +// 5. Otherwise return as-is. func roleNormalize(s string) string { - out := []rune{} - // in-place lowercase + skip leading/trailing whitespace - for _, r := range s { - if r == ' ' || r == '\t' { - if len(out) == 0 { - continue - } - } - if r >= 'A' && r <= 'Z' { - r += 32 - } - out = append(out, r) - } - // trim trailing whitespace - for len(out) > 0 && (out[len(out)-1] == ' ' || out[len(out)-1] == '\t') { - out = out[:len(out)-1] - } - if len(out) == 0 { + s = strings.ToLower(strings.TrimSpace(s)) + if s == "" { return "" } - // crude singularization for staffing plurals - n := len(out) - if n >= 3 && out[n-1] == 's' && out[n-2] == 'e' && - (out[n-3] == 'x' || out[n-3] == 's' || out[n-3] == 'h') { - // "boxes", "kisses", "dishes" → strip "es" - out = out[:n-2] - } else if n >= 2 && out[n-1] == 's' { - // "operators" → "operator"; harmless on words ending in s if - // callers mind their canonicalization (rare in role names). - out = out[:n-1] + // Find the last word. + lastSpace := strings.LastIndex(s, " ") + prefix, last := "", s + if lastSpace >= 0 { + prefix, last = s[:lastSpace+1], s[lastSpace+1:] } - return string(out) + if nonPluralSWords[last] { + return s + } + if strings.HasSuffix(last, "ss") { + return s + } + n := len(last) + if n >= 3 && last[n-1] == 's' && last[n-2] == 'e' && + (last[n-3] == 'x' || last[n-3] == 's' || last[n-3] == 'h') { + return prefix + last[:n-2] + } + if strings.HasSuffix(last, "s") { + return prefix + last[:n-1] + } + return s } // InjectPlaybookMisses appends synthetic Results for playbook hits diff --git a/internal/matrix/playbook_test.go b/internal/matrix/playbook_test.go index 0f5030e..6eb4bee 100644 --- a/internal/matrix/playbook_test.go +++ b/internal/matrix/playbook_test.go @@ -556,6 +556,80 @@ func TestRoleEqual_PluralAndCase(t *testing.T) { } } +// TestRoleNormalize_NonPluralS locks the convergent finding from the +// role_gate_v1 scrum (Opus + Kimi + Qwen): words ending in 's' that +// aren't plurals must NOT be stripped. Bug shape per Opus' write-up: +// the original implementation collapsed "Sales" to "Sale", "Logistics" +// to "Logistic", etc., which would silently fail role-equality on +// every staffing client whose roles include those tokens. +func TestRoleNormalize_NonPluralS(t *testing.T) { + cases := []struct { + in, want string + }{ + // Allowlist: end in 's' but represent role names as-is. + {"Sales", "sales"}, + {"Sales Associate", "sales associate"}, + {"Sales Associates", "sales associate"}, // plural-stripped on the head noun only + {"Logistics", "logistics"}, + {"Logistics Coordinator", "logistics coordinator"}, + {"Operations Manager", "operations manager"}, + {"Facilities", "facilities"}, + // -ss (not plural): must not strip. + {"Press", "press"}, + {"Press Operator", "press operator"}, + {"Boss", "boss"}, + // Real plurals still strip correctly. + {"Operators", "operator"}, + {"Pickers", "picker"}, + {"Boxes", "box"}, + {"Dishes", "dish"}, + // Multi-word real plurals: strip head only. + {"Forklift Operators", "forklift operator"}, + {"Production Workers", "production worker"}, + // Whitespace + case tolerance. + {" CNC Operator ", "cnc operator"}, + {"FORKLIFT OPERATOR", "forklift operator"}, + // Empty. + {"", ""}, + } + for _, c := range cases { + got := roleNormalize(c.in) + if got != c.want { + t.Errorf("roleNormalize(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +// TestRoleEqual_NonPluralS is the gate-level pairing for +// TestRoleNormalize_NonPluralS — proves that two equal-shape role +// strings compare equal AND two different-role strings (one plural, +// one allowlisted) compare different. +func TestRoleEqual_NonPluralS(t *testing.T) { + cases := []struct { + a, b string + want bool + }{ + // Same role, allowlisted token: must stay equal. + {"Sales", "sales", true}, + {"Logistics Coordinator", "logistics coordinator", true}, + {"Press Operator", "PRESS OPERATOR", true}, + // Allowlisted vs naïve-singular (the original bug): Sales + // would have falsely matched "Sale" if normalize stripped 's'. + // "Sale" isn't in the allowlist + isn't a plural-of-something, + // so they should NOT match. + {"Sales", "Sale", false}, + // Plural ↔ singular cross-comparison still works for legit plurals. + {"Sales Associates", "Sales Associate", true}, + {"Forklift Operators", "Forklift Operator", true}, + } + for _, c := range cases { + got := roleEqual(c.a, c.b) + if got != c.want { + t.Errorf("roleEqual(%q, %q) = %v, want %v", c.a, c.b, got, c.want) + } + } +} + func abs(f float64) float64 { if f < 0 { return -f