Merge pull request #403 from razzant/claude/review-findings-ui-20260830

Show review findings in the task-card Reviews checkpoint
This commit is contained in:
Anton Razzhigaev 2026-08-30 15:35:46 +03:00 • committed by GitHub
commit dd676121d2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 970 additions and 66 deletions

File diff suppressed because one or more lines are too long

View file

@ -1,14 +1,61 @@
"""Tiny public execution receipts projected from review actor usage.
"""Tiny public read-side projections from review actor records.
This leaf owns the cross-surface presentation wire. It deliberately imports no
review engine: callers pass returned actor usage, and only actual receipt facts
can become an API or harness execution badge.
This leaf owns the cross-surface presentation wire: execution receipts and the
bounded finding rows. It deliberately imports no review engine: callers pass
returned actor facts, and only actual receipt/finding content is projected.
"""
from __future__ import annotations
import json
from typing import Any, Dict, List
from ouroboros.observability import redact_projection
from ouroboros.utils import truncate_review_artifact
# Bounded structured findings on the public actor projection: the bound is the
# ROW COUNT, never a second aggressive cut of the finding bodies — capping the
# only owner-reachable copy of reviewer text is the class v6.70.0 removed. The
# per-string bound mirrors plan review's MAX_FINDING_TEXT_CHARS (2000), not the
# 200-char path-list default; the durable remainder stays addressable through
# the actor's existing response_ref, so no full-set hash is needed.
MAX_PROJECTED_ACTOR_FINDINGS = 8
PROJECTED_FINDING_TEXT_CHARS = 2000
_PROJECTED_FINDING_KEYS = (
"id", "severity", "verdict", "item", "summary", "evidence", "reason",
"recommendation",
)
def projected_finding_row(item: Any) -> Dict[str, str]:
"""One bounded, redacted finding row for the public actor projection."""
row: Dict[str, str] = {}
if not isinstance(item, dict):
return row
for key in _PROJECTED_FINDING_KEYS:
value = item.get(key)
if value is None or value == "":
continue
if isinstance(value, str):
rendered = str(redact_projection(value).value)
else:
# A non-string value keeps structural key-based masking: str()
# first would flatten a nested secret past the key-name redactor.
rendered = json.dumps(
redact_projection(value).value, ensure_ascii=False, default=str,
)
row[key] = truncate_review_artifact(rendered, PROJECTED_FINDING_TEXT_CHARS)
if not row:
# An unknown finding shape still carries evidence; a silently empty row
# would destroy it without a trace. Redact the OBJECT before
# serializing: structural key-based secret masking does not survive a
# pre-serialized string.
row["item"] = truncate_review_artifact(
json.dumps(redact_projection(item).value, ensure_ascii=False, default=str),
PROJECTED_FINDING_TEXT_CHARS,
)
return row
_API_EXECUTION_RECEIPT_KEYS = frozenset({
"resolved_model", "provider", "prompt_tokens", "completion_tokens",
@ -78,4 +125,10 @@ def review_executions_from_actor_usage(actors: Any) -> List[Dict[str, str]]:
return normalize_review_executions(executions)
__all__ = ["normalize_review_executions", "review_executions_from_actor_usage"]
__all__ = [
"MAX_PROJECTED_ACTOR_FINDINGS",
"PROJECTED_FINDING_TEXT_CHARS",
"normalize_review_executions",
"projected_finding_row",
"review_executions_from_actor_usage",
]

View file

@ -22,7 +22,10 @@ log = logging.getLogger("review_substrate")
from ouroboros.llm import LLMClient
from ouroboros.observability import new_call_id, persist_call, redact_projection
from ouroboros.provider_models import provider_for_model
from ouroboros.review_execution_projection import review_executions_from_actor_usage
from ouroboros.review_execution_projection import (
MAX_PROJECTED_ACTOR_FINDINGS, projected_finding_row,
review_executions_from_actor_usage,
)
from ouroboros.task_results import review_binding_hash
# Everything below the seam. Re-exported here because the substrate is the
# historical import site for the api_chat prompt renderers; `review_execution`
@ -60,6 +63,7 @@ from ouroboros.usage_accounting import (
usage_scope,
)
from ouroboros.utils import sanitize_tool_result_for_log, truncate_review_artifact
from ouroboros._outcome_receipts import disclosed_list_projection
class _CustodyUsageContext:
@ -346,7 +350,7 @@ def _review_actor_projection(actor: Any, surface: str) -> Dict[str, Any]:
)
if dialogue_vote not in DIALOGUE_STATUS_VALUES:
dialogue_vote = ""
return {
projection = {
"slot_id": str(row.get("slot_id") or ""), "model": model, "provider": provider,
"actor_role": str(row.get("actor_role") or f"{surface} reviewer"),
"transport_status": transport,
@ -369,6 +373,16 @@ def _review_actor_projection(actor: Any, surface: str) -> Dict[str, Any]:
# Flat, redacted pointer to the private full response artifact.
"response_ref": _response_ref_projection(row.get("response_ref")),
}
# Structured rows ride only where a parsed response exists: an absent
# `findings` key is a hole, never the claim "zero findings reported".
if parsed is not None:
projection.update(disclosed_list_projection(
parsed_findings,
key="findings",
limit=MAX_PROJECTED_ACTOR_FINDINGS,
item=projected_finding_row,
))
return projection
def _response_ref_projection(ref: Any) -> Dict[str, str]:

View file

@ -233,5 +233,5 @@ BYTE_DEBT = {
"ouroboros/loop.py": 312772,
"tests/test_delegated_subagent_transport.py": 320340,
"tests/test_devtools_benchmarks.py": 328116,
"web/modules/chat.js": 224315,
"web/modules/chat.js": 224244,
}

View file

@ -97,7 +97,8 @@ def _file_stamp(path: pathlib.Path) -> tuple:
def skill_review_ui_projection(
drive_root: pathlib.Path, skill_name: str,
) -> Dict[str, Any]:
"""Sanitized current run and the last ten rows in its review group."""
"""Sanitized current run, the last ten rows in its review group, and the
exact count of older group rows that ten-row window leaves out."""
cache_key = (str(drive_root), str(skill_name))
stamp = (
_file_stamp(review_job_state_path(drive_root, skill_name)),
@ -111,7 +112,8 @@ def skill_review_ui_projection(
group_id = str(current.get("group_id") or "")
if not group_id and all_history:
group_id = str(all_history[-1].get("group_id") or "")
history = [row for row in all_history if not group_id or row.get("group_id") == group_id][-10:]
group_rows = [row for row in all_history if not group_id or row.get("group_id") == group_id]
history = group_rows[-10:]
projection: Dict[str, Any]
if not current and not history:
projection = {}
@ -119,6 +121,9 @@ def skill_review_ui_projection(
projection = {
"current": _review_ui_row(current) if current else {},
"history": [_review_ui_row(row) for row in history],
# Group-scoped disclosed bound (BIBLE P1): the exact number of
# older rows the ten-row window left out, 0 included.
"history_omitted": max(0, len(group_rows) - len(history)),
}
_UI_PROJECTION_CACHE[cache_key] = (stamp, projection)
return projection

View file

@ -1054,6 +1054,150 @@ def test_compact_review_projection_redacts_public_reasons_before_truncation():
assert "***REDACTED***" in panel["reason"]
def test_actor_projection_carries_bounded_disclosed_finding_rows():
from ouroboros.review_substrate import (
MAX_PROJECTED_ACTOR_FINDINGS, compact_review_projection,
)
secret = "sk-or-" + ("FindingSecret456" * 4)
long_recommendation = "Re-run the verifier with the fixed seed. " * 120
findings = [
{
"severity": "critical",
"item": f"finding {index}",
"evidence": f"evidence {index}",
"recommendation": f"fix {index}",
}
for index in range(MAX_PROJECTED_ACTOR_FINDINGS + 2)
]
findings[0]["evidence"] = "credential=" + secret
findings[1]["recommendation"] = long_recommendation
run = {
"request": {"surface": "task_acceptance", "policy": {"min_successful_slots": 1}},
"aggregate_signal": "FAIL",
"actors": [
{
"slot_id": "with-findings",
"model": "model-a",
"status": "ok",
"signal": "FAIL",
"parsed": {"verdict": "FAIL", "summary": "s", "findings": findings},
"quorum_contribution": True,
},
{
"slot_id": "clean",
"model": "model-b",
"status": "ok",
"signal": "PASS",
"parsed": {"verdict": "PASS", "summary": "ok", "findings": []},
"quorum_contribution": True,
},
{
"slot_id": "transport-hole",
"model": "model-c",
"status": "error",
"error": "timed out",
"parsed": None,
},
{
"slot_id": "odd-shape",
"model": "model-d",
"status": "ok",
"signal": "FAIL",
"parsed": {
"verdict": "FAIL",
"summary": "s",
"findings": [{
"weird_key": "the only copy of this evidence",
"password": "hunter2-odd-shape",
}],
},
},
{
# A non-string value under a KNOWN key keeps structural
# key-based masking: str() first would flatten the nested
# secret past the key-name redactor.
"slot_id": "nested-evidence",
"model": "model-f",
"status": "ok",
"signal": "FAIL",
"parsed": {
"verdict": "FAIL",
"summary": "s",
"findings": [{
"severity": "high",
"item": "nested shape",
"evidence": {"password": "hunter2-nested-shape"},
}],
},
},
{
# The array-ladder reviewer contract shapes findings as
# {item, verdict, severity, reason}: the substantive `reason`
# text must survive projection.
"slot_id": "triad-shape",
"model": "model-e",
"status": "ok",
"signal": "FAIL",
"parsed": [{
"item": "missing rollback test",
"verdict": "FAIL",
"severity": "high",
"reason": "the new path has no failure-injection coverage",
}],
},
],
}
panel = compact_review_projection([run])["panels"][0]
actors = {actor["slot_id"]: actor for actor in panel["actors"]}
rendered = json.dumps(panel, ensure_ascii=False)
rows = actors["with-findings"]["findings"]
assert len(rows) == MAX_PROJECTED_ACTOR_FINDINGS
assert actors["with-findings"]["findings_omitted"] == 2
assert rows[2] == {
"severity": "critical", "item": "finding 2",
"evidence": "evidence 2", "recommendation": "fix 2",
}
# The count stays beside the rows: coverage keeps the full total.
assert actors["with-findings"]["coverage"]["findings"] == len(findings)
# Redaction covers finding bodies exactly like reasons.
assert secret not in rendered
assert "***REDACTED***" in rows[0]["evidence"]
# A clipped string discloses its own cut instead of clipping silently.
assert "OMISSION NOTE" in rows[1]["recommendation"]
assert len(rows[1]["recommendation"]) < len(long_recommendation)
# A reviewer that reported no findings states that as an empty disclosed
# list; a reviewer with no parsed response leaves a hole, not a zero.
assert actors["clean"]["findings"] == []
assert actors["clean"]["findings_omitted"] == 0
assert "findings" not in actors["transport-hole"]
assert "findings_omitted" not in actors["transport-hole"]
# An unknown finding shape still ships its evidence as a bounded row, and
# structural key-based secret masking applies BEFORE serialization.
odd_rows = actors["odd-shape"]["findings"]
assert odd_rows and "the only copy of this evidence" in odd_rows[0]["item"]
assert "hunter2-odd-shape" not in rendered
assert "***REDACTED***" in odd_rows[0]["item"]
nested_rows = actors["nested-evidence"]["findings"]
assert "hunter2-nested-shape" not in rendered
assert "***REDACTED***" in nested_rows[0]["evidence"]
assert nested_rows[0]["item"] == "nested shape"
# A list-shaped parsed response (array reviewer contract) projects its
# rows too, and the substantive `reason`/`verdict` fields survive.
triad_rows = actors["triad-shape"]["findings"]
assert triad_rows == [{
"severity": "high", "verdict": "FAIL", "item": "missing rollback test",
"reason": "the new path has no failure-injection coverage",
}]
assert actors["triad-shape"]["findings_omitted"] == 0
class _MixedPassPassFailLLM:
def chat(self, **kwargs):
if str(kwargs.get("model") or "").endswith("-2"):

View file

@ -929,6 +929,9 @@ def test_skill_review_ui_projection_is_group_scoped_bounded_and_sanitized(tmp_pa
assert projection["history"][0]["review_round"] == 3
assert all(row["group_id"] == "manual:alpha" for row in projection["history"])
assert all("raw_actor_records" not in row for row in projection["history"])
# The ten-row window is a disclosed bound: 12 group rows minus 10 shown.
# The foreign-group row must not count into the omitted number.
assert projection["history_omitted"] == 2
def test_cancel_and_timeout_each_write_one_idempotent_terminal_row(tmp_path):

View file

@ -45,6 +45,18 @@ def test_ui_smoke_review_truth_is_visible_in_chat_and_logs(direct_server_with_da
"quorum_contribution": True,
"enforcement_impact": "supports_pass",
"reason": "The browser evidence is incomplete.",
"coverage": {"criteria_total": 3, "findings": 3},
"findings": [
{
"severity": "high",
"item": "Browser evidence missing for the checkout flow",
"evidence": "no screenshot covers step 3",
"recommendation": "Capture the payment page state",
},
{"severity": "low", "item": "Trace summary is terse"},
],
"findings_omitted": 1,
"response_ref": {"call_id": "call-fable-1", "sha256": "a" * 64},
},
{
"slot_id": "sol",
@ -110,12 +122,46 @@ def test_ui_smoke_review_truth_is_visible_in_chat_and_logs(direct_server_with_da
}) + "\n", encoding="utf-8")
task_results = data_dir / "task_results"
task_results.mkdir(parents=True, exist_ok=True)
plan_state = {
"schema_version": 2,
"current_attempt": {"fingerprint": "f" * 64, "status": "open"},
"waves": [{
"request_fingerprint": "f" * 64,
"cycle_index": 1,
"aggregate": "REVISE_PLAN",
"closed": False,
"paid": True,
"counts": {"blocking": 1, "note": 0, "need_evidence": 0},
"findings": [{
"finding_id": "slot_1:f1",
"id": "f1",
"class": "blocking",
"summary": "The migration step drops the audit ledger",
"breaks": "invariant_1",
"locator": "ouroboros/usage_accounting.py",
"recommendation": "Keep the ledger append-only through the migration",
"slot": "slot_1",
"model": "anthropic/claude-fable-5",
}],
"dispositions": [{
"finding_id": "slot_1:f1",
"decision": "accept",
"rationale": "will rework the migration",
}],
"actors": [
{"slot_id": "slot_1", "model": "anthropic/claude-fable-5", "ok": True},
],
"reviewed_at": "2026-07-15T09:59:58+00:00",
}],
"waves_omitted": 0,
}
(task_results / "review-no-summary.json").write_text(json.dumps({
"task_id": "review-no-summary",
"status": "completed",
"reason_code": "acceptance_degraded",
"outcome_axes": axes,
"review_projection": projection,
"plan_review_state": plan_state,
}) + "\n", encoding="utf-8")
try:
@ -135,11 +181,30 @@ def test_ui_smoke_review_truth_is_visible_in_chat_and_logs(direct_server_with_da
assert "Review panel panel_visual_truth" in chat_text
assert "Reviewer fable" in chat_text
assert "Reviewer sol" in chat_text
# The reviewer's actual findings are readable, not just counted.
assert "Browser evidence missing for the checkout flow" in chat_text
assert "fix: Capture the payment page state" in chat_text
assert "Reviewer fable findings omitted: 1" in chat_text
assert "observability call call-fable-1" in chat_text
no_summary = page.locator('.chat-live-card[data-task-id="review-no-summary"]')
no_summary.wait_for(state="attached", timeout=30_000)
assert no_summary.get_attribute("data-expanded") == "0"
# This card carries BOTH groups; the helper opens the first
# (Plan review), whose finding text must be readable.
_open_review_checkpoint(no_summary)
assert no_summary.locator('[data-live-phase]').first.get_attribute("data-phase") == "warn"
plan_text = no_summary.inner_text()
assert "The migration step drops the audit ledger" in plan_text
assert "breaks invariant_1" in plan_text
assert "agent: accept — will rework the migration" in plan_text
acceptance_toggle = no_summary.locator(
'[data-review-group-toggle="task_acceptance:review-no-summary"]',
)
acceptance_toggle.click()
no_summary.locator(
'[data-review-group="task_acceptance:review-no-summary"]'
' [data-review-attempt-toggle]',
).first.click()
assert "Review panel panel_visual_truth" in no_summary.inner_text()
page.wait_for_timeout(900) # cover the routine background history sync
assert no_summary.locator('.chat-live-line-repeat:not([hidden])').count() == 0
@ -157,6 +222,9 @@ def test_ui_smoke_review_truth_is_visible_in_chat_and_logs(direct_server_with_da
assert "Review panel panel_visual_truth" in log_text
assert "Reviewer fable" in log_text
assert "Reviewer sol" in log_text
# The same formatter serves Logs: finding bodies ride along.
assert "Browser evidence missing for the checkout flow" in log_text
assert "Reviewer fable findings omitted: 1" in log_text
assert log_card.locator('[data-task-phase]').inner_text() == "warn"
review.scroll_into_view_if_needed()
review.screenshot(path=str(data_dir.parent / "review-truth-logs.png"))

View file

@ -106,6 +106,23 @@ export async function fetchTaskDetail(taskId) {
return (resp && typeof resp.json === 'function' && resp.ok !== false) ? resp.json() : null;
}
/**
* Strict task-detail read for consumers that must tell a genuinely absent
* record (404 → null) apart from a failed read (rejects). The lenient
* fetchTaskDetail above keeps its every-failure→null contract for reconcile
* flows that treat all misses alike.
* @param {string} taskId
* @returns {Promise<import('./api_types.js').TaskDetailResponse|null>}
*/
export async function fetchTaskDetailStrict(taskId) {
const resp = await apiFetch(`/api/tasks/${encodeURIComponent(taskId)}`);
if (resp && typeof resp.json === 'function') {
if (resp.ok !== false) return resp.json();
if (resp.status === 404) return null;
}
throw new Error(`task detail read failed (HTTP ${resp?.status ?? 'no response'})`);
}
export function cleanExtensionRoute(value) {
const route = String(value || '').trim().replace(/^\/+/, '');
const parts = route.split('/').filter(Boolean);

View file

@ -317,6 +317,13 @@
* v6.74.0 additive keys: panels[].dialogue ({status, votes} — the reviewer-authored
* dialogue-status reduction), panels[].single_reviewer_no_diversity (boolean label),
* and actors[].dialogue_status ("continue_actionable"|"unreachable_here"|"stable_disagreement"|"").
* Additive bounded-findings keys: actors[].findings (disclosed rows
* {id?, severity?, verdict?, item?, summary?, evidence?, reason?,
* recommendation?} — redacted, each string
* bounded with an explicit omission marker, at most 8 rows per actor) and
* actors[].findings_omitted (exact count, 0 included). Both are emitted only
* when that reviewer produced a parsed response; their absence is a
* transport/parse hole, never "zero findings".
* @property {boolean=} worker_saturation_warning
* @property {string=} source
* @property {string=} sender_label
@ -601,7 +608,7 @@
* @property {boolean=} official_hub_verified
* @property {boolean=} owner_attestable
* @property {{visible: boolean, publication_ready: boolean, task_start_allowed: boolean, disabled: boolean, state: "ready"|"warnings"|"needs_attention"|"repairable"|"hard_block", reason: string}=} submit_hub
* @property {{current: Object, history: Object[]}=} skill_review
* @property {{current: Object, history: Object[], history_omitted: number=}=} skill_review
* @property {boolean=} is_self_authored
* @property {Object=} grants
* @property {string[]=} permissions

View file

@ -8,7 +8,7 @@ import { PAGE_ICONS } from './page_icons.js';
import { showToast } from './toast.js';
import { createSystemMessageAction, downloadViaHostBridge, openViaHostBridge } from './ui_helpers.js';
import { clientSurfaceField } from './client_surface.js';
import { apiClient, apiFetch, fetchTaskDetail } from './api_client.js';
import { apiClient, apiFetch, fetchTaskDetail, fetchTaskDetailStrict } from './api_client.js';
import {
OWNER_STOP_DETAIL_MARKER,
getLogTaskGroupId,
@ -147,7 +147,7 @@ const MAX_PENDING_ATTACHMENT_BYTES = 100 * 1024 * 1024;
const shownIncidentToastKeys = new Set();
function showTaskIncidentToast(msg) {
const incident = String(msg?.task_incident || '').trim();
const incident = taskKey(msg?.task_incident);
if (!incident) return;
const key = String(msg?.toast_once || `${msg?.task_id || ''}:${incident}`).trim();
if (!key || shownIncidentToastKeys.has(key)) return;
@ -164,6 +164,8 @@ export function initChat(ctx) {
return createChatInstance(ctx);
}
const taskKey = (value) => String(value || '').trim();
export function createChatInstance({
ws, state, updateUnreadBadge, openSettingsTab, openDashboardTab,
stateSnapshots,
@ -507,12 +509,12 @@ export function createChatInstance({
const reviewDisclosureByTask = new Map();
const skillReviewDetailStore = new Map();
const reviewHydrator = createReviewHydrator({
fetchDetail: (taskId) => fetchTaskDetail(taskId),
applyDetail: (taskId, detail) => !destroyed && attachTaskDetailReviews(taskId, detail),
fetchDetail: fetchTaskDetailStrict,
applyDetail: (id, d) => !destroyed && attachTaskDetailReviews(id, d),
onState: (id, status) => !destroyed
&& liveCardRecords.get(id)?.reviewController?.setHydrateStatus?.(status),
});
// Cluster B: a proactively-coined name (task_named) can arrive BEFORE the card's
// liveCardRecords entry exists (the namer broadcasts as the task starts). Buffer it
// here so createLiveCardRecord can apply it when the card appears (no lost title).
// A task_named frame can arrive before the card's record exists; buffer it.
const pendingSuggestedNames = new Map();
const taskUiStates = new Map();
// Busy-chat decision turns reuse the normal agent/event path for ordering and
@ -545,7 +547,7 @@ export function createChatInstance({
}
function recordConcludedActivity(activityId) {
const aid = String(activityId || '').trim();
const aid = taskKey(activityId);
if (!aid) return;
missingManagedTaskIds.delete(aid);
concludedDirectActivities.delete(aid);
@ -556,7 +558,7 @@ export function createChatInstance({
}
}
function recordTerminalActivity(taskId) {
const id = String(taskId || '').trim();
const id = taskKey(taskId);
if (!id) return;
activeDirectActivities.delete(id);
missingManagedTaskIds.delete(id);
@ -577,7 +579,7 @@ export function createChatInstance({
}
function registerEphemeralDecisionFrameMutation(frame) {
const taskId = String(frame?.task_id || '').trim();
const taskId = taskKey(frame?.task_id);
if (!taskId) return false;
if (frame?.ephemeral_decision) {
ephemeralDecisionTaskIds.add(taskId);
@ -1136,7 +1138,7 @@ export function createChatInstance({
async function turnTaskIntoProject(record) {
if (!record || record.root?.dataset?.projectCreating === '1' || record.root?.dataset?.projectCreated === '1') return;
const taskId = String(record.groupId || '').trim();
const taskId = taskKey(record.groupId);
const projectId = projectIdFromTask(taskId);
record.root.dataset.projectCreating = '1';
const actions = record.turnProjectBtn?.parentElement || record.root.querySelector('.chat-live-actions');
@ -1250,7 +1252,7 @@ export function createChatInstance({
// Pending intent stays live until settled; soft stop shows Finalizing….
function markLiveCardCancelPending(taskId = '', soft = false) {
const record = liveCardRecords.get(String(taskId || '').trim());
const record = liveCardRecords.get(taskKey(taskId));
if (!record || record.finished || !record.phaseEl) return;
record.cancelPendingPolicy = soft ? 'finalize' : 'immediate';
record.finalizingHold = false; // owner cancel outranks the hold
@ -1262,7 +1264,7 @@ export function createChatInstance({
// Early final stays live while post-task synthesis runs.
function markLiveCardFinalizing(taskId = '') {
const record = liveCardRecords.get(String(taskId || '').trim());
const record = liveCardRecords.get(taskKey(taskId));
if (!record || record.finished || !record.phaseEl) return;
markReviewAnchor(record);
if (record.cancelPendingPolicy) return;
@ -1294,7 +1296,7 @@ export function createChatInstance({
}
async function cancelRunFromCard(record, action = '') {
const taskId = String(record?.groupId || '').trim();
const taskId = taskKey(record?.groupId);
if (!taskId || record.finished) return;
// Q2: the dropdown itself is the confirmation surface — dismissing it
// continued the run, so a selected action executes immediately.
@ -1352,7 +1354,7 @@ export function createChatInstance({
}
function markTaskCancelable(taskId = '') {
const id = String(taskId || '').trim();
const id = taskKey(taskId);
if (!id || cancelableTaskIds.has(id)) return;
cancelableTaskIds.add(id);
const record = liveCardRecords.get(id);
@ -1411,12 +1413,14 @@ export function createChatInstance({
_chatFreedTimer = setTimeout(() => row.classList.remove('chat-freed'), 900);
}
const reviewAnchorEligible = (id) => !liveCardRecords.has(id)
&& !taskUiStates.has(id) && !activeDirectActivities.has(id);
function attachReviewGroup(group, rawTs = '') {
const ownerTaskId = String(group?.presentationOwnerTaskId || '').trim();
const ownerTaskId = taskKey(group?.presentationOwnerTaskId);
if (!ownerTaskId) return false;
if (retiredTaskIds.has(ownerTaskId) && !liveCardRecords.has(ownerTaskId)) return true;
const reviewAnchor = !liveCardRecords.has(ownerTaskId)
&& !taskUiStates.has(ownerTaskId) && !activeDirectActivities.has(ownerTaskId);
const reviewAnchor = reviewAnchorEligible(ownerTaskId);
const ownerState = forceTaskCard(ownerTaskId, rawTs);
if (!ownerState?.cardVisible) return false;
const record = liveCardRecords.get(ownerTaskId);
@ -1434,7 +1438,7 @@ export function createChatInstance({
}
function attachTaskDetailReviews(taskId, detail) {
const id = String(taskId || '').trim();
const id = taskKey(taskId);
const groups = reviewGroupsFromTaskDetail(detail, id);
if (!id || groups.length === 0) return false;
if (!liveCardRecords.has(id)) forceTaskCard(id, detail?.ts || detail?.timestamp || '');
@ -1446,9 +1450,7 @@ export function createChatInstance({
}
function hydrateCardReviews(taskId, revision = null) {
const id = String(taskId || '').trim();
if (!id || destroyed) return Promise.resolve(false);
return reviewHydrator.hydrate(id, revision);
return destroyed ? Promise.resolve(false) : reviewHydrator.hydrate(taskId, revision);
}
function attachReviewFromRow(row, rawTs = '', showPointerAck = false) {
@ -1481,7 +1483,13 @@ export function createChatInstance({
function handleReviewReference(row) {
const reference = reviewReferenceFromRow(row);
if (!reference) return false;
hydrateCardReviews(reference.presentationOwnerTaskId, reference.stateRevision);
const owner = reference.presentationOwnerTaskId;
// A reference proves a review exists: mint an anchored card so a
// failed hydrate has a home, not fake task activity.
const anchor = reviewAnchorEligible(owner);
forceTaskCard(owner, row?.ts);
if (anchor) markReviewAnchor(liveCardRecords.get(owner), true);
hydrateCardReviews(owner, reference.stateRevision);
return true;
}
@ -1580,8 +1588,7 @@ export function createChatInstance({
// used to name a project on "turn into project" when the server has no
// title/objective yet (P1, direct-chat conversion). One-shot handoff.
objectiveHint: (isMain && !options.isSubagent) ? _pendingCardObjective : '',
// Cluster B: the proactively-coined LLM project name; when set it becomes
// the card title (the activity headline keeps rendering in the lines below).
// The proactively-coined LLM name; becomes the card title when set.
suggestedName: '',
// P1 (v6.82): last bounded activity projection (remembered even while
// the collapsed line is suppressed on unnamed root cards) + sticky cost.
@ -1674,8 +1681,8 @@ export function createChatInstance({
}
function applySuggestedNameMutation(taskId, name) {
const tid = String(taskId || '').trim();
const nm = String(name || '').trim();
const tid = taskKey(taskId);
const nm = taskKey(name);
if (!tid || !nm) return;
const record = liveCardRecords.get(tid);
if (!record) {
@ -2380,17 +2387,17 @@ export function createChatInstance({
subagentChildParents.set(childId, {
parentId: parentId || prev.parentId || '',
role: role || prev.role || '',
model: String(model || '').trim() || prev.model || '',
model: taskKey(model) || prev.model || '',
});
}
function learnSubagentLineage(msg) {
if (String(msg?.delegation_role || '').toLowerCase() !== 'subagent') return '';
const parentId = String(msg.parent_task_id || '').trim();
const parentId = taskKey(msg.parent_task_id);
const childId = String(msg.subagent_task_id || msg.task_id || '').trim();
if (!parentId || !childId || parentId === childId) return '';
setSubagentParent(childId, {
parentId, role: String(msg.subagent_role || '').trim(), model: msg.model,
parentId, role: taskKey(msg.subagent_role), model: msg.model,
});
const event = String(msg.subagent_event || '').toLowerCase();
const replayTerminal = msg.task_terminal_status
@ -2424,7 +2431,7 @@ export function createChatInstance({
markTaskCancelable(String(msg.task_id));
}
// Child lifecycle pings must not update the parent's terminal state.
const lifecycleParent = String(msg?.parent_task_id || '').trim();
const lifecycleParent = taskKey(msg?.parent_task_id);
if (
msg?.subagent_event
&& lifecycleParent
@ -2489,11 +2496,11 @@ export function createChatInstance({
function updateSubagentCardFromEvent(evt, tsValue) {
if (!evt || String(evt.delegation_role || '').toLowerCase() !== 'subagent') return false;
const parentId = String(evt.parent_task_id || '').trim();
const parentId = taskKey(evt.parent_task_id);
const childId = String(evt.subagent_task_id || evt.task_id || '').trim();
if (!parentId || !childId || parentId === childId) return false;
const event = String(evt.subagent_event || '').toLowerCase();
const role = String(evt.subagent_role || '').trim();
const role = taskKey(evt.subagent_role);
setSubagentParent(childId, { parentId, role, model: evt.model });
// Worker narration carries subagent_event="progress" too. It is activity,
// not a lifecycle row: route it through the progress key so the later
@ -2571,7 +2578,7 @@ export function createChatInstance({
}
function routeSubagentFinalMessageToCard(taskId, msg) {
const childId = String(taskId || '').trim();
const childId = taskKey(taskId);
const info = subagentChildParents.get(childId);
if (!childId || !info) return false;
const { parentId, role, model } = info;
@ -3984,7 +3991,7 @@ export function createChatInstance({
}
function showTyping(activityId = '', meta = {}) {
const actId = String(activityId || '').trim() || ('direct-' + chatId);
const actId = taskKey(activityId) || ('direct-' + chatId);
// A typing frame after its turn's keyed final must not resurrect the
// concluded turn — but it still carries the activity<->cmid link, so
// it settles the linked submission (broadcasts are not ordered).
@ -4050,7 +4057,7 @@ export function createChatInstance({
if (!currentRecord || currentRecord.isSubagent || subagentChildParents.has(taskId)) return;
attachTaskDetailReviews(taskId, detail);
const cancelPending = taskCancelPending(detail);
if (cancelPending || String(detail?.status || '').trim()) {
if (cancelPending || taskKey(detail?.status)) {
markReviewAnchor(currentRecord);
}
if (cancelPending) {
@ -4075,7 +4082,7 @@ export function createChatInstance({
}
function observeMissingManagedTask(taskId) {
const id = String(taskId || '').trim();
const id = taskKey(taskId);
if (!id || concludedDirectActivities.has(id)) return;
const record = liveCardRecords.get(id);
if (subagentChildParents.has(id) || record?.isSubagent) return;

View file

@ -2,6 +2,7 @@ function reviewNodeKey(node) {
const dataset = node?.dataset || {};
if (Object.hasOwn(dataset, 'reviewSection')) return 'section';
if (Object.hasOwn(dataset, 'reviewSectionToggle')) return 'section-toggle';
if (Object.hasOwn(dataset, 'reviewHydrateStatus')) return 'hydrate-status';
if (dataset.reviewGroup) return `group:${dataset.reviewGroup}`;
if (dataset.reviewGroupToggle) return `group-toggle:${dataset.reviewGroupToggle}`;
if (dataset.reviewAttempt) return `attempt:${dataset.reviewAttempt}`;

View file

@ -461,6 +461,66 @@ function currentPlanAttempt(current, index) {
};
}
function planFindingLines(wave) {
const findings = (Array.isArray(wave.findings) ? wave.findings : [])
.filter((item) => item && typeof item === 'object');
const dispositions = (Array.isArray(wave.dispositions) ? wave.dispositions : [])
.filter((item) => item && typeof item === 'object');
// EVERY disposition row renders: the backend refuses duplicate rows for
// one finding as contradictory intent and keeps the finding open, so
// showing only the first would present a refused decision as operative.
const dispositionsByFinding = new Map();
for (const disposition of dispositions) {
const key = text(disposition.finding_id);
const bucket = dispositionsByFinding.get(key) || [];
bucket.push(disposition);
dispositionsByFinding.set(key, bucket);
}
const dispositionLine = (disposition, prefix) => (
`${prefix}${text(disposition.decision) || 'disposition'}${text(disposition.rationale) ? ` — ${text(disposition.rationale)}` : ''}`
);
const lines = [];
for (const finding of findings) {
const head = [
`[${text(finding.class) || 'finding'}] ${text(finding.summary) || '(no summary)'}`,
text(finding.breaks) ? `breaks ${text(finding.breaks)}` : '',
text(finding.locator) ? `at ${text(finding.locator)}` : '',
].filter(Boolean).join(' — ');
const source = [text(finding.slot), text(finding.model)].filter(Boolean).join(' · ');
lines.push(source ? `${head} — ${source}` : head);
if (text(finding.recommendation)) lines.push(` fix: ${text(finding.recommendation)}`);
const findingId = text(finding.finding_id);
if (findingId) {
for (const disposition of dispositionsByFinding.get(findingId) || []) {
lines.push(dispositionLine(disposition, ' agent: '));
}
dispositionsByFinding.delete(findingId);
}
}
if (dispositionsByFinding.size) {
lines.push('General dispositions:');
for (const [findingId, bucket] of dispositionsByFinding) {
for (const disposition of bucket) {
lines.push(dispositionLine(disposition, ` ${findingId || '(no finding id)'}: `));
}
}
}
return lines;
}
function planActorAvailabilityLines(wave) {
// The bug report's own bar: a result that was never received must say so
// explicitly instead of contributing silently-zero findings.
const lines = [];
for (const actor of (Array.isArray(wave.actors) ? wave.actors : [])) {
if (!actor || typeof actor !== 'object' || actor.ok !== false) continue;
const identity = [text(actor.slot_id), text(actor.model)].filter(Boolean).join(' · ') || 'reviewer';
const cause = text(actor.failure_code) || text(actor.error) || 'no parseable verdict';
lines.push(`Reviewer unavailable: ${identity} — ${cause}`);
}
return lines;
}
function planWaveDetail(wave) {
const lines = [
wave.aggregate ? `Verdict: ${wave.aggregate}` : '',
@ -468,8 +528,36 @@ function planWaveDetail(wave) {
wave.paid != null ? `Reviewer panel dispatched: ${wave.paid ? 'yes' : 'no'}` : '',
wave.quorum_unreachable ? 'Quorum unavailable' : '',
wave.cycles_exhausted ? 'Review cycles exhausted' : '',
wave.reason ? `Reason: ${wave.reason}` : '',
wave.reason ? `Reason: ${text(wave.reason)}` : '',
];
const counts = wave.counts && typeof wave.counts === 'object' ? wave.counts : {};
if (wave.compact) {
// A compacted wave keeps counts while its finding bodies moved to the
// immutable wave artifact; name that remainder instead of rendering a
// bound that looks like the whole record.
const recorded = [
finiteCount(counts.findings) != null ? `${finiteCount(counts.findings)} findings` : '',
finiteCount(counts.blocking) != null ? `${finiteCount(counts.blocking)} blocking` : '',
finiteCount(counts.dispositions) != null ? `${finiteCount(counts.dispositions)} dispositions` : '',
].filter(Boolean).join(' · ');
if (recorded) lines.push(`Recorded: ${recorded}`);
const artifact = wave.wave_artifact && typeof wave.wave_artifact === 'object' ? wave.wave_artifact : {};
const sha = text(artifact.sha256);
lines.push(`Finding bodies compacted${sha ? ` · artifact sha256=${sha.slice(0, 12)}…` : ''}${finiteCount(artifact.bytes) != null ? ` (${finiteCount(artifact.bytes)} bytes)` : ''}`);
return lines.filter(Boolean).join('\n');
}
const countParts = ['blocking', 'note', 'need_evidence']
.filter((key) => finiteCount(counts[key]) != null)
.map((key) => `${finiteCount(counts[key])} ${key}`);
if (countParts.length) lines.push(`Findings: ${countParts.join(' · ')}`);
lines.push(...planFindingLines(wave));
lines.push(...planActorAvailabilityLines(wave));
const findingsShown = (Array.isArray(wave.findings) ? wave.findings : []).length;
if (wave.findings_paged && finiteCount(wave.findings_total) != null) {
lines.push(`Showing ${findingsShown} of ${finiteCount(wave.findings_total)} findings (per-slot page cap)`);
}
if (wave.findings_texts_truncated) lines.push('Some finding texts were truncated at capture.');
if (wave.spec_body_truncated) lines.push('Spec body was truncated at capture.');
return lines.filter(Boolean).join('\n');
}
@ -642,12 +730,39 @@ export function formatReviewProjection(projection) {
if (binding.length) lines.push(`Panel binding: ${binding.join(' · ')}`);
(Array.isArray(panel.actors) ? panel.actors : []).forEach((actor) => {
if (!actor || typeof actor !== 'object') return;
const slotId = String(actor.slot_id || '?');
lines.push(
`Reviewer ${String(actor.slot_id || '?')}: role=${String(actor.actor_role || 'reviewer')} · provider=${String(actor.provider || 'unknown')} · model=${String(actor.model || 'unknown')} · transport=${String(actor.transport_status || 'unknown')} · parse=${String(actor.parse_status || 'unknown')} · verdict=${String(actor.semantic_verdict || 'none')}${actor.outcome_tier ? ` · outcome_tier=${String(actor.outcome_tier)}` : ''}${actor.dialogue_status ? ` · dialogue=${String(actor.dialogue_status)}` : ''} · quorum=${actor.quorum_contribution ? 'contributes' : 'abstains'} · enforcement=${String(actor.enforcement_impact || 'unknown')}`,
`Reviewer ${slotId}: role=${String(actor.actor_role || 'reviewer')} · provider=${String(actor.provider || 'unknown')} · model=${String(actor.model || 'unknown')} · transport=${String(actor.transport_status || 'unknown')} · parse=${String(actor.parse_status || 'unknown')} · verdict=${String(actor.semantic_verdict || 'none')}${actor.outcome_tier ? ` · outcome_tier=${String(actor.outcome_tier)}` : ''}${actor.dialogue_status ? ` · dialogue=${String(actor.dialogue_status)}` : ''} · quorum=${actor.quorum_contribution ? 'contributes' : 'abstains'} · enforcement=${String(actor.enforcement_impact || 'unknown')}`,
);
const actorCoverage = compactCoverage(actor.coverage);
if (actorCoverage) lines.push(`Reviewer ${String(actor.slot_id || '?')} coverage: ${actorCoverage}`);
if (actor.reason) lines.push(`Reviewer ${String(actor.slot_id || '?')} reason: ${String(actor.reason)}`);
if (actorCoverage) lines.push(`Reviewer ${slotId} coverage: ${actorCoverage}`);
if (actor.reason) lines.push(`Reviewer ${slotId} reason: ${String(actor.reason)}`);
if (Array.isArray(actor.findings)) {
for (const finding of actor.findings) {
if (!finding || typeof finding !== 'object') continue;
const label = [text(finding.severity), text(finding.verdict)]
.filter(Boolean).join(' ') || 'finding';
const title = text(finding.item) || text(finding.summary) || '(no item)';
const summaryText = text(finding.summary);
const body = [
`[${label}]${text(finding.id) ? ` ${text(finding.id)}` : ''} ${title}`,
summaryText && summaryText !== title ? `summary: ${summaryText}` : '',
text(finding.reason) ? `reason: ${text(finding.reason)}` : '',
text(finding.evidence) ? `evidence: ${text(finding.evidence)}` : '',
text(finding.recommendation) ? `fix: ${text(finding.recommendation)}` : '',
].filter(Boolean).join(' — ');
lines.push(`Reviewer ${slotId} finding: ${body}`);
}
const omitted = finiteCount(actor.findings_omitted);
if (omitted) lines.push(`Reviewer ${slotId} findings omitted: ${omitted}`);
}
// P1: name the durable full copy unconditionally — bounded rows,
// per-string truncation markers and pre-findings-era projections
// all resolve through the same observability call.
const callId = text(actor.response_ref?.call_id);
if (callId) {
lines.push(`Reviewer ${slotId} full response: observability call ${callId}`);
}
});
});
return lines.join('\n');
@ -954,20 +1069,38 @@ function reviewRevision(value) {
* not counters. A distinct token arriving during a GET schedules one trailing
* read; an identical applied/in-flight/pending token is a no-op/same flight.
*/
export function createReviewHydrator({ fetchDetail, applyDetail } = {}) {
export function createReviewHydrator({ fetchDetail, applyDetail, onState = () => {} } = {}) {
const states = new Map();
const start = (taskId, state, revision) => {
const generation = ++state.generation;
state.inFlightRevision = revision;
// Status is typed presentation state, not a per-attempt DOM FSM: a
// first load (or a retry after a failure) announces itself; routine
// background refreshes over already-applied content stay silent.
const notify = (status) => {
state.lastStatus = status;
onState(taskId, status);
};
if (!state.everApplied || state.lastStatus === 'error') notify('loading');
const request = Promise.resolve()
.then(() => fetchDetail(taskId))
.then((detail) => applyDetail(taskId, detail))
.then((detail) => {
// A strict fetch seam REJECTS on a failed read; a null detail
// means the record is genuinely absent (404) — not an error.
if (detail === null || detail === undefined) return false;
return applyDetail(taskId, detail);
})
.then((applied) => {
if (applied !== false && revision !== null) state.appliedRevision = revision;
state.everApplied = true;
notify('idle');
return applied;
})
.catch(() => false)
.catch(() => {
notify('error');
return false;
})
.finally(() => {
if (state.inFlight !== request || state.generation !== generation) return;
state.inFlight = null;
@ -993,6 +1126,8 @@ export function createReviewHydrator({ fetchDetail, applyDetail } = {}) {
pendingRevision: null,
inFlight: null,
generation: 0,
everApplied: false,
lastStatus: 'idle',
};
states.set(taskId, state);
}
@ -1108,12 +1243,24 @@ export function reviewReferenceFromRow(row) {
export function renderReviewsSection(groupsInput, disclosure = {}) {
const groups = orderedReviewGroups(groupsInput);
if (!groups.length) return '';
// A failed FIRST hydration has no groups to hang the error on: the shell
// renders anyway and stays mounted through the retry's own loading pass
// (hadHydrateError) so the recovery control cannot unmount mid-flight. A
// quiet first-load zero-group loading pass stays invisible (every card
// expand hydrates, most tasks have no reviews).
const hydrateStatus = text(disclosure.hydrateStatus);
const emptyShell = !groups.length && (
hydrateStatus === 'error'
|| (hydrateStatus === 'loading' && disclosure.hadHydrateError === true)
);
if (!groups.length && !emptyShell) return '';
const expandedGroups = disclosure.expandedGroups instanceof Set ? disclosure.expandedGroups : new Set();
const expandedAttempts = disclosure.expandedAttempts instanceof Set ? disclosure.expandedAttempts : new Set();
const sectionExpanded = disclosure.sectionExpanded === true;
const { groupCount, activeCount } = reviewGroupCounts(groups);
const countText = `${groupCount}${activeCount ? ` · ${activeCount} active` : ''}`;
const countText = groupCount
? `${groupCount}${activeCount ? ` · ${activeCount} active` : ''}`
: '—';
const groupHtml = groups.map((group) => {
const groupExpanded = expandedGroups.has(group.id);
const shown = group.countIsAuthoritative ? `${group.attemptCount}` : `${group.attempts.length} shown`;
@ -1174,12 +1321,27 @@ export function renderReviewsSection(groupsInput, disclosure = {}) {
</div>
</div>`;
}).join('');
// Section-level hydration truth (typed controller state, no per-attempt
// FSM): a first load announces itself, a failed refresh names itself and
// offers Retry instead of silently presenting stale-or-missing detail.
// The message rides inside a <span>: the keyed status node is patched in
// place across loading↔error, and the DOM patcher syncs text only through
// childless-element innerHTML — a bare text node beside the Retry button
// would survive the transition stale.
const hydrateHtml = hydrateStatus === 'loading'
? '<div class="skill-review-loading" data-review-hydrate-status role="status" aria-live="polite"><span>Loading review details…</span></div>'
: (hydrateStatus === 'error'
? '<div class="skill-review-error" data-review-hydrate-status role="alert"><span>Review details failed to refresh — shown data may be incomplete. </span><button type="button" class="skill-review-retry" data-review-hydrate-retry>Retry</button></div>'
: '');
// The status node sits OUTSIDE the collapsible groups container: a failed
// refresh stays visible on a collapsed section and the keyed node survives
// loading↔error transitions. Disclosure stays user-owned — nothing expands.
return `
<section class="chat-live-reviews" data-review-section data-expanded="${sectionExpanded ? '1' : '0'}">
<button type="button" class="chat-review-section-toggle" data-review-section-toggle aria-expanded="${sectionExpanded ? 'true' : 'false'}">
<span>Reviews</span><span class="chat-review-section-count">${escapeHtmlText(countText)}</span>
</button>
<div class="chat-review-groups"${sectionExpanded ? '' : ' hidden'}>${groupHtml}</div>
${hydrateHtml}<div class="chat-review-groups"${sectionExpanded ? '' : ' hidden'}>${groupHtml}</div>
</section>`;
}
@ -1233,10 +1395,14 @@ export function createReviewPresentationController({
if (!host || !summary) return;
const focused = focusedControl();
const { groupCount, activeCount } = reviewGroupCounts(groups);
summary.hidden = groupCount === 0;
const failedEmpty = groupCount === 0 && (
state.hydrateStatus === 'error'
|| (state.hydrateStatus === 'loading' && state.hadHydrateError === true)
);
summary.hidden = groupCount === 0 && !failedEmpty;
summary.textContent = groupCount
? `Reviews ${groupCount}${activeCount ? ` · ${activeCount} active` : ''}`
: '';
: (failedEmpty ? 'Reviews' : '');
const reconciled = reconcileReviewMarkup(host, renderReviewsSection(groups, state));
const active = host?.ownerDocument?.activeElement;
if (!reconciled || !active || !host.contains?.(active)) restoreFocus(focused);
@ -1250,6 +1416,19 @@ export function createReviewPresentationController({
};
host?.addEventListener('click', (event) => {
const hydrateRetry = event.target?.closest?.('[data-review-hydrate-retry]');
if (hydrateRetry) {
// A failed pass never records an applied revision, so a plain
// revision-less re-hydrate always re-issues the physical GET.
onHydrate();
const status = host.querySelector?.('[data-review-hydrate-status]');
if (status) {
status.setAttribute?.('tabindex', '-1');
status.focus?.();
}
onLayout();
return;
}
const retry = event.target?.closest?.('[data-skill-review-retry]');
if (retry) {
const detail = retry.closest?.('[data-review-attempt-detail]');
@ -1307,5 +1486,15 @@ export function createReviewPresentationController({
if (changed) render();
return changed;
},
setHydrateStatus(statusValue) {
const status = text(statusValue);
if (state.hydrateStatus === status) return;
// Remember a failure across the retry's loading pass so the
// zero-group shell stays mounted until the retry settles.
if (status === 'error') state.hadHydrateError = true;
else if (status === 'idle') state.hadHydrateError = false;
state.hydrateStatus = status;
render();
},
};
}

View file

@ -177,7 +177,10 @@ function reviewRunTitle(run) {
function reviewHistory(skill) {
const review = skill.skill_review && typeof skill.skill_review === 'object'
? skill.skill_review : {};
const history = Array.isArray(review.history) ? review.history.slice(-10) : [];
// The backend projection already bounds history to its ten-row window and
// discloses the exact omitted count; a second client-side slice would be
// an undisclosed bound on top of a disclosed one.
const history = Array.isArray(review.history) ? review.history : [];
const current = review.current && Object.keys(review.current).length
? review.current : history[history.length - 1];
if (!current) return '';
@ -186,9 +189,13 @@ function reviewHistory(skill) {
const source = run.source ? ` · ${run.source}` : '';
return `<li>${escapeHtml(reviewRunTitle(run))} · ${escapeHtml(status)}${escapeHtml(source)}</li>`;
}).join('');
const omitted = Number(review.history_omitted);
const historyLabel = Number.isFinite(omitted) && omitted > 0
? `${history.length} of ${history.length + omitted}`
: `${history.length}`;
const currentStatus = current.review_status || current.status || current.job_status || 'unknown';
return `<div class="skills-review-current"><strong>${escapeHtml(reviewRunTitle(current))}</strong> · ${escapeHtml(currentStatus)}</div>
${rows ? `<details class="skills-review-history ui-rich-content"><summary class="muted">Skill Review history (${history.length})</summary><ol>${rows}</ol></details>` : ''}`;
${rows ? `<details class="skills-review-history ui-rich-content"><summary class="muted">Skill Review history (${historyLabel})</summary><ol>${rows}</ol></details>` : ''}`;
}
function grantBlock(skill) {

View file

@ -99,7 +99,8 @@ export function topReviewFinding(entity) {
const label = first.item || first.check || first.title || 'finding';
const verdict = first.verdict || first.severity || '';
const reason = first.reason || first.message || '';
return `${verdict ? `${verdict} ` : ''}${label}: ${reason}`.trim();
const more = findings.length > 1 ? ` (+${findings.length - 1} more)` : '';
return `${`${verdict ? `${verdict} ` : ''}${label}: ${reason}`.trim()}${more}`;
}
export function renderHubCard(item, {

View file

@ -0,0 +1,20 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import { topReviewFinding } from '../modules/utils.js';
test('the compact review hint discloses how many further findings it hides', () => {
const one = topReviewFinding({ review_findings: [
{ verdict: 'FAIL', item: 'writes outside payload', reason: 'escapes the bucket' },
] });
assert.equal(one, 'FAIL writes outside payload: escapes the bucket');
const three = topReviewFinding({ review_findings: [
{ verdict: 'FAIL', item: 'writes outside payload', reason: 'escapes the bucket' },
{ verdict: 'WARN', item: 'b', reason: 'r' },
{ verdict: 'WARN', item: 'c', reason: 'r' },
] });
assert.equal(three, 'FAIL writes outside payload: escapes the bucket (+2 more)');
assert.equal(topReviewFinding({ review_findings: [] }), '');
});

View file

@ -0,0 +1,290 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import {
createReviewHydrator,
createReviewPresentationController,
planReviewGroupFromTaskDetail,
renderReviewsSection,
taskAcceptanceGroupFromTaskDetail,
} from '../modules/review_presentation.js';
import { reconcileReviewElementTree } from '../modules/review_dom_patch.js';
test('a full Plan wave renders findings, mapped dispositions, degraded reviewers and honesty stamps', () => {
const fingerprint = 'd'.repeat(64);
const group = planReviewGroupFromTaskDetail({
task_id: 'root',
plan_review_state: {
schema_version: 2,
current_attempt: { fingerprint, status: 'open' },
waves: [{
request_fingerprint: fingerprint,
cycle_index: 2,
aggregate: 'REVISE_PLAN',
closed: false,
paid: true,
reason: 'blocking_slots_at_quorum:2/3',
counts: { blocking: 1, note: 1, need_evidence: 0 },
findings: [
{
finding_id: 'slot_1:f1', id: 'f1', class: 'blocking',
summary: 'The rollback path loses the stash', breaks: 'invariant_2',
locator: 'supervisor/update_merge.py',
recommendation: 'Restore the stash before reset',
slot: 'slot_1', model: 'anthropic/claude-opus-5',
},
{
finding_id: 'slot_2:f1', id: 'f1', class: 'note',
summary: 'Naming could be clearer', slot: 'slot_2',
},
],
dispositions: [
{ finding_id: 'slot_1:f1', decision: 'reject', rationale: 'stash is restored by boot finalize' },
{ finding_id: 'slot_1:f1', decision: 'accept', rationale: 'second thoughts after re-reading' },
{ finding_id: 'slot_9:gone', decision: 'accept', rationale: 'will fold into phase 2' },
],
actors: [
{ slot_id: 'slot_1', model: 'anthropic/claude-opus-5', ok: true },
{ slot_id: 'slot_3', model: 'openai/gpt-5.6-sol', ok: false, failure_code: 'window_exhausted' },
],
findings_paged: true,
findings_total: 40,
findings_texts_truncated: true,
spec_body_truncated: true,
}],
waves_omitted: 0,
},
});
const detail = group.attempts[0].detailText;
assert.match(detail, /Findings: 1 blocking · 1 note · 0 need_evidence/);
assert.match(detail, /\[blocking\] The rollback path loses the stash — breaks invariant_2 — at supervisor\/update_merge\.py — slot_1 · anthropic\/claude-opus-5/);
assert.match(detail, / fix: Restore the stash before reset/);
assert.match(detail, / agent: reject — stash is restored by boot finalize/);
// Contradictory duplicate dispositions both render: the backend refuses
// the pair and keeps the finding open, so hiding one would present the
// other as operative.
assert.match(detail, / agent: accept — second thoughts after re-reading/);
assert.match(detail, /\[note\] Naming could be clearer — slot_2/);
assert.match(detail, /General dispositions:\n slot_9:gone: accept — will fold into phase 2/);
assert.match(detail, /Reviewer unavailable: slot_3 · openai\/gpt-5\.6-sol — window_exhausted/);
assert.match(detail, /Showing 2 of 40 findings \(per-slot page cap\)/);
assert.match(detail, /Some finding texts were truncated at capture\./);
assert.match(detail, /Spec body was truncated at capture\./);
assert.match(detail, /Cost unavailable/);
// The rendered section carries the finding text to the reader.
const html = renderReviewsSection([group], {
sectionExpanded: true,
expandedGroups: new Set(['plan:root']),
expandedAttempts: new Set([`plan:root:${group.attempts[0].id}`]),
});
assert.match(html, /The rollback path loses the stash/);
});
test('a compact Plan wave names its recorded counts and the immutable artifact remainder', () => {
const fingerprint = 'e'.repeat(64);
const group = planReviewGroupFromTaskDetail({
task_id: 'root',
plan_review_state: {
schema_version: 2,
current_attempt: {},
waves: [{
compact: true,
request_fingerprint: fingerprint,
cycle_index: 1,
aggregate: 'GREEN',
closed: true,
counts: { findings: 5, dispositions: 3, blocking: 2 },
wave_artifact: { root: 'artifact_store', path: 'w.json', sha256: 'abc123def4567890', bytes: 321 },
}],
waves_omitted: 0,
},
});
const detail = group.attempts[0].detailText;
assert.match(detail, /Recorded: 5 findings · 2 blocking · 3 dispositions/);
assert.match(detail, /Finding bodies compacted · artifact sha256=abc123def456… \(321 bytes\)/);
assert.doesNotMatch(detail, /w\.json/);
});
test('the hydrator announces first load, failure and retry without narrating background refreshes', async () => {
const events = [];
let mode = 'ok';
const hydrator = createReviewHydrator({
fetchDetail: async () => {
if (mode === 'reject') throw new Error('transport failed');
return mode === 'missing' ? null : { ok: true };
},
applyDetail: () => true,
onState: (taskId, status) => events.push(`${taskId}:${status}`),
});
mode = 'reject';
assert.equal(await hydrator.hydrate('root', 'a'.repeat(64)), false);
assert.deepEqual(events, ['root:loading', 'root:error']);
events.length = 0;
mode = 'ok';
await hydrator.hydrate('root', 'a'.repeat(64));
assert.deepEqual(
events, ['root:loading', 'root:idle'],
'a plain re-hydrate after failure re-fetches (no applied receipt was recorded) and announces',
);
events.length = 0;
await hydrator.hydrate('root', 'b'.repeat(64));
assert.deepEqual(events, ['root:idle'], 'a background refresh over applied content stays silent');
events.length = 0;
mode = 'missing';
await hydrator.hydrate('gone', 'c'.repeat(64));
assert.deepEqual(
events, ['gone:loading', 'gone:idle'],
'a genuinely absent record (404 → null) is not an error',
);
});
test('the section renders hydration truth and the controller retries through onHydrate', () => {
const errorHtml = renderReviewsSection([
taskAcceptanceGroupFromTaskDetail({
task_id: 'root',
review_projection: { panels: [{ surface: 'task_acceptance', panel_id: 'p1', aggregate_signal: 'PASS' }] },
}, 'root'),
], { sectionExpanded: true, hydrateStatus: 'error' });
assert.match(errorHtml, /data-review-hydrate-status/);
assert.match(errorHtml, /role="alert"/);
assert.match(errorHtml, /<span>Review details failed to refresh/);
assert.match(errorHtml, /data-review-hydrate-retry/);
const loadingHtml = renderReviewsSection([
taskAcceptanceGroupFromTaskDetail({
task_id: 'root',
review_projection: { panels: [{ surface: 'task_acceptance', panel_id: 'p1', aggregate_signal: 'PASS' }] },
}, 'root'),
], { sectionExpanded: true, hydrateStatus: 'loading' });
assert.match(loadingHtml, /Loading review details…/);
const hydrated = [];
let clickHandler = null;
const statusNode = {
setAttribute(key, value) { this[key] = value; },
focus() { this.focused = true; },
};
const controller = createReviewPresentationController({
host: {
addEventListener: (_type, handler) => { clickHandler = handler; },
querySelector: (selector) => (selector === '[data-review-hydrate-status]' ? statusNode : null),
},
summary: null,
onHydrate: (...args) => hydrated.push(args),
});
controller.setHydrateStatus('error');
clickHandler({
target: {
closest: (selector) => (selector === '[data-review-hydrate-retry]' ? {} : null),
},
});
assert.deepEqual(hydrated, [[]]);
assert.equal(statusNode.tabindex, '-1');
assert.equal(statusNode.focused, true);
});
test('a failed first hydration renders the section shell with no groups', () => {
assert.equal(renderReviewsSection([], {}), '');
assert.equal(renderReviewsSection([], { hydrateStatus: 'loading' }), '',
'a quiet first-load zero-group loading pass stays invisible (every card expand hydrates)');
const errorShell = renderReviewsSection([], { sectionExpanded: true, hydrateStatus: 'error' });
assert.match(errorShell, /data-review-section/);
assert.match(errorShell, /Review details failed to refresh/);
assert.match(errorShell, /data-review-hydrate-retry/);
assert.match(errorShell, /chat-review-section-count">—</);
// The error must be readable on a COLLAPSED section too: the status node
// sits outside the hidden groups container.
const collapsedShell = renderReviewsSection([], { hydrateStatus: 'error' });
assert.match(collapsedShell, /data-expanded="0"/);
const statusIndex = collapsedShell.indexOf('data-review-hydrate-status');
const groupsIndex = collapsedShell.indexOf('chat-review-groups');
assert.ok(statusIndex >= 0 && statusIndex < groupsIndex,
'status node renders before (outside) the collapsible groups container');
assert.match(collapsedShell, /Review details failed to refresh/);
// A retry's own loading pass keeps the shell mounted (hadHydrateError),
// so the recovery control cannot unmount mid-flight.
const retryLoading = renderReviewsSection([], { hydrateStatus: 'loading', hadHydrateError: true });
assert.match(retryLoading, /Loading review details…/);
assert.equal(renderReviewsSection([], { hydrateStatus: 'loading', hadHydrateError: false }), '');
});
test('the hydrate status node swaps its message text across the loading→error patch', () => {
class Node {
constructor({ tag = 'div', dataset = {}, classes = [], attrs = {}, html = '', children = [] } = {}) {
this.tagName = tag.toUpperCase();
this.dataset = { ...dataset };
this._attrs = new Map(Object.entries(attrs));
for (const [key, value] of Object.entries(dataset)) {
this._attrs.set(`data-${key.replace(/[A-Z]/g, (c) => `-${c.toLowerCase()}`)}`, String(value));
}
this._classes = new Set(classes);
if (classes.length) this._attrs.set('class', classes.join(' '));
this.classList = { contains: (name) => this._classes.has(name) };
this._innerHTML = html;
this.children = [];
children.forEach((child) => this.insertBefore(child, null));
}
get attributes() { return [...this._attrs].map(([name, value]) => ({ name, value })); }
get innerHTML() { return this._innerHTML; }
set innerHTML(value) { this._innerHTML = String(value); this.children = []; }
hasAttribute(name) { return this._attrs.has(name); }
setAttribute(name, value) {
this._attrs.set(name, String(value));
if (name.startsWith('data-')) {
this.dataset[name.slice(5).replace(/-([a-z])/g, (_a, c) => c.toUpperCase())] = String(value);
}
}
removeAttribute(name) { this._attrs.delete(name); }
insertBefore(child, before) {
child.remove();
const index = before ? this.children.indexOf(before) : -1;
if (index >= 0) this.children.splice(index, 0, child); else this.children.push(child);
child.parentElement = this;
return child;
}
remove() {
if (!this.parentElement) return;
const index = this.parentElement.children.indexOf(this);
if (index >= 0) this.parentElement.children.splice(index, 1);
this.parentElement = null;
}
cloneNode(deep) {
return new Node({
tag: this.tagName,
dataset: this.dataset,
classes: [...this._classes],
attrs: Object.fromEntries(this._attrs),
html: this._innerHTML,
children: deep ? this.children.map((child) => child.cloneNode(true)) : [],
});
}
}
const statusNode = (state) => new Node({
dataset: { reviewHydrateStatus: '' },
classes: [state === 'error' ? 'skill-review-error' : 'skill-review-loading'],
children: state === 'error'
? [
new Node({ tag: 'span', html: 'Review details failed to refresh — shown data may be incomplete. ' }),
new Node({ tag: 'button', dataset: { reviewHydrateRetry: '' }, html: 'Retry' }),
]
: [new Node({ tag: 'span', html: 'Loading review details…' })],
});
const current = statusNode('loading');
assert.equal(reconcileReviewElementTree(current, statusNode('error')), true);
assert.match(current.children[0].innerHTML, /failed to refresh/);
assert.doesNotMatch(current.children[0].innerHTML, /Loading review details/);
assert.equal(current.children.length, 2);
assert.equal(current.children[1].innerHTML, 'Retry');
assert.equal(reconcileReviewElementTree(current, statusNode('loading')), true);
assert.match(current.children[0].innerHTML, /Loading review details/);
assert.equal(current.children.length, 1);
});

View file

@ -448,3 +448,50 @@ test('an evidence-bearing chip is marked so sticky rendering can refuse downgrad
execution_evidence: { delegated_runs_started: 1, delegated_runs_settled: 1, delegated_runs_failed: 0, subscription_cost_usd: null },
}).hasEvidence, true);
});
test('actor findings rows, omitted counts and the durable pointer ride the shared formatter', () => {
const text = formatReviewProjection({ panels: [{
panel_id: 'panel_f', surface: 'task_acceptance', aggregate_signal: 'FAIL',
quorum: { required: 2, contributed: 2, configured: 3 },
actors: [
{
slot_id: 'slot_1', model: 'm1', reason: 'summary text',
coverage: { criteria_total: 3, findings: 10 },
findings: [
{
id: 'f1', severity: 'critical', item: 'Preflop sizing is wrong',
summary: 'Sizing deviates from the baseline in early position',
evidence: 'raises 2bb from UTG', recommendation: 'raise 2.5bb',
},
{ severity: 'low', verdict: 'FAIL', item: 'Style nit', reason: 'inconsistent spacing' },
],
findings_omitted: 8,
response_ref: { call_id: 'review_task_acceptance_slot_1_resp', sha256: 'a'.repeat(64) },
},
{
slot_id: 'slot_2', model: 'm2',
coverage: { criteria_total: 3, findings: 0 },
findings: [], findings_omitted: 0,
response_ref: { call_id: 'c2' },
},
{
// A pre-findings-era projection: count says findings existed,
// rows are absent — the pointer is the only resolvable source.
slot_id: 'slot_3', model: 'm3',
coverage: { criteria_total: 3, findings: 4 },
response_ref: { call_id: 'c3' },
},
],
}] });
assert.match(text, /Reviewer slot_1 finding: \[critical\] f1 Preflop sizing is wrong — summary: Sizing deviates from the baseline in early position — evidence: raises 2bb from UTG — fix: raise 2\.5bb/);
assert.match(text, /Reviewer slot_1 finding: \[low FAIL\] Style nit — reason: inconsistent spacing/);
assert.match(text, /Reviewer slot_1 findings omitted: 8/);
assert.match(text, /Reviewer slot_1 full response: observability call review_task_acceptance_slot_1_resp/);
assert.doesNotMatch(text, /Reviewer slot_2 finding:/);
// The durable pointer is unconditional: bounded rows, per-string
// truncation markers and pre-findings-era projections all resolve there.
assert.match(text, /Reviewer slot_2 full response: observability call c2/);
assert.match(text, /Reviewer slot_3 full response: observability call c3/);
assert.doesNotMatch(text, /Reviewer slot_3 finding:/);
});

View file

@ -3,7 +3,7 @@ import test from 'node:test';
import { renderInstalledSkillCard } from '../modules/skill_card_renderer.js';
test('skill card shows current review round and collapses only the last ten group rows', () => {
test('skill card shows the current review round over the runner-bounded ten-row window', () => {
const history = Array.from({ length: 10 }, (_, idx) => ({
status: 'clean',
content_hash: `snapshot-${idx}`,
@ -38,3 +38,34 @@ test('skill card shows current review round and collapses only the last ten grou
assert.match(html, /Skill Review history \(10\)/);
assert.doesNotMatch(html, /must stay private/);
});
test('a bounded history window names the exact omitted remainder in its label', () => {
const history = Array.from({ length: 10 }, (_, idx) => ({
status: 'clean',
content_hash: `snapshot-${idx}`,
group_id: 'manual:alpha',
review_round: idx + 15,
snapshot_attempt: 1,
}));
const skill = {
name: 'alpha',
type: 'instruction',
version: '1.0.0',
description: 'test',
source: 'external',
enabled: false,
review_status: 'clean',
review_gate: { executable_review: true },
review_findings: [],
permissions: [],
grants: {},
skill_review: {
current: history[history.length - 1],
history,
history_omitted: 14,
},
};
const html = renderInstalledSkillCard(skill);
assert.match(html, /Skill Review history \(10 of 24\)/);
});