mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix(ui): L-shell step 5e.1b — unify risk-level handling onto conversation.js
Both panes carried their own risk-level logic that disagreed on the fallback: interactive's normalizeRiskLevel sent an unknown level to "medium" (and, lacking the crit/med aliases, rendered a "crit" verdict as medium), while the coordinator's _riskRank sent unknown to "high". Lift one canonical normalize + rank into conversation.js and route both panes through it. - conversation.js: normalizeRiskLevel (aliases crit->critical / med->medium; unknown -> "medium"), riskRank, maxSeverityItem (keeps the no-verdict -> -1 edge so an unassessed item never wins the max-severity pick). - interactive.js: import normalizeRiskLevel, drop the local copy (its 3 callers unchanged); a "crit" verdict now renders critical instead of medium. - coordinator.js: import maxSeverityItem, drop RISK_SEVERITY / _riskRank / _maxSeverityItem; an unknown-level item now ranks medium, not high. - unknown -> medium is the deliberate fallback (per decision), not "high": medium is the neutral default both panes' displays already used. - tests: conversation guards for the fallback + aliases + the no-verdict edge; the two pane guards now check the shared module. The per-site risk->CSS-class display mappings (coordinator's inline chips and renderApprovalBlock's deliberate unknown->crit over-alert pill) are left for the 5e.2 vocabulary reconcile.
This commit is contained in:
@@ -1567,16 +1567,19 @@ def test_risk_level_normalized_before_dom_interpolation() -> None:
|
||||
straight into the class string and silently break selector targeting —
|
||||
guard that the chokepoint exists and no site skips it."""
|
||||
body = _INTERACTIVE_JS.read_text(encoding="utf-8")
|
||||
assert "function normalizeRiskLevel(" in body
|
||||
assert "VALID_RISK_LEVELS" in body
|
||||
for level in ("low", "medium", "high", "critical"):
|
||||
assert f'"{level}"' in body
|
||||
# The canonical normalizer + its enum moved to the shared conversation.js
|
||||
# (step 5e.1b); the pane imports it and routes all three sites through it.
|
||||
assert "normalizeRiskLevel," in body and 'from "./conversation.js"' in body
|
||||
# The raw fallback antipattern must be gone from every interpolation site.
|
||||
assert 'risk_level || "medium"' not in body
|
||||
assert 'risk_level) || "medium"' not in body
|
||||
# The three known sites (updateVerdictBadge / _buildOutputWarningEl /
|
||||
# The three sites (updateVerdictBadge / _buildOutputWarningEl /
|
||||
# renderVerdictBadge) all route through the chokepoint.
|
||||
assert body.count("normalizeRiskLevel(") >= 4 # 1 def + 3 call sites
|
||||
assert body.count("normalizeRiskLevel(") >= 3 # 3 call sites (def is shared)
|
||||
shared = (_INTERACTIVE_JS.parent / "conversation.js").read_text(encoding="utf-8")
|
||||
assert "export function normalizeRiskLevel(" in shared
|
||||
for level in ("low", "medium", "high", "critical"):
|
||||
assert f'"{level}"' in shared
|
||||
|
||||
|
||||
def test_announced_rail_outspecifies_inline_cyan_hold() -> None:
|
||||
|
||||
@@ -65,3 +65,24 @@ def test_no_inner_html() -> None:
|
||||
"""House style: programmatic DOM only — no innerHTML *usage* in the shared
|
||||
module (the header comment names it; guard the access pattern)."""
|
||||
assert ".innerHTML" not in _body()
|
||||
|
||||
|
||||
def test_normalize_risk_level_unknown_to_medium() -> None:
|
||||
"""Unified canonical fallback (step 5e.1b): an unknown / unrecognized risk
|
||||
normalizes to "medium" (the user's decision; the coordinator's old rank used
|
||||
"high"). The crit/med abbreviations alias to critical/medium so a 'crit'
|
||||
verdict no longer renders as medium (the latent interactive bug)."""
|
||||
body = _body()
|
||||
assert 'return RISK_LEVELS.indexOf(s) >= 0 ? s : "medium";' in body
|
||||
assert 'crit: "critical"' in body and 'med: "medium"' in body
|
||||
|
||||
|
||||
def test_risk_rank_and_max_severity_exported() -> None:
|
||||
"""riskRank + maxSeverityItem (lifted from the coordinator's _riskRank /
|
||||
_maxSeverityItem) are exported and build on the canonical normalize, so the
|
||||
rank and the display can't disagree on the fallback. An item with no verdict
|
||||
ranks below low so it never wins the max-severity pick."""
|
||||
body = _body()
|
||||
assert "export function riskRank(" in body
|
||||
assert "export function maxSeverityItem(" in body
|
||||
assert "? riskRank(v.risk_level) : -1;" in body
|
||||
|
||||
@@ -73,7 +73,7 @@ def test_coordinator_js_exposes_inline_approval_helpers():
|
||||
body = coord_js.read_text(encoding="utf-8")
|
||||
# Approval-block rendering helpers
|
||||
assert "function renderApprovalBlock" in body
|
||||
assert "function _maxSeverityItem" in body
|
||||
assert "maxSeverityItem," in body # imported from conversation.js (5e.1b)
|
||||
assert "function _renderSubItem" in body
|
||||
# The submit + 409 race-handling path
|
||||
assert "function submitChildApproval" in body or "submitChildApproval(" in body
|
||||
@@ -103,11 +103,14 @@ def test_coordinator_js_exposes_inline_approval_helpers():
|
||||
# regress to a buttoned approve UI on the wrong state.
|
||||
assert "POLICY-BLOCKED" in body
|
||||
assert "judge unavailable" in body
|
||||
# Critical-risk handling — bug-1 was that risk_level='critical'
|
||||
# rendered as low because RISK_SEVERITY only mapped 'crit'.
|
||||
# Both aliases must remain in the table so a 'critical' verdict
|
||||
# ranks at 3 and renders with the .risk.crit pill.
|
||||
assert "critical: 3" in body
|
||||
# Critical-risk handling: bug-1 was that risk_level='critical' rendered as
|
||||
# low because the old severity table only mapped 'crit'. The crit/critical
|
||||
# alias moved to the shared conversation.js (step 5e.1b); verify it there so
|
||||
# a 'critical' verdict still ranks like 'crit'.
|
||||
shared = Path(__file__).resolve().parent.parent / (
|
||||
"turnstone/shared_static/conversation.js"
|
||||
)
|
||||
assert 'crit: "critical"' in shared.read_text(encoding="utf-8")
|
||||
# Child approves must round-trip through the routing proxy at
|
||||
# /v1/api/route/workstreams/{ws_id}/approve — the bare
|
||||
# /v1/api/workstreams/.../approve path only works for the
|
||||
|
||||
@@ -30,6 +30,7 @@ import {
|
||||
stripAnsi,
|
||||
buildWatchResultCard,
|
||||
buildSystemNudgeMarker,
|
||||
maxSeverityItem,
|
||||
} from "/shared/conversation.js";
|
||||
|
||||
function buildCoordChrome(root, opts) {
|
||||
@@ -3222,50 +3223,10 @@ function createCoordinatorPane(root, wsId, opts) {
|
||||
// null if the detail is unusable (defensive \u2014 server is supposed to
|
||||
// emit None when no items). Stays DOM-method-only to match the
|
||||
// zero-innerHTML XSS posture of the rest of the row template.
|
||||
// Risk-level → numeric severity for max-across-items computation.
|
||||
// Values are ordinal: higher integer = higher severity. Both "crit"
|
||||
// and "critical" map to 3 because production emitters disagree:
|
||||
// turnstone/core/judge.py validates against ('low','medium','high',
|
||||
// 'critical') and the heuristic seeds emit the full word, but
|
||||
// earlier dock UI history used the abbreviation. Accept both.
|
||||
// Unknown / malformed risk_level falls back to "high" rank so a
|
||||
// schema drift fails *safe* (over-alert) rather than silently
|
||||
// downgrading to a green pill — fixes the failure mode where
|
||||
// "critical" was treated as unknown and rendered as low.
|
||||
const RISK_SEVERITY = {
|
||||
low: 0,
|
||||
medium: 1,
|
||||
med: 1,
|
||||
high: 2,
|
||||
crit: 3,
|
||||
critical: 3,
|
||||
};
|
||||
const UNKNOWN_RISK_RANK = 2; // fail-safe: treat unknown as "high"
|
||||
|
||||
function _riskRank(verdict) {
|
||||
if (!verdict) return -1;
|
||||
const risk = (verdict.risk_level || "").toLowerCase();
|
||||
return RISK_SEVERITY[risk] != null
|
||||
? RISK_SEVERITY[risk]
|
||||
: UNKNOWN_RISK_RANK;
|
||||
}
|
||||
|
||||
// Pick the item carrying the highest risk_level — pill colour and
|
||||
// body display follow the worst tool in the envelope so a low-risk
|
||||
// item[0] can't visually mask a crit item[2].
|
||||
function _maxSeverityItem(items) {
|
||||
let best = items[0];
|
||||
let bestRank = _riskRank(best.judge_verdict || best.heuristic_verdict);
|
||||
for (let i = 1; i < items.length; i += 1) {
|
||||
const v = items[i].judge_verdict || items[i].heuristic_verdict;
|
||||
const r = _riskRank(v);
|
||||
if (r > bestRank) {
|
||||
best = items[i];
|
||||
bestRank = r;
|
||||
}
|
||||
}
|
||||
return best;
|
||||
}
|
||||
// Risk-level severity ranking moved to the shared conversation.js
|
||||
// (maxSeverityItem / riskRank, imported above) so the coordinator and
|
||||
// interactive panes can't drift on the fallback. Unknown / malformed
|
||||
// risk_level ranks "medium" (step 5e.1b: this pane's old rank used "high").
|
||||
|
||||
function _evidenceLineText(line) {
|
||||
if (typeof line === "string") return line;
|
||||
@@ -3350,7 +3311,7 @@ function createCoordinatorPane(root, wsId, opts) {
|
||||
// them all so leading with [0] keeps the operator's mental model
|
||||
// anchored on "what the LLM dispatched first").
|
||||
const primary = items[0];
|
||||
const severityItem = _maxSeverityItem(items);
|
||||
const severityItem = maxSeverityItem(items);
|
||||
const judge = severityItem.judge_verdict || null;
|
||||
const heuristic = severityItem.heuristic_verdict || null;
|
||||
const verdict = judge || heuristic;
|
||||
|
||||
@@ -83,3 +83,47 @@ export function buildSystemNudgeMarker() {
|
||||
el.textContent = "system nudge";
|
||||
return el;
|
||||
}
|
||||
|
||||
// Canonical risk-level vocabulary shared by both panes. RISK_LEVELS is ordinal
|
||||
// (index == severity rank); the aliases cover emitters that abbreviate. An
|
||||
// unknown / unrecognized level normalizes to "medium": not "low" (an unlabelled
|
||||
// risk must not render benign) and not "high" (medium is the neutral default
|
||||
// both panes already displayed; the coordinator's old rank used "high", which
|
||||
// step 5e.1b brings to "medium" per the unify decision).
|
||||
const RISK_LEVELS = ["low", "medium", "high", "critical"];
|
||||
const RISK_ALIASES = { med: "medium", crit: "critical" };
|
||||
|
||||
export function normalizeRiskLevel(raw) {
|
||||
let s = String(raw == null ? "" : raw)
|
||||
.trim()
|
||||
.toLowerCase();
|
||||
s = RISK_ALIASES[s] || s;
|
||||
return RISK_LEVELS.indexOf(s) >= 0 ? s : "medium";
|
||||
}
|
||||
|
||||
// Ordinal rank (low=0 .. critical=3) via the canonical normalize, so an alias or
|
||||
// unknown value ranks consistently with how it displays. Unknown -> medium (1).
|
||||
export function riskRank(raw) {
|
||||
return RISK_LEVELS.indexOf(normalizeRiskLevel(raw));
|
||||
}
|
||||
|
||||
// Pick the item carrying the highest risk_level (judge verdict preferred over
|
||||
// heuristic) so a low-risk item[0] can't visually mask a higher-risk item[2].
|
||||
// An item with NO verdict ranks below "low" (-1) so it never wins; a verdict
|
||||
// with an unknown level ranks "medium" via riskRank.
|
||||
export function maxSeverityItem(items) {
|
||||
function rank(it) {
|
||||
const v = it && (it.judge_verdict || it.heuristic_verdict);
|
||||
return v ? riskRank(v.risk_level) : -1;
|
||||
}
|
||||
let best = items[0];
|
||||
let bestRank = rank(best);
|
||||
for (let i = 1; i < items.length; i += 1) {
|
||||
const r = rank(items[i]);
|
||||
if (r > bestRank) {
|
||||
best = items[i];
|
||||
bestRank = r;
|
||||
}
|
||||
}
|
||||
return best;
|
||||
}
|
||||
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
stripAnsi,
|
||||
buildWatchResultCard,
|
||||
buildSystemNudgeMarker,
|
||||
normalizeRiskLevel,
|
||||
} from "./conversation.js";
|
||||
|
||||
let _paneCounter = 0;
|
||||
@@ -3182,21 +3183,6 @@ function buildToolDiv(item) {
|
||||
return div;
|
||||
}
|
||||
|
||||
// Server-supplied ``risk_level`` is constrained to this enum, but it lands
|
||||
// in ``className`` / ``data-risk`` strings that the verdict + output-warning
|
||||
// CSS and the ``badge.nextElementSibling`` / ``data-risk`` selectors rely on.
|
||||
// Funnel every interpolation through one chokepoint so whitespace, a stray
|
||||
// case, or a future relaxed-validation server value can't break selector
|
||||
// targeting silently (issue #562) — unknown / blank → the neutral default.
|
||||
const VALID_RISK_LEVELS = new Set(["low", "medium", "high", "critical"]);
|
||||
|
||||
function normalizeRiskLevel(raw) {
|
||||
const s = String(raw || "")
|
||||
.trim()
|
||||
.toLowerCase();
|
||||
return VALID_RISK_LEVELS.has(s) ? s : "medium";
|
||||
}
|
||||
|
||||
function renderVerdictBadge(verdict, judgePending) {
|
||||
if (!verdict) return document.createDocumentFragment();
|
||||
const risk = normalizeRiskLevel(verdict.risk_level);
|
||||
|
||||
Reference in New Issue
Block a user