Skip to content

[codex] collapse duplicate report markdown rendering#13

Open
mars167 wants to merge 1 commit intomainfrom
fix/repository-pull-requests-endpoint
Open

[codex] collapse duplicate report markdown rendering#13
mars167 wants to merge 1 commit intomainfrom
fix/repository-pull-requests-endpoint

Conversation

@mars167
Copy link
Copy Markdown
Owner

@mars167 mars167 commented Mar 29, 2026

Summary

  • fix the report detail page so structured review content is the default primary view
  • collapse the raw reportMarkdown block by default and expose it behind a toggle
  • reset the raw markdown panel when switching to another report

Root Cause

The report detail page rendered two top-level representations of the same review payload at once:

  • structured summary/findings/file context cards
  • the full generated reportMarkdown

For improve-mode reports like /dashboard/reports/9267, that duplicated the same content across the page and made the layout look broken.

Validation

  • npm --prefix web run lint -- 'app/dashboard/reports/[id]/page.tsx'
  • browser verification on http://localhost:3000/dashboard/reports/9267

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5b1091594

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const traceEntries = report?.trace?.entries || [];
const skippedFiles = report?.coverage?.skippedFiles || [];
const suppressedCount = report?.suppressedFindings?.length || 0;
const hasStructuredInsights = overallFindings.length > 0 || (report?.fileContexts.length || 0) > 0 || (report?.findings.length || 0) > 0 || traceEntries.length > 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Base markdown collapse on rendered context availability

hasStructuredInsights treats any report.fileContexts entry as structured content, but this page only renders highlightedFileContexts where totalFindings > 0. In reports that carry file context metadata but no actionable findings, the raw markdown is now collapsed by default even though the structured sections show only empty placeholders, so users must manually expand the only detailed report content. Derive this condition from the same rendered predicate (e.g., highlightedFileContexts.length) to avoid hiding content unexpectedly.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

@mars167 mars167 left a comment

Choose a reason for hiding this comment

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

CodaGraph Review 摘要

  • PR: #13 [codex] collapse duplicate report markdown rendering
  • 风险等级: high
  • 运行模式: normal
  • 置信度: medium
  • 覆盖情况: 1/1 文件
  • 跳过文件: 0
  • 报告 ID: 4
  • 已发布行级评论: 2

PR 修改了代码或配置,但没有看到对应的测试改动,回归风险较高。 模式:LLM + 规则;风险等级:high;置信度:medium。 共审查 1/1 个文件,发现 3 个高价值问题(critical 0 / high 1 / medium 1 / low 1)。 本次变更没有检测到跳过文件。

摘要中的补充问题

  • 本次变更缺少测试变更 (PR_OVERALL)
    • 严重性: medium
    • 描述: PR 修改了代码或配置,但没有看到对应的测试改动,回归风险较高。
    • 建议: 为新增逻辑、边界条件或修复路径补充测试,至少覆盖主要成功/失败分支。

建议动作

  • 确保 ReactMarkdown 未配置 rehype-raw 等允许原始 HTML 的插件;后端 API 应在入库前对 Markdown 内容进行 XSS 过滤(如 DOMPurify sanitize 或等效处理)。

  • 为新增逻辑、边界条件或修复路径补充测试,至少覆盖主要成功/失败分支。

  • 如果希望更细粒度控制,可以考虑将阈值改为基于严重性(如只当存在 critical/high findings 时折叠),或增加可配置性。

{shouldRenderMarkdown ? (
<div className="[&_a]:text-teal-700 [&_a]:no-underline hover:[&_a]:underline [&_blockquote]:rounded-r-2xl [&_blockquote]:border-l-4 [&_blockquote]:border-teal-400 [&_blockquote]:bg-teal-50/70 [&_blockquote]:px-5 [&_blockquote]:py-3 [&_blockquote]:text-slate-700 dark:[&_blockquote]:bg-teal-950/25 dark:[&_blockquote]:text-slate-200 [&_code]:rounded-md [&_code]:bg-slate-100 [&_code]:px-1.5 [&_code]:py-0.5 [&_code]:text-[0.92em] [&_code]:font-medium [&_code]:text-slate-700 dark:[&_code]:bg-slate-800 dark:[&_code]:text-slate-100 [&_h1]:mb-4 [&_h1]:text-3xl [&_h1]:font-semibold [&_h2]:mt-10 [&_h2]:border-b [&_h2]:border-slate-200 [&_h2]:pb-3 [&_h2]:text-xl [&_h2]:font-semibold dark:[&_h2]:border-slate-800 [&_h3]:mt-7 [&_h3]:text-lg [&_h3]:font-semibold [&_li]:my-1.5 [&_ol]:pl-5 [&_p]:my-4 [&_p]:leading-7 [&_pre]:overflow-x-auto [&_pre]:rounded-2xl [&_pre]:border [&_pre]:border-slate-800 [&_pre]:bg-slate-950 [&_pre]:px-4 [&_pre]:py-4 [&_pre]:text-[13px] [&_pre]:leading-6 [&_pre]:text-slate-100 [&_pre_code]:bg-transparent [&_pre_code]:p-0 [&_table]:block [&_table]:overflow-x-auto [&_table]:rounded-2xl [&_table]:border [&_table]:border-slate-200 dark:[&_table]:border-slate-800 [&_tbody_tr:nth-child(odd)]:bg-slate-50/80 dark:[&_tbody_tr:nth-child(odd)]:bg-slate-900/50 [&_td]:border-t [&_td]:border-slate-200 [&_td]:px-3 [&_td]:py-2 dark:[&_td]:border-slate-800 [&_th]:bg-slate-100 [&_th]:px-3 [&_th]:py-2 [&_th]:text-left dark:[&_th]:bg-slate-900">
<ReactMarkdown remarkPlugins={[remarkGfm]}>
{report.reportMarkdown}
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

HIGH · 用户生成内容直接渲染存在潜在 XSS 风险

report.reportMarkdown 来自后端 API,是用户生成内容。虽然 ReactMarkdown 默认不渲染原始 HTML(需要 rehype 插件),但如果后端或传输过程中被注入恶意内容,渲染行为可能不符合预期。需要确认 ReactMarkdown 的安全配置以及后端是否对 Markdown 内容进行了校验或 sanitization。

建议:确保 ReactMarkdown 未配置 rehype-raw 等允许原始 HTML 的插件;后端 API 应在入库前对 Markdown 内容进行 XSS 过滤(如 DOMPurify sanitize 或等效处理)。

const traceEntries = report?.trace?.entries || [];
const skippedFiles = report?.coverage?.skippedFiles || [];
const suppressedCount = report?.suppressedFindings?.length || 0;
const hasStructuredInsights = overallFindings.length > 0 || (report?.fileContexts.length || 0) > 0 || (report?.findings.length || 0) > 0 || traceEntries.length > 0;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

LOW · hasStructuredInsights 阈值可能过于宽松

当前条件 (report?.findings.length || 0) > 0 意味着只要存在任意一个 finding(即使严重级别为 low),就会默认折叠 Markdown。这意味着绝大多数报告都会默认隐藏原始 Markdown 内容,用户需要手动展开才能看到。如果这是预期行为则无问题,但可能需要与产品确认 UX 策略。

建议:如果希望更细粒度控制,可以考虑将阈值改为基于严重性(如只当存在 critical/high findings 时折叠),或增加可配置性。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant