Skip to content

feat(sight): add grader dashboard controls#1396

Open
haoowa wants to merge 2 commits into
alibaba:mainfrom
haoowa:feat/sight/grader-dashboard
Open

feat(sight): add grader dashboard controls#1396
haoowa wants to merge 2 commits into
alibaba:mainfrom
haoowa:feat/sight/grader-dashboard

Conversation

@haoowa

@haoowa haoowa commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

Add dashboard controls for the conversation grader. The UI can run manual evaluations, show latest-run badges in the conversation list, and display score dimensions, findings, root cause, and evidence links in a detail panel.

This PR is frontend-only and split from the grader backend so the dashboard interaction surface can be reviewed separately.

Related Issue

Refs #1304

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional change)
  • Performance improvement
  • CI/CD or build changes

Scope

  • cosh (copilot-shell)
  • cosh-ng (cosh-ng)
  • sec-core (agent-sec-core)
  • skill (os-skills)
  • sight (agentsight)
  • tokenless (tokenless)
  • ckpt (ws-ckpt)
  • memory (agent-memory)
  • anolisa (anolisa-cli)
  • skillfs (SkillFS)
  • Multiple / Project-wide

Checklist

  • I have read the Contributing Guide
  • My code follows the project's code style
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation accordingly
  • For cosh: Lint passes, type check passes, and tests pass
  • For cosh-ng: cargo clippy --all-targets -- -D warnings and cargo fmt --check pass
  • For sec-core (Rust): cargo clippy -- -D warnings and cargo fmt --check pass
  • For sec-core (Python): Ruff format and pytest pass
  • For skill: Skill directory structure is valid and shell scripts pass syntax check
  • For sight: dashboard typecheck and tests pass
  • For tokenless: cargo clippy -- -D warnings and cargo fmt --check pass
  • For memory (Linux only): cargo clippy --all-targets -- -D warnings, cargo fmt --check, and cargo test pass
  • For anolisa: cargo clippy --all-targets --locked -- -D warnings, cargo fmt --all --check, and cargo test --locked pass
  • For skillfs: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, and cargo test --workspace pass
  • Lock files are up to date (package-lock.json / Cargo.lock)

Testing

Ran on the Linux AgentSight test host from an archive of this split branch:

  • npm ci
  • npm run typecheck
  • npm test -- src/test/apiClient.test.ts src/test/EvaluationBadge.test.tsx src/test/EvaluationPanel.test.tsx src/test/ConversationList.test.tsx src/test/AtifViewerPage.test.tsx

Result: 5 test files passed, 95 tests passed.

Additional Notes

Split from #1349 following review feedback. The backend API is intentionally left to a separate PR.

@haoowa haoowa requested review from chengshuyi and jfeng18 as code owners July 8, 2026 07:59
@github-actions github-actions Bot added the component:sight src/agentsight/ label Jul 8, 2026
@haoowa haoowa mentioned this pull request Jul 8, 2026
33 tasks
@jfeng18

jfeng18 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Frontend reads cleanly overall. Two small things plus a coordination note:

  • ConversationList.tsx:355-371 — the per-conversation badge lookup does .catch(() => null) (apiFetch throws on any non-2xx) and then unconditionally adds the id to evaluationLookupDone, so a transient 5xx/timeout is treated the same as "no evaluation yet": the badge stays absent with no error surface and no retry until the session/time-range changes.
  • EvaluationBadge.tsx:25/28 and the recommendedAction / rootCause label maps in EvaluationPanel lack the ?? value fallback used by the other label helpers, so a backend enum variant not yet mirrored in the TS union renders an empty label / undefined className.

Coordination: this PR calls /api/grader/latest and /api/grader/evaluate, which are added in #1395 — worth merging #1395 first so badges/Evaluate don't 404. It degrades gracefully, just flagging the order since both target main.

@haoowa

haoowa commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Updated in cdee9bf to address the two dashboard review notes:

  • Latest evaluation badge lookups no longer mark a conversation as fetched when the request fails. A transient 5xx/timeout now surfaces a retryable 加载失败 badge state instead of being silently treated as "no evaluation yet".
  • Evaluation label rendering now has forward-compatible fallbacks. Unknown backend verdict values render with a neutral style and raw label, while unknown root_cause values fall back to the raw value and the backend-provided recommended_action.

Added regression coverage for:

  • failed latest-run lookup visibility and retry
  • unknown verdict fallback rendering
  • unknown root-cause/action fallback rendering

Validation:

  • local targeted dashboard tests: 35 passed
  • local npm run typecheck
  • local npm run build
  • Linux targeted dashboard tests: 35 passed
  • Linux npm run typecheck
  • Linux npm run build

I also confirmed that the remaining full npm test failures are in unrelated existing suites (AgentHealthSidebar and TokenSavingsPage), outside this PR feedback path.

haoowa added 2 commits July 9, 2026 19:59
Add conversation-list controls, latest-run badges, and a detail panel for
manually running and inspecting grader results.

Keep the frontend split from the Rust API implementation so the dashboard
surface can be reviewed independently.

