fix: address critical and major issues from comprehensive testing

[oren153]

Summary

• XSS fix in HTML report onclick handler — use escJs() for JavaScript context escaping
• MCP logger fix — route all logging to stderr to prevent JSON-RPC protocol corruption on stdout
• Silent error swallowing — log skipped conversations in triage_check with counter in results
• Missing trace source check — add axiom/langfuse to triage_explain source gate
• averageMetrics fix — exclude binary truncationScore from quality average
• Zero-conversations guard — early exit in CLI when no conversations remain after filtering
• rootCauseTurn clamping — prevent out-of-bounds turn index from LLM responses
• failureType validation — use validateEnum consistently in both batch and individual paths
• MCP annotations — add readOnlyHint: true to 4 read-only tools
• Recommendation logging — replace silent catch {} with logged warnings in MCP tools

Test plan

• All 295 unit tests pass
• TypeScript compiles clean (tsc --noEmit)
• Full build succeeds
• Security review: XSS escaping, stderr routing, safePath coverage — all verified
• Logic review: all 6 change areas verified for correctness

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added axiom and langfuse as valid trace sources for diagnosis.
  • Added read-only annotations for read-only tools.
  • Enhanced error tracking with skipped conversation counts in summaries.

• Bug Fixes

  • Improved error handling when no conversations match applied filters.
  • Fixed data validation for failure types with appropriate defaults.
  • Corrected metric averaging calculations to exclude truncation scores.
  • Fixed rootCauseTurn clamping to valid message ranges.
  • Enhanced logging for non-fatal evaluation errors to prevent silent failures.
  • Secured command string output in HTML reports.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: edf9db0f-2ae8-4d4f-b23f-a7a2924e3c27

📥 Commits

Reviewing files that changed from the base of the PR and between e1afebd and 8d0fb39.

📒 Files selected for processing (10)

• src/cli/analyze.ts
• src/evaluation/diagnosis.ts
• src/evaluation/policy-checker.ts
• src/evaluation/shared.ts
• src/mcp/helpers.ts
• src/mcp/index.ts
• src/mcp/tools-eval.ts
• src/mcp/tools-read.ts
• src/report/sections.ts
• test/evaluation/diagnosis.test.ts

---

📝 Walkthrough

Walkthrough

The PR introduces validation improvements and error handling enhancements across CLI, evaluation, and MCP modules. Changes include clamping rootCauseTurn values, adding enum validation, excluding metrics from averaging, expanding logging capabilities, adding tool read-only annotations, sanitizing HTML output, and updating test fixtures.

Changes

Cohort / File(s)	Summary
Validation & Clamping src/evaluation/diagnosis.ts, src/mcp/helpers.ts	Added clamping for rootCauseTurn to ensure values remain within valid range [1, message count], replacing direct parsed value usage.
Enum Validation src/evaluation/policy-checker.ts	Wrapped failureType assignment with validateEnum to enforce enum constraints and provide default value "prompt_issue".
Metrics Filtering src/evaluation/shared.ts	Introduced exclusion set to filter out truncationScore from averageMetrics calculation.
Error Handling & Logging src/cli/analyze.ts, src/mcp/tools-eval.ts	Added early abort for empty conversation sets; expanded triage_explain source eligibility to include axiom/langfuse; enhanced error tracking with skippedCount and warning logs across triage workflows.
Logger Configuration src/mcp/index.ts	Replaced consoleLogger with new stderrLogger implementation that routes all messages to console.error.
Tool Metadata src/mcp/tools-read.ts	Added readOnlyHint annotation to six read-only tool registrations (triage_status, triage_list_policies, triage_diff, triage_history, triage_sample, triage_view).
Output Sanitization src/report/sections.ts	Applied escJs escaping to command string embedded in HTML onclick handler for JavaScript context safety.
Test Fixtures test/evaluation/diagnosis.test.ts	Updated conv-1 test case with explicit messages array containing user and assistant messages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Twitches nose with glee
Validation clamps and logs now flow,
Metrics filter, truths we know,
HTML escapes keep attacks low—
A safer trail through code we grow! 🌿

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/test-findings-batch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}