Ahsan_Abbasi_Week 8: Triage improvements — critical recall 0/16 → 16/16 - #15
Open
1abbasia wants to merge 8 commits into
Open
Ahsan_Abbasi_Week 8: Triage improvements — critical recall 0/16 → 16/16#151abbasia wants to merge 8 commits into
1abbasia wants to merge 8 commits into
Conversation
Adds src/evals/goldenSet.js (32 hand-labelled customer messages spanning terse-but-critical outages, politely-worded emergencies, churn/legal threats, ordinary defects, feature requests, pure praise, and adversarial cases where tone and true priority disagree) and src/evals/runEval.js, which runs the golden set through the current calculateUrgency scorer and reports overall accuracy, a confusion matrix, critical recall (share of true-High messages caught), and every individual failure. Exits non-zero below threshold (75% accuracy, 90% critical recall) so it can gate CI. Also adds an `npm run eval` script. No fixes yet - this only adds the measurement tooling.
Ran `npm run eval` against the unmodified calculateUrgency() in src/utils/urgencyScorer.js and recorded the result in src/evals/BASELINE.md so future changes can be measured against a fixed snapshot. Baseline results (32-message golden set): - Overall accuracy: 12/32 (37.5%) -- gate threshold 75%, FAIL - Critical recall (true-High caught as High): 0/16 (0.0%) -- gate threshold 90%, FAIL Every true-High message in the golden set (terse outages, polite emergencies, churn/legal threats, and the High-labelled adversarial cases) was scored Medium or Low -- zero were caught. The two false-High results were both trivial complaints written in all caps with exclamation marks. Confusion matrix and full failure list are in src/evals/BASELINE.md. Confirms the harness genuinely fails against the current, unmodified scorer (exit code 1). No production code changed in this commit.
…signals
calculateUrgency() previously read new Date() directly and scored surface
style (exclamation count, ALL CAPS, message length, politeness words,
question marks) instead of content. It was not a pure function - the same
message could return a different priority depending on the day of week or
hour it happened to be analyzed - which made the queue unauditable and
made the eval harness itself non-deterministic between runs. It also
caught zero real emergencies: baseline critical recall was 0/16.
Rewrites the scorer around 13 weighted business-risk categories (security
exposure, data loss, outage, production impact, blocked users, legal
threat, churn, payment failure, blast radius, repeat contact, plain
defect, degraded performance, explicit urgency). Each category is matched
against the message text only; there is no Date, randomness, or other I/O,
so the function is now pure - same input, same output, always.
Design notes:
- security, dataLoss, outage, and legalThreat are weighted at the High
threshold on their own - a confirmed breach, data loss event, outage, or
legal threat is P1 by itself and shouldn't need a corroborating signal.
Every other category sits below threshold and only reaches High in
combination with something else.
- Explicit urgency language ("urgent", "ASAP") firing as the *only*
signal is discounted to zero - a bare urgency claim with no described
problem is easy to game and isn't trusted on its own.
- Tone (caps, exclamation marks) is capped at a small additive bonus and
can never be a primary driver, per the requirement.
- Praise/gratitude language is capped to a low score, but only when no
problem signal fired - "thank you for your help, but production is
down" is scored as an outage, not gratitude, because the praise cap
explicitly requires the absence of any problem signal.
- scoreUrgency(message) now returns { level, score, signals }, where
signals lists which phrase matched each category and its weight, so a
support lead can see why a ticket is P1. calculateUrgency(message) is
kept as a thin wrapper returning just the level, so existing callers
(AnalyzePage, templates) don't need to change.
Also updates runEval.js to print the signals breakdown for any failing
case, since that's what's needed to diagnose a miscategorization instead
of guessing.
Eval after this fix: 32/32 accuracy (100%), 16/16 critical recall (100%),
up from the 12/32 (37.5%) / 0/16 (0%) baseline. Two intermediate failures
were fixed by addressing root causes, not by tuning to the test: (1) smart
quotes from pasted email/Word text weren't matching apostrophe-sensitive
patterns like "doesn't work", fixed by normalizing curly quotes before
matching; (2) a lone outage or security report scored just under the High
threshold because those categories were originally weighted below it,
fixed by weighting the four standalone-sufficient categories at the
threshold itself.
…ogic
templates.js previously mapped a category to one hardcoded advice string
regardless of urgency - getRecommendedAction(category, urgency) accepted
urgency but never read it. Feature Request incorrectly reused the Billing
Issue string ("Ask user to check billing portal."), so a dark-mode request
got billing-portal advice. shouldEscalate() ignored category and urgency
entirely and returned message.length > 100, so a two-word outage report
was never escalated while a long polite thank-you note was.
Worse: shouldEscalate and getAvailableCategories were exported but never
imported anywhere in the app, so escalation logic has never actually run
in this product - fixing the function alone would just have produced
better dead code.
Changes:
- getRecommendedAction(category, urgency) now routes on the (category x
urgency) pair to a named owning team, a P1-P4 priority, and an SLA,
via a routing table. Feature Request now correctly routes to Product
Management instead of the billing portal.
- shouldEscalate({ category, urgency, message | urgencySignals,
confidence, degraded }) now checks real signals - urgency scored High,
a security-exposure signal, a legal threat, churn/cancellation
language, low classifier confidence (<50%), or degraded AI mode - and
returns { escalate, reasons } so the decision is auditable rather than
a black box. confidence/degraded default to trusting values (1/false)
since llmHelper doesn't supply them yet as of this commit; that lands
in the next commit.
- AnalyzePage.jsx now actually calls shouldEscalate as part of the
analysis flow and stores the result on the analysis object, and
renders it as an "Escalation" panel with the specific reasons when
triggered. getRecommendedAction's object result (team/priority/sla/
action) is rendered in place of the old single-string advice, and the
copy-to-clipboard text was updated to match.
- Also fixes a regex bug found while testing shouldEscalate's churn-risk
reason: urgencyScorer.js's churn pattern was cancel(l)?ing?, which only
makes the trailing "g" optional, not "ing" as a whole, so it required
a literal "in" after "cancel" and silently failed to match plain-tense
phrasing like "cancel our contract" (only gerunds like "cancelling"
happened to work). Replaced with cancel\w* (my|our) (account|
subscription|contract).
Eval: unchanged at 32/32 accuracy, 16/16 critical recall - this fix
doesn't touch the scorer, as expected.
categorizeMessage() previously sent an unstructured prompt ("Categorize
this customer support message: ...") at temperature 0.7 and parsed the
category by substring-matching the model's free-text response in a fixed
if/else order with no negation handling - "this is not a billing issue,
it's technical" matched "billing" first and mis-filed. On any API failure
it silently returned a keyword-matched mock, rendered identically to a
real AI response with no error surfaced anywhere - a trust failure for a
product sold on AI triage, and one that gets more likely exactly as
customer volume grows (rate limits, transient errors).
Rewrite:
- System prompt states the fixed category taxonomy explicitly and asks
for a single JSON object (category, confidence, reasoning).
- response_format: { type: 'json_object' }, temperature: 0 - deterministic,
structured output instead of free text and substring matching.
- The parsed response is schema-validated: category must be one of the
five taxonomy values, confidence must be a number in [0, 1], reasoning
must be a non-empty string. Any violation throws NonRetryableError -
retrying the same prompt against the same model wouldn't fix a bad
response, so these fail fast into the fallback instead of wasting
retries.
- 8s timeout per attempt, up to 3 attempts total, with a short backoff -
but only for transient failures (network errors, our own timeout, or
a 408/429/500/502/503/504 status). Schema/validation failures and Groq
client construction failures are never retried.
- The fallback (unchanged keyword heuristics) now returns source:
'fallback' and degraded: true, confidence capped at 0.2 (below
shouldEscalate's 0.5 threshold from the previous commit, so a degraded
result always routes to a human), and states in its own reasoning text
that it's a fallback guess and should be verified by a person.
- AnalyzePage.jsx now reads confidence/source/degraded from
categorizeMessage and passes confidence/degraded into shouldEscalate
(which accepted but couldn't use them until now, since llmHelper
didn't supply them). It shows an explicit amber "AI categorization is
unavailable" banner, a confidence percentage next to the category, and
relabels the reasoning panel "Fallback Reasoning (AI unavailable)"
instead of "AI Reasoning" when degraded - so a fallback result can no
longer render identically to a real one. That's the propagation the
page reacts to; categorizeMessage still resolves rather than rejects,
because a triage tool failing a message analysis outright (the old
outer try/catch's only option) is a worse outcome than a clearly
labeled, auto-escalated fallback.
Found while testing this change: the Groq client was constructed at
module load time, and groq-sdk throws synchronously if the API key is
missing - meaning a missing key crashed the whole module on import,
before any try/catch could run, defeating graceful degradation entirely
(confirmed: this repo currently has no VITE_GROQ_API_KEY set and running
it against the old code would never have reached the fallback path at
all). Fixed by constructing the client lazily on first use, inside the
retry path, so a missing/invalid key is just another NonRetryableError
that degrades gracefully like any other configuration problem.
Verified directly (not just via the eval, which only covers the urgency
scorer and is unaffected by this file):
- With no API key configured: categorizeMessage resolves in ~1ms with
source: 'fallback', degraded: true, confidence: 0.2, and shouldEscalate
returns escalate: true with reasons for both low confidence and
degraded mode, in addition to any urgency/security/churn reasons.
- With a real key: three varied test messages (feature request, billing,
outage) all returned clean structured JSON - correct category each
time, confidence 0.9-0.99, source: 'llm', degraded: false, ~400ms
latency.
Eval: unchanged at 32/32 accuracy, 16/16 critical recall - this fix
doesn't touch the scorer, as expected.
…n reasons Most of this was already wired up in the routing/LLM fixes: urgency was already passed to getRecommendedAction, the routing decision (team/ priority/SLA) and escalation reasons were already rendered, and the degraded-mode banner already showed when result.degraded is true. Two gaps remained: - The urgency score and the signals that produced it were computed (scoreUrgency returns them, and AnalyzePage already stored them on the result object) but never displayed - a triage decision was still a bare "High"/"Medium"/"Low" label with no way to see why. Now shows the numeric score next to the badge and a "Why:" breakdown listing each signal's label, the matched phrase, and its weight. - Classifier confidence was shown but not visually flagged - a 45% confidence categorization looked identical to a 95% one. Below 60% it now renders in amber with a warning icon. This is a separate, more sensitive threshold than shouldEscalate's 50% auto-escalation trigger - the UI should flag "pay attention" earlier than the system decides to force a human handoff.
Documents the triage engine defects found, their business impact against the product's "handle more volume without hiring" claim, and the fixes applied across the urgency/routing/LLM commits, with before/after eval numbers (37.5% -> 100% accuracy, 0/16 -> 16/16 critical recall). Includes a section on what the eval harness caught in my own rewrites (smart-quote normalization, the ing? regex bug that broke plain-tense churn phrasing, and the Groq client being constructed at module load so it threw synchronously on a missing key) as evidence the harness earns its keep beyond the original bug list, plus an honest limitations section covering golden-set overfitting risk, the rules-based engine's blind spots, untested category accuracy, the still-browser-exposed API key, and discarded agent corrections.
Adds an "Evaluating the Triage Engine" section: how to run npm run eval, what accuracy/confusion-matrix/critical-recall mean, and why critical recall is reported as its own headline metric rather than folded into overall accuracy - a scorer can hit high accuracy while still missing every real emergency, since easy low-stakes cases can outnumber and average away the failures that actually matter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The original triage engine caught 0 of 16 genuinely urgent messages in a
32-case labelled test set. It scored terse outages and polite emergencies as
Low/Medium, while the only two messages it rated High were trivial all-caps
complaints. This PR rewrites the classification, urgency, and routing layers and
adds an evaluation harness so quality is measurable rather than assumed.
What changed
business-risk signals. Now a pure function; the same message always returns the
same priority.
SLA.
shouldEscalatewas exported but never imported anywhere — escalation hadnever run in this app. It's now fixed and wired in.
confidence scoring, timeout and bounded retry. The silent keyword fallback now
identifies itself as degraded and auto-escalates to a human.
matrix, non-zero exit for CI.
Why this order
Urgency drives queue order, and queue order is the product. A support lead who
doesn't trust the priority label re-reads the whole queue — which inverts the
core value claim of handling more volume without hiring.
Evidence the harness earned its place
It caught three bugs in my own rewrites: smart-quote normalization, an
ing?regex that broke plain-tense churn phrasing, and the Groq client being
constructed at module load — which threw synchronously on a missing key and would
have bypassed the fallback entirely.
Known limitations are documented in
IMPROVEMENTS.md, including that 100% on aset I tuned against overstates real-world accuracy.
Submitted by Ahsan Abbasi