fix(webui): address #905 REQUEST_CHANGES on notifications classifier

B1: classify_attention_event uses structured flags/category only — never
substring-match human-authored title/summary for escalation.

B2: make notification ids unique across probe_errors and collisions
(include loop index / kind).

B3: do not assign probe_errors to fetch_error (avoids false Fetch Warning
and double-reporting).

Regression tests cover all three blockers.

Refs #648
This commit is contained in:
2026-07-25 18:26:46 -04:00
parent bb8c3a537b
commit a64ba08e27
2 changed files with 206 additions and 11 deletions
+189
View File
@@ -274,3 +274,192 @@ def test_notifications_app_routes():
res_alias = client.get("/api/notifications") res_alias = client.get("/api/notifications")
assert res_alias.status_code == 200 assert res_alias.status_code == 200
assert res_alias.json()["project_id"] == json_data["project_id"] assert res_alias.json()["project_id"] == json_data["project_id"]
def test_classify_ignores_human_authored_title_and_summary_keywords():
"""B1: keywords in human-authored titles must not escalate routine work (#905)."""
# Routine transition whose title/summary mention critical-boundary words
att_cls, req_human = classify_attention_event(
CATEGORY_WORKFLOW,
"record irrecoverable decision lock provenance",
"PR #999 'record irrecoverable decision lock provenance' is in routine state in-review.",
)
assert att_cls == ATTENTION_ROUTINE
assert req_human is False
att_cls, req_human = classify_attention_event(
CATEGORY_WORKFLOW,
"fix unauthorized token path",
"Issue #1 'fix unauthorized token path' state: claimed. hard stop docs only.",
)
assert att_cls == ATTENTION_ROUTINE
assert req_human is False
# Structured flags still escalate (machine-driven)
att_cls, req_human = classify_attention_event(
CATEGORY_SYSTEM,
"anything",
"anything with hard stop in text",
is_hard_stop=True,
)
assert att_cls == ATTENTION_HUMAN_REQUIRED
assert req_human is True
def test_notification_ids_are_unique_across_probe_errors_and_collisions():
"""B2: published notification ids must be unique within a snapshot (#905)."""
reg = load_registry()
proj_id = reg.projects[0].id if reg.projects else "gitea-tools"
mock_queue = QueueSnapshot(
project_id=proj_id,
repo_label="org/repo",
prs=(),
issues=(),
pr_pagination=None,
issue_pagination=None,
)
mock_leases = LeaseSnapshot(
project_id=proj_id,
repo_label="org/repo",
issue_lock=None,
claim_inventory={},
reviewer_leases=(),
duplicate_prs=(
CollisionWarning(
kind="duplicate_pr",
message="Multiple open PRs for issue #10",
issue_number=10,
pr_numbers=(10, 11),
),
CollisionWarning(
kind="duplicate_branch",
message="Another collision without issue",
issue_number=None,
pr_numbers=(12, 13),
),
CollisionWarning(
kind="duplicate_pr",
message="Second issue collision",
issue_number=10,
pr_numbers=(14, 15),
),
),
duplicate_branches=(),
collision_history=(),
fetch_error=None,
)
mock_version = VersionInfo(
git_sha="abc1234",
git_describe="v1.0.0",
control_plane_schema_version=1,
python_version="3.11",
known=True,
)
mock_stale = StaleRuntime(
daemon_head="abc1234",
checkout_head="abc1234",
remote_head="abc1234",
stale=False,
determinable=True,
mutation_safe=True,
reasons=(),
)
mock_health = SystemHealthSnapshot(
status="degraded",
ready=False,
readiness_complete=True,
readiness_reasons=(),
service="webui",
mode="test",
version=mock_version,
started_at="2026-07-25T00:00:00Z",
uptime_seconds=100.0,
timestamp="2026-07-25T00:00:00Z",
deep_probes_requested=True,
dependencies=(),
mcp_namespaces=(),
stale_runtime=mock_stale,
probe_errors=("error alpha", "error beta"),
)
snapshot = load_notifications_snapshot(
proj_id,
load_queue=lambda _id: mock_queue,
load_leases=lambda **_kwargs: mock_leases,
load_health=lambda **_kwargs: mock_health,
)
ids = [item.id for item in snapshot.items]
assert len(ids) == len(set(ids)), f"duplicate notification ids: {ids}"
assert any(i.startswith(f"notif-sys-err-{proj_id}-") for i in ids)
assert any(i.startswith("notif-collision-") for i in ids)
def test_probe_errors_do_not_set_fetch_error():
"""B3: probe_errors must not be reported as fetch_error (#905)."""
reg = load_registry()
proj_id = reg.projects[0].id if reg.projects else "gitea-tools"
mock_queue = QueueSnapshot(
project_id=proj_id,
repo_label="org/repo",
prs=(),
issues=(),
pr_pagination=None,
issue_pagination=None,
fetch_error=None,
)
mock_leases = LeaseSnapshot(
project_id=proj_id,
repo_label="org/repo",
issue_lock=None,
claim_inventory={},
reviewer_leases=(),
duplicate_prs=(),
duplicate_branches=(),
collision_history=(),
fetch_error=None,
)
mock_version = VersionInfo(
git_sha="abc1234",
git_describe="v1.0.0",
control_plane_schema_version=1,
python_version="3.11",
known=True,
)
mock_stale = StaleRuntime(
daemon_head="abc1234",
checkout_head="abc1234",
remote_head="abc1234",
stale=False,
determinable=True,
mutation_safe=True,
reasons=(),
)
mock_health = SystemHealthSnapshot(
status="degraded",
ready=False,
readiness_complete=True,
readiness_reasons=(),
service="webui",
mode="test",
version=mock_version,
started_at="2026-07-25T00:00:00Z",
uptime_seconds=100.0,
timestamp="2026-07-25T00:00:00Z",
deep_probes_requested=True,
dependencies=(),
mcp_namespaces=(),
stale_runtime=mock_stale,
probe_errors=("probe blew up",),
)
snapshot = load_notifications_snapshot(
proj_id,
load_queue=lambda _id: mock_queue,
load_leases=lambda **_kwargs: mock_leases,
load_health=lambda **_kwargs: mock_health,
)
assert snapshot.fetch_error is None
# probe errors still appear as items
assert any("probe blew up" in item.summary for item in snapshot.items)
+17 -11
View File
@@ -7,13 +7,12 @@ operators receive direct alerts only for human-required escalation boundaries
from __future__ import annotations from __future__ import annotations
import os
from dataclasses import dataclass, field from dataclasses import dataclass, field
from datetime import datetime, timezone from datetime import datetime, timezone
from typing import Any, Callable, Sequence from typing import Any, Callable
from webui import console_redaction from webui import console_redaction
from webui.project_registry import find_project, load_registry from webui.project_registry import load_registry
from webui.queue_loader import QueueSnapshot, load_queue_snapshot from webui.queue_loader import QueueSnapshot, load_queue_snapshot
from webui.lease_loader import LeaseSnapshot, load_lease_snapshot from webui.lease_loader import LeaseSnapshot, load_lease_snapshot
from webui.system_health import SystemHealthSnapshot, load_system_health from webui.system_health import SystemHealthSnapshot, load_system_health
@@ -158,7 +157,13 @@ def classify_attention_event(
2. Operational queues (blocker, stale lease, unassigned ready work, queue collision) 2. Operational queues (blocker, stale lease, unassigned ready work, queue collision)
-> ATTENTION_OPERATOR (requires_human=False). -> ATTENTION_OPERATOR (requires_human=False).
3. Routine state transitions (clean progression, healthy heartbeats) -> ATTENTION_ROUTINE (requires_human=False). 3. Routine state transitions (clean progression, healthy heartbeats) -> ATTENTION_ROUTINE (requires_human=False).
Classification uses structured flags and category only. Human-authored
``title`` / ``summary`` text is never substring-matched for escalation
(PR #905 review B1) — callers that need text signals must set flags from
machine-generated status/detail fields before calling this function.
""" """
del title, summary # kept for API stability; never used for classification
if ( if (
is_hard_stop is_hard_stop
or is_auth_failure or is_auth_failure
@@ -166,9 +171,6 @@ def classify_attention_event(
or is_decision_lock or is_decision_lock
or is_validation_failure or is_validation_failure
or category in {CATEGORY_AUTH, CATEGORY_VALIDATION} or category in {CATEGORY_AUTH, CATEGORY_VALIDATION}
or "hard stop" in summary.lower()
or "unauthorized" in summary.lower()
or "irrecoverable" in summary.lower()
): ):
return ATTENTION_HUMAN_REQUIRED, True return ATTENTION_HUMAN_REQUIRED, True
@@ -234,7 +236,7 @@ def load_notifications_snapshot(
now_iso = datetime.now(timezone.utc).isoformat() now_iso = datetime.now(timezone.utc).isoformat()
# 1. System health alerts (highest priority) # 1. System health alerts (highest priority)
for probe_err in getattr(health_snap, "probe_errors", ()): for err_idx, probe_err in enumerate(getattr(health_snap, "probe_errors", ())):
att_cls, req_human = classify_attention_event( att_cls, req_human = classify_attention_event(
CATEGORY_SYSTEM, CATEGORY_SYSTEM,
"System Health Probe Error", "System Health Probe Error",
@@ -243,7 +245,7 @@ def load_notifications_snapshot(
) )
items.append( items.append(
NotificationItem( NotificationItem(
id=f"notif-sys-err-{project.id}", id=f"notif-sys-err-{project.id}-{err_idx}",
attention_class=att_cls, attention_class=att_cls,
category=CATEGORY_SYSTEM, category=CATEGORY_SYSTEM,
title="System Health Error", title="System Health Error",
@@ -431,16 +433,18 @@ def load_notifications_snapshot(
) )
) )
for collision in lease_snap.duplicate_prs: for col_idx, collision in enumerate(lease_snap.duplicate_prs):
att_cls, req_human = classify_attention_event( att_cls, req_human = classify_attention_event(
CATEGORY_BLOCKER, CATEGORY_BLOCKER,
f"Duplicate PR Collision ({collision.kind})", f"Duplicate PR Collision ({collision.kind})",
collision.message, collision.message,
is_blocker=True, is_blocker=True,
) )
issue_part = collision.issue_number if collision.issue_number is not None else "none"
kind_part = (collision.kind or "unknown").replace(" ", "-")
items.append( items.append(
NotificationItem( NotificationItem(
id=f"notif-collision-{collision.issue_number or 0}", id=f"notif-collision-{kind_part}-{issue_part}-{col_idx}",
attention_class=att_cls, attention_class=att_cls,
category=CATEGORY_BLOCKER, category=CATEGORY_BLOCKER,
title=f"Collision Alert ({collision.kind})", title=f"Collision Alert ({collision.kind})",
@@ -459,7 +463,9 @@ def load_notifications_snapshot(
operator_count = sum(1 for i in items if i.attention_class == ATTENTION_OPERATOR) operator_count = sum(1 for i in items if i.attention_class == ATTENTION_OPERATOR)
routine_count = sum(1 for i in items if i.attention_class == ATTENTION_ROUTINE) routine_count = sum(1 for i in items if i.attention_class == ATTENTION_ROUTINE)
fetch_err = queue_snap.fetch_error or lease_snap.fetch_error or getattr(health_snap, "probe_errors", None) # Fetch errors are transport/load failures only — not probe results that
# already surface as first-class notification items (PR #905 review B3).
fetch_err = queue_snap.fetch_error or lease_snap.fetch_error
if isinstance(fetch_err, (tuple, list)): if isinstance(fetch_err, (tuple, list)):
fetch_err = "; ".join(fetch_err) if fetch_err else None fetch_err = "; ".join(fetch_err) if fetch_err else None