design(pr-detail): humanize old AI errors, drop dead live chrome, legible diffs, readable prose (#211) #5463
3 changed files+187−73
Addedsrc/__tests__/pulls-present-ai-comment.test.ts+56−0View fileUnifiedSplit
@@ -0,0 +1,56 @@
1/**
2 * Render-time humanizing of historical AI-review failure comments
3 * (issue #211, PR-detail sweep). Old comment rows persisted raw Anthropic
4 * error envelopes verbatim; `presentAiCommentBody` rewrites the envelope
5 * portion at render time without touching the DB row.
6 */
7
8import { describe, it, expect } from "bun:test";
9import { presentAiCommentBody } from "../routes/pulls";
10
11const RAW_CREDIT_ENVELOPE =
12 '400 {"type":"error","error":{"type":"invalid_request_error","message":"Your credit balance is too low to access the Anthropic API."},"request_id":"req_011CRExample"}';
13
14describe("pulls detail — presentAiCommentBody (historical AI error envelopes)", () => {
15 it("humanizes the legacy 'AI review attempt failed' advisory shape", () => {
16 const body =
17 "<!-- gluecron:ai-review -->\n## AI review unavailable\n\n" +
18 `The AI review attempt failed: AI call failed: ${RAW_CREDIT_ENVELOPE}. The PR is otherwise unchanged.`;
19 const out = presentAiCommentBody(body);
20 expect(out).not.toContain("request_id");
21 expect(out).not.toContain('{"type":"error"');
22 expect(out).toContain("AI review unavailable");
23 expect(out).toContain("The PR is otherwise unchanged.");
24 // Credit-balance failures get the specific translation.
25 expect(out).toContain("AI balance is exhausted");
26 });
27
28 it("humanizes a bare envelope without the legacy sentence", () => {
29 const body = `## AI review unavailable\n\nAI call failed: ${RAW_CREDIT_ENVELOPE}`;
30 const out = presentAiCommentBody(body);
31 expect(out).not.toContain("request_id");
32 expect(out).not.toContain('{"type":"error"');
33 expect(out).toContain("AI balance is exhausted");
34 });
35
36 it("translates rate-limit envelopes to the rate-limit message", () => {
37 const body =
38 "The AI review attempt failed: AI call failed: " +
39 '429 {"type":"error","error":{"type":"rate_limit_error","message":"Number of requests has exceeded your rate limit"},"request_id":"req_x"}' +
40 ". The PR is otherwise unchanged.";
41 const out = presentAiCommentBody(body);
42 expect(out).not.toContain("request_id");
43 expect(out.toLowerCase()).toContain("rate-limited");
44 });
45
46 it("leaves normal AI review comments untouched", () => {
47 const body =
48 "## AI Review\n\nLooks good overall. One note on error handling in `src/lib/foo.ts:42`.";
49 expect(presentAiCommentBody(body)).toBe(body);
50 });
51
52 it("leaves prose that merely mentions a 400 status untouched", () => {
53 const body = "The endpoint now returns 400 when the payload is malformed.";
54 expect(presentAiCommentBody(body)).toBe(body);
55 });
56});
Modifiedsrc/routes/pulls.tsx+77−22View fileUnifiedSplit
@@ -71,7 +71,7 @@ import { triggerPrTriage } from "../lib/pr-triage";
7171import { logActivity } from "../lib/notify";
7272import { fireWebhooks } from "./webhooks";
7373import { generatePrSummary } from "../lib/ai-generators";
74import { isAiAvailable } from "../lib/ai-client";
74import { isAiAvailable, humanizeAiError } from "../lib/ai-client";
7575import { getReviewContext, recordPrVisit, type ReviewContext } from "../lib/review-context";
7676import {
7777 computePrRiskForPullRequest,
@@ -598,6 +598,14 @@ const PRS_DETAIL_STYLES = `
598598 color: var(--text-muted);
599599 }
600600
601 /* Readable measure — the PR description and conversation prose cap at
602 ~900px instead of spanning the full content shell (~1500px on wide
603 screens, unreadable line lengths). Diffs, tabs and panels keep the
604 full width. */
605 .issue-comment-box,
606 .prs-comment,
607 .slash-pill { max-width: 900px; }
608
601609 /* Comment cards */
602610 .prs-comment {
603611 margin-top: 14px;
@@ -2571,6 +2579,37 @@ function isAiGeneratedPr(body: string | null, headBranch: string): boolean {
25712579 return AI_BODY_MARKERS.some((m) => lower.includes(m.toLowerCase()));
25722580}
25732581
2582/**
2583 * Historical AI-review comments persisted raw Anthropic error envelopes
2584 * verbatim ("AI review unavailable … AI call failed: 400 {\"type\":\"error\"
2585 * …\"request_id\":…}") before `humanizeAiError()` was wired into
2586 * ai-review.ts. The DB rows are left untouched — this rewrites only what
2587 * the page shows, so old failure comments read like the humanized ones new
2588 * failures produce.
2589 */
2590export function presentAiCommentBody(body: string): string {
2591 if (!body.includes('{"type":"error"') && !body.includes('"request_id"')) {
2592 return body;
2593 }
2594 // Legacy advisory shape: a fixed sentence wrapping the raw reason.
2595 const legacy = body.match(
2596 /The AI review attempt failed: ([\s\S]*?)\.?\s*The PR is otherwise unchanged\./
2597 );
2598 if (legacy) {
2599 return body.replace(
2600 legacy[0],
2601 `${humanizeAiError(legacy[1])} The PR is otherwise unchanged.`
2602 );
2603 }
2604 // Bare envelope: swallow from the failure prefix (or the HTTP status)
2605 // through the end of the JSON blob and translate it.
2606 const envelope = body.match(/(?:AI call failed:\s*)?\b\d{3}\s*\{[\s\S]*\}/);
2607 if (envelope) {
2608 return body.replace(envelope[0], humanizeAiError(envelope[0]));
2609 }
2610 return body;
2611}
2612
25742613// List PRs
25752614pulls.get("/:owner/:repo/pulls", softAuth, requireRepoAccess("read"), async (c) => {
25762615 const { owner: ownerName, repo: repoName } = c.req.param();
@@ -4338,7 +4377,7 @@ pulls.get("/:owner/:repo/pulls/:number", softAuth, requireRepoAccess("read"), as
43384377 filePath: r.filePath!,
43394378 lineNumber: r.lineNumber!,
43404379 authorUsername: r.authorUsername,
4341 body: renderMarkdown(r.body),
4380 body: renderMarkdown(r.isAiReview ? presentAiCommentBody(r.body) : r.body),
43424381 isAiReview: r.isAiReview,
43434382 createdAt: r.createdAt.toISOString(),
43444383 }));
@@ -4647,18 +4686,23 @@ pulls.get("/:owner/:repo/pulls/:number", softAuth, requireRepoAccess("read"), as
46474686 </button>
46484687 </form>
46494688 )}
4650 <span
4651 id="live-pill"
4652 class="live-pill"
4653 title="People editing this PR right now"
4654 >
4655 <span class="live-pill-dot" aria-hidden="true"></span>
4656 <span>
4657 Live: <strong id="live-count">0</strong> editing
4689 {/* Live-state chrome (co-editing pill, preview badge) is only
4690 meaningful while the PR can still change — after merge/close
4691 a frozen "Building" badge or "Live: 0 editing" is noise. */}
4692 {pr.state === "open" && (
4693 <span
4694 id="live-pill"
4695 class="live-pill"
4696 title="People editing this PR right now"
4697 >
4698 <span class="live-pill-dot" aria-hidden="true"></span>
4699 <span>
4700 Live: <strong id="live-count">0</strong> editing
4701 </span>
4702 <span id="live-avatars" class="live-avatars" aria-hidden="true"></span>
46584703 </span>
4659 <span id="live-avatars" class="live-avatars" aria-hidden="true"></span>
4660 </span>
4661 {preview && (
4704 )}
4705 {pr.state === "open" && preview && (
46624706 <a
46634707 class={`preview-prpill is-${preview.status}`}
46644708 href={
@@ -4688,20 +4732,25 @@ pulls.get("/:owner/:repo/pulls/:number", softAuth, requireRepoAccess("read"), as
46884732 )}
46894733 </div>
46904734 </div>
4691 <script
4692 dangerouslySetInnerHTML={{
4693 __html: LIVE_COEDIT_SCRIPT(pr.id),
4694 }}
4695 />
4735 {pr.state === "open" && (
4736 <script
4737 dangerouslySetInnerHTML={{
4738 __html: LIVE_COEDIT_SCRIPT(pr.id),
4739 }}
4740 />
4741 )}
46964742 <script dangerouslySetInnerHTML={{ __html: mentionAutocompleteScript() }} />
46974743
46984744
46994745
47004746 {/* Presence styles + bar (shown only on the files tab so cursor pills work) */}
47014747 <style dangerouslySetInnerHTML={{ __html: PRESENCE_STYLES + IMPACT_STYLES }} />
4702 {/* Toast container — always present for join/leave toasts */}
4703 <div id="presence-toasts" class="presence-toast-wrap" aria-live="polite" />
4704 {user && (
4748 {/* Toast container — present for join/leave toasts while the PR is
4749 open; presence is meaningless on merged/closed PRs. */}
4750 {pr.state === "open" && (
4751 <div id="presence-toasts" class="presence-toast-wrap" aria-live="polite" />
4752 )}
4753 {user && pr.state === "open" && (
47054754
47064755 <div class="presence-bar" id="presence-bar">
47074756 <span class="presence-bar-label">Live reviewers</span>
@@ -4951,7 +5000,13 @@ pulls.get("/:owner/:repo/pulls/:number", softAuth, requireRepoAccess("read"), as
49515000 )}
49525001 </div>
49535002 <div class="prs-comment-body">
4954 <MarkdownContent html={renderMarkdown(comment.body)} />
5003 <MarkdownContent
5004 html={renderMarkdown(
5005 comment.isAiReview
5006 ? presentAiCommentBody(comment.body)
5007 : comment.body
5008 )}
5009 />
49555010 </div>
49565011 </div>
49575012 );
Modifiedsrc/views/diff-view.tsx+54−51View fileUnifiedSplit
@@ -545,8 +545,8 @@ export const DiffView: FC<DiffViewProps> = ({ raw, files, viewFileBase, inlineCo
545545 </td>
546546 </tr>
547547 {splitRows.map((row) => {
548 const leftBg = row.left?.kind === "del" ? "rgba(248,113,113,0.08)" : "transparent";
549 const rightBg = row.right?.kind === "add" ? "rgba(52,211,153,0.08)" : "transparent";
548 const leftBg = row.left?.kind === "del" ? "color-mix(in srgb, var(--red) 12%, var(--bg-elevated))" : "transparent";
549 const rightBg = row.right?.kind === "add" ? "color-mix(in srgb, var(--green) 14%, var(--bg-elevated))" : "transparent";
550550 return (
551551 <tr class="diff-split-row">
552552 <td class="diff-ln" style={`background:${leftBg};color:var(--text-muted);padding:0 6px;font-size:11px;font-family:var(--font-mono);text-align:right;user-select:none;border-right:1px solid var(--border);`}>
@@ -965,14 +965,14 @@ const DIFF_VIEW_CSS = `
965965 line-height: 1.4;
966966 }
967967 .diff-stat-add {
968 color: #6ee7b7;
969 background: rgba(52,211,153,0.12);
970 border: 1px solid rgba(52,211,153,0.22);
968 color: var(--green);
969 background: color-mix(in srgb, var(--green) 12%, transparent);
970 border: 1px solid color-mix(in srgb, var(--green) 30%, transparent);
971971 }
972972 .diff-stat-del {
973 color: #fca5a5;
974 background: rgba(248,113,113,0.10);
975 border: 1px solid rgba(248,113,113,0.22);
973 color: var(--red);
974 background: color-mix(in srgb, var(--red) 12%, transparent);
975 border: 1px solid color-mix(in srgb, var(--red) 30%, transparent);
976976 }
977977
978978 .diff-file {
@@ -1051,16 +1051,16 @@ const DIFF_VIEW_CSS = `
10511051 border-color: var(--border);
10521052 }
10531053 .diff-file-copy.is-copied {
1054 color: var(--green, #6ee7b7);
1055 background: rgba(52,211,153,0.12);
1056 border-color: rgba(52,211,153,0.30);
1054 color: var(--green);
1055 background: color-mix(in srgb, var(--green) 12%, transparent);
1056 border-color: color-mix(in srgb, var(--green) 30%, transparent);
10571057 }
10581058 .diff-file-copy.is-copied::after {
10591059 content: 'Copied';
10601060 position: absolute;
10611061 margin-left: 28px;
10621062 font-size: 11px;
1063 color: var(--green, #6ee7b7);
1063 color: var(--green);
10641064 font-family: var(--font-sans, inherit);
10651065 }
10661066
@@ -1097,24 +1097,24 @@ const DIFF_VIEW_CSS = `
10971097 border: 1px solid transparent;
10981098 }
10991099 .diff-status-added {
1100 color: #6ee7b7;
1101 background: rgba(52,211,153,0.10);
1102 border-color: rgba(52,211,153,0.22);
1100 color: var(--green);
1101 background: color-mix(in srgb, var(--green) 12%, transparent);
1102 border-color: color-mix(in srgb, var(--green) 30%, transparent);
11031103 }
11041104 .diff-status-modified {
1105 color: #fcd34d;
1106 background: rgba(252,211,77,0.08);
1107 border-color: rgba(252,211,77,0.22);
1105 color: var(--yellow);
1106 background: color-mix(in srgb, var(--yellow) 10%, transparent);
1107 border-color: color-mix(in srgb, var(--yellow) 30%, transparent);
11081108 }
11091109 .diff-status-renamed {
1110 color: #93c5fd;
1111 background: rgba(147,197,253,0.10);
1112 border-color: rgba(147,197,253,0.25);
1110 color: var(--blue);
1111 background: color-mix(in srgb, var(--blue) 12%, transparent);
1112 border-color: color-mix(in srgb, var(--blue) 30%, transparent);
11131113 }
11141114 .diff-status-deleted {
1115 color: #fca5a5;
1116 background: rgba(248,113,113,0.10);
1117 border-color: rgba(248,113,113,0.22);
1115 color: var(--red);
1116 background: color-mix(in srgb, var(--red) 12%, transparent);
1117 border-color: color-mix(in srgb, var(--red) 30%, transparent);
11181118 }
11191119 .diff-status-binary {
11201120 color: var(--text-muted);
@@ -1187,28 +1187,31 @@ const DIFF_VIEW_CSS = `
11871187 overflow-x: visible;
11881188 }
11891189
1190 /* Row tints — additions / deletions / context */
1190 /* Row tints — additions / deletions / context. Mixed from the semantic
1191 --green/--red tokens so both themes get a clearly visible tint (the
1192 old fixed rgba() pastels were tuned for dark and near-invisible on
1193 the light theme's warm paper). */
11911194 .diff-row-add {
1192 background: rgba(52,211,153,0.08);
1195 background: color-mix(in srgb, var(--green) 14%, var(--bg-elevated));
11931196 }
11941197 .diff-row-add .diff-gutter,
11951198 .diff-row-add .diff-marker {
1196 background: rgba(52,211,153,0.14);
1197 color: #6ee7b7;
1198 border-right-color: rgba(52,211,153,0.20);
1199 background: color-mix(in srgb, var(--green) 26%, var(--bg-elevated));
1200 color: var(--green);
1201 border-right-color: color-mix(in srgb, var(--green) 40%, transparent);
11991202 }
1200 .diff-row-add .diff-marker { color: #6ee7b7; font-weight: 600; }
1203 .diff-row-add .diff-marker { color: var(--green); font-weight: 600; }
12011204
12021205 .diff-row-del {
1203 background: rgba(248,113,113,0.08);
1206 background: color-mix(in srgb, var(--red) 12%, var(--bg-elevated));
12041207 }
12051208 .diff-row-del .diff-gutter,
12061209 .diff-row-del .diff-marker {
1207 background: rgba(248,113,113,0.14);
1208 color: #fca5a5;
1209 border-right-color: rgba(248,113,113,0.20);
1210 background: color-mix(in srgb, var(--red) 22%, var(--bg-elevated));
1211 color: var(--red);
1212 border-right-color: color-mix(in srgb, var(--red) 40%, transparent);
12101213 }
1211 .diff-row-del .diff-marker { color: #fca5a5; font-weight: 600; }
1214 .diff-row-del .diff-marker { color: var(--red); font-weight: 600; }
12121215
12131216 .diff-row:hover .diff-gutter { opacity: 1; }
12141217
@@ -1329,23 +1332,23 @@ const DIFF_VIEW_CSS = `
13291332 /* ─── Suggestion blocks ─── */
13301333 .diff-suggestion-block {
13311334 margin: 8px 0;
1332 border: 1px solid rgba(52,211,153,0.3);
1335 border: 1px solid color-mix(in srgb, var(--green) 30%, transparent);
13331336 border-radius: 6px;
13341337 overflow: hidden;
13351338 }
13361339 .diff-suggestion-header {
13371340 padding: 6px 12px;
1338 background: rgba(52,211,153,0.08);
1339 border-bottom: 1px solid rgba(52,211,153,0.2);
1341 background: color-mix(in srgb, var(--green) 10%, transparent);
1342 border-bottom: 1px solid color-mix(in srgb, var(--green) 22%, transparent);
13401343 font-size: 12px;
1341 color: #6ee7b7;
1344 color: var(--green);
13421345 display: flex;
13431346 align-items: center;
13441347 justify-content: space-between;
13451348 }
13461349 .diff-suggestion-code {
13471350 padding: 8px 12px;
1348 background: rgba(52,211,153,0.05);
1351 background: color-mix(in srgb, var(--green) 6%, transparent);
13491352 font-family: var(--font-mono);
13501353 font-size: 12.5px;
13511354 white-space: pre;
@@ -1353,9 +1356,9 @@ const DIFF_VIEW_CSS = `
13531356 margin: 0;
13541357 }
13551358 .diff-apply-btn {
1356 background: rgba(52,211,153,0.15);
1357 color: #6ee7b7;
1358 border: 1px solid rgba(52,211,153,0.35);
1359 background: color-mix(in srgb, var(--green) 14%, transparent);
1360 color: var(--green);
1361 border: 1px solid color-mix(in srgb, var(--green) 35%, transparent);
13591362 border-radius: 5px;
13601363 padding: 4px 12px;
13611364 font-size: 12px;
@@ -1363,7 +1366,7 @@ const DIFF_VIEW_CSS = `
13631366 font-family: var(--font-sans, inherit);
13641367 }
13651368 .diff-apply-btn:hover {
1366 background: rgba(52,211,153,0.25);
1369 background: color-mix(in srgb, var(--green) 24%, transparent);
13671370 }
13681371 .diff-suggestion-toggle {
13691372 background: transparent;
@@ -1377,15 +1380,15 @@ const DIFF_VIEW_CSS = `
13771380 margin-top: 6px;
13781381 }
13791382 .diff-suggestion-toggle.is-active {
1380 background: rgba(52,211,153,0.1);
1381 color: #6ee7b7;
1382 border-color: rgba(52,211,153,0.35);
1383 background: color-mix(in srgb, var(--green) 12%, transparent);
1384 color: var(--green);
1385 border-color: color-mix(in srgb, var(--green) 35%, transparent);
13831386 }
13841387 .diff-suggestion-textarea {
13851388 width: 100%;
1386 background: rgba(52,211,153,0.04);
1389 background: color-mix(in srgb, var(--green) 5%, transparent);
13871390 color: var(--text);
1388 border: 1px solid rgba(52,211,153,0.25);
1391 border: 1px solid color-mix(in srgb, var(--green) 25%, transparent);
13891392 border-radius: 4px;
13901393 padding: 8px;
13911394 font-size: 12.5px;
@@ -1465,8 +1468,8 @@ const DIFF_VIEW_CSS = `
14651468 flex: 1;
14661469 }
14671470 .diff-jump-pills { display: flex; gap: 4px; flex-shrink: 0; }
1468 .diff-jump-add { color: #6ee7b7; font-size: 11px; }
1469 .diff-jump-del { color: #fca5a5; font-size: 11px; }
1471 .diff-jump-add { color: var(--green); font-size: 11px; }
1472 .diff-jump-del { color: var(--red); font-size: 11px; }
14701473
14711474 /* ─── Unified/Split view toggle ─── */
14721475 .diff-view-tab {
14731476
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts