matrix: roleNormalize allowlist for non-plural-s tokens (scrum role_gate_v1)
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) <noreply@anthropic.com>
This commit is contained in:
parent
0331288641
commit
434f466288
@ -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).
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user