Assisted-by: Codex:GPT-5
Signed-off-by: wanghao <1562495626@qq.com>
Only successful latest-run lookups mark conversations as fetched.

Failed lookups now surface a retryable badge state instead of hiding errors.

Unknown backend verdict and root-cause values fall back to raw labels and server actions.

This keeps the dashboard compatible with small grader API enum extensions.

Assisted-by: Codex:GPT-5
Signed-off-by: wanghao <1562495626@qq.com>
@haoowa haoowa force-pushed the feat/sight/grader-dashboard branch from cdee9bf to 3018079 Compare July 9, 2026 12:00

@jfeng18 jfeng18 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the diff end-to-end (independent sweep + adversarial verification, plus a local npm ci + vitest run of the grader tests — 55 pass). The cross-component contract with the #1395 backend is clean: verdict / root_cause / evidence_type / grader_type serialization, optional-vs-null semantics, the 409 conversation_not_ready shape, /grader/latest returning a bare result or JSON null, and the request body all line up field-for-field. Both new effects converge (no re-fetch loop; the AtifViewer mount effect stays []-gated so widening handleLoad's deps is loop-safe). Nice split.

Two low, non-blocking items:

1. [low] 7 of 12 interruption types render as raw English in the grader panel. findingLabel (EvaluationPanel.tsx:278) and evidenceLabel (:320, via its findingLabel fallback) only translate 5 interruption codes (llm_error, sse_truncated, network_timeout, service_unavailable, agent_crash). But interruption-derived findings carry finding.code = record.interruption_type (grader/rule.rs:277) and their evidence carries label = interruption_type (grader/evidence.rs:155), and the full vocabulary is 12 — all real emitted variants in InterruptionType::as_str (interruption/types.rs:41-51). So findings/evidence for auth_error, context_overflow, dead_loop, rate_limit, retry_storm, safety_filter, token_limit fall through labels[value] ?? value and show raw snake_case, even though apiClient.ts already exports INTERRUPTION_TYPE_CN with Chinese for all of them (and InterruptionBadge already renders those same types translated). Consulting INTERRUPTION_TYPE_CN in the fallback would close the gap.

2. [nit] Duplicate React key in the findings list. key={${finding.code}-${finding.message}} (EvaluationPanel.tsx:160). build_findings emits one finding per interrupted event, and one per unresolved interruption of the same type, so (code, message) is not unique — 2+ such findings produce identical keys and React logs "Encountered two children with the same key". I verified all rows still render on initial mount (no dropped rows), so this is a console-warning / reconciliation-hygiene nit, not a visible bug. A per-item index or a ref id in the key clears it.

Everything else LGTM.

@chengshuyi chengshuyi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

整体代码结构清晰,组件拆分合理,测试覆盖充分(95 tests pass)。类型定义完整且与后端 schema 对齐良好。以下是几点建议:

🔴 Finding 1(建议必须修复)— evaluateConversation 未使用 apiFetch 封装

apiClient.tsfetchLatestEvaluation 使用了项目统一的 apiFetch,但 evaluateConversation 直接调用原生 fetch

const res = await fetch(`${API_BASE}/api/grader/evaluate`, {
  method: "POST",
  headers: { "Content-Type": "application/json" },
  body: JSON.stringify({ ... }),
});

apiFetch 封装了两个认证关键逻辑:

  1. credentials: "same-origin" — 确保携带 session cookie
  2. 401 状态码自动跳转登录页

绕过 apiFetch 意味着在 server.auth.enabled = true 的环境下,POST 请求不会携带 cookie,后端返回 401 后用户也不会被引导到登录页,"开始评估"按钮会直接失败。

建议:扩展 apiFetch 支持 POST(如新增 apiFetchPost<T>(url, body)),或让 evaluateConversation 复用相同的 credentials 和 401 处理逻辑。

🟡 Finding 2 — EvaluationPanelinitialResult prop 更新后不生效

const [result, setResult] = useState<EvaluationResult | null>(initialResult);

useState(initialValue) 只在首次 mount 时使用初始值,后续 prop 变化不会同步。当前因为面板展开/收起会重新挂载,影响有限,但如果将来需要在面板打开状态下更新数据会出问题。建议添加 useEffect 监听 initialResult 变化。

🟡 Finding 5 — searchParams 加入 useEffect 依赖可能导致不必要的重新请求

AtifViewerPage.tsx 中将 searchParams 加入了 fetch ATIF 文档的 useEffect 依赖数组,但 effect 内部又会调用 setSearchParams,形成潜在的循环触发。建议从依赖中移除 searchParams,改用 useRef 或在回调体内读取最新值。

🟢 其他小建议

  • highlightedSectionsdoc.steps 缺少空值防御,建议加 doc.steps ?? []
  • Evidence ref 的 React key 可能不唯一,建议加上数组 index 作为 tiebreaker
  • EvaluationPanel.tsx 中约 140 行是独立的 label 映射函数,可考虑统一到单一 i18n 对象中(不阻塞合入)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:sight src/agentsight/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants