wehub/thariqs--html-effectiveness
1
0
Fork 0
Code Issues Pull Requests Actions Deployments Models Agent Notes Packages Projects Releases Wiki Activity

chore: import upstream snapshot with attribution

Browse Source
This commit is contained in:
wehub-resource-sync
2026-07-13 12:33:58 +08:00
commit 6a2f6934bb
38 changed files with 20049 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
17-pr-writeup.html
+596
View File
@@ -0,0 +1,596 @@
<!-- Copyright 2026 Anthropic PBC · SPDX-License-Identifier: Apache-2.0 -->
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>PR #312 — Move notification delivery onto a queue</title>
<style>
:root {
--ivory: #FAF9F5;
--slate: #141413;
--clay: #D97757;
--oat: #E3DACC;
--olive: #788C5D;
--gray-150: #F0EEE6;
--gray-300: #D1CFC5;
--gray-500: #87867F;
--gray-700: #3D3D3A;
--white: #FFFFFF;
--serif: ui-serif, Georgia, 'Times New Roman', serif;
--sans: system-ui, -apple-system, 'Segoe UI', Roboto, sans-serif;
--mono: ui-monospace, 'SF Mono', Menlo, Monaco, monospace;
}
* { margin: 0; padding: 0; box-sizing: border-box; }
body {
font-family: var(--sans);
background: var(--ivory);
color: var(--gray-700);
line-height: 1.55;
padding: 56px 32px 120px;
-webkit-font-smoothing: antialiased;
}
.page { max-width: 1040px; margin: 0 auto; }
/* ---------- header ---------- */
header.page-head { margin-bottom: 44px; max-width: 820px; }
.eyebrow {
font-size: 12px;
letter-spacing: 0.08em;
text-transform: uppercase;
color: var(--gray-500);
margin-bottom: 12px;
}
h1 {
font-family: var(--serif);
font-weight: 500;
font-size: 36px;
line-height: 1.15;
color: var(--slate);
margin-bottom: 10px;
letter-spacing: -0.01em;
}
.pr-meta {
display: flex;
flex-wrap: wrap;
gap: 18px;
font-family: var(--mono);
font-size: 12.5px;
color: var(--gray-500);
margin-bottom: 18px;
}
.pr-meta .stat strong { color: var(--slate); font-weight: 600; }
.pr-meta .add { color: var(--olive); }
.pr-meta .del { color: var(--clay); }
.prompt-box {
background: var(--gray-150);
border: 1.5px solid var(--gray-300);
border-radius: 12px;
padding: 16px 20px;
font-size: 14.5px;
color: var(--gray-700);
}
.prompt-box .label {
font-family: var(--mono);
font-size: 11px;
text-transform: uppercase;
letter-spacing: 0.06em;
color: var(--gray-500);
display: block;
margin-bottom: 6px;
}
/* ---------- layout ---------- */
.layout {
display: grid;
grid-template-columns: 1fr 240px;
gap: 48px;
align-items: start;
}
@media (max-width: 900px) { .layout { grid-template-columns: 1fr; } .toc { display: none; } }
section { margin-bottom: 56px; scroll-margin-top: 24px; }
h2 {
font-family: var(--serif);
font-weight: 500;
font-size: 25px;
color: var(--slate);
margin-bottom: 14px;
letter-spacing: -0.01em;
}
p.lede { font-size: 15.5px; max-width: 680px; margin-bottom: 12px; }
p + p.lede { margin-top: 12px; }
.toc {
position: sticky;
top: 32px;
border-left: 1.5px solid var(--gray-300);
padding-left: 18px;
font-size: 13px;
}
.toc .t-head {
font-family: var(--mono);
font-size: 11px;
text-transform: uppercase;
letter-spacing: 0.06em;
color: var(--gray-500);
margin-bottom: 10px;
}
.toc a { display: block; color: var(--gray-700); text-decoration: none; padding: 4px 0; }
.toc a:hover { color: var(--clay); }
.toc .sub { padding-left: 12px; color: var(--gray-500); font-size: 12.5px; }
/* ---------- TL;DR ---------- */
.tldr {
background: var(--white);
border: 1.5px solid var(--gray-300);
border-left: 4px solid var(--clay);
border-radius: 12px;
padding: 20px 24px;
margin-bottom: 48px;
max-width: 760px;
}
.tldr .k {
font-family: var(--mono);
font-size: 11px;
text-transform: uppercase;
letter-spacing: 0.06em;
color: var(--gray-500);
margin-bottom: 8px;
}
.tldr p { font-size: 15px; color: var(--slate); }
/* ---------- before/after ---------- */
.ba {
display: grid;
grid-template-columns: 1fr 1fr;
gap: 24px;
margin: 22px 0 4px;
}
@media (max-width: 720px) { .ba { grid-template-columns: 1fr; } }
.ba .panel {
background: var(--white);
border: 1.5px solid var(--gray-300);
border-radius: 12px;
padding: 18px 20px;
}
.ba .panel.after { border-color: var(--olive); }
.ba .k {
font-family: var(--mono);
font-size: 11px;
text-transform: uppercase;
letter-spacing: 0.06em;
color: var(--gray-500);
margin-bottom: 10px;
}
.ba .after .k { color: var(--olive); }
.ba ul { list-style: none; display: flex; flex-direction: column; gap: 8px; font-size: 13.5px; }
.ba li::before { content: '·'; color: var(--gray-500); margin-right: 8px; }
.ba .after li::before { color: var(--olive); }
/* ---------- file tour ---------- */
.file-tour { display: flex; flex-direction: column; gap: 22px; }
.file {
background: var(--white);
border: 1.5px solid var(--gray-300);
border-radius: 12px;
overflow: hidden;
}
.file summary {
padding: 14px 20px;
cursor: pointer;
display: flex;
align-items: center;
gap: 12px;
list-style: none;
background: var(--gray-150);
border-bottom: 1.5px solid var(--gray-300);
}
.file[open] summary { border-bottom-color: var(--gray-300); }
.file:not([open]) summary { border-bottom: none; }
.file summary::-webkit-details-marker { display: none; }
.file summary .chev {
width: 8px; height: 8px;
border-right: 2px solid var(--gray-500);
border-bottom: 2px solid var(--gray-500);
transform: rotate(-45deg);
transition: transform .15s ease;
flex-shrink: 0;
}
.file[open] summary .chev { transform: rotate(45deg); }
.file summary .path {
font-family: var(--mono);
font-size: 13px;
color: var(--slate);
flex: 1;
}
.file summary .badge {
font-family: var(--mono);
font-size: 10.5px;
text-transform: uppercase;
letter-spacing: 0.05em;
padding: 2px 8px;
border-radius: 6px;
font-weight: 600;
}
.badge.new { background: #E4E9DC; color: #4B5C39; }
.badge.mod { background: var(--oat); color: var(--slate); }
.badge.del { background: #F3D9CC; color: #8A3B1E; }
.file summary .stat { font-family: var(--mono); font-size: 12px; }
.file summary .stat .add { color: var(--olive); }
.file summary .stat .del { color: var(--clay); }
.file .file-body { padding: 18px 20px 22px; }
.file .why {
font-size: 14.5px;
margin-bottom: 14px;
max-width: 680px;
}
.file .why strong { color: var(--slate); }
/* ---------- code ---------- */
.code {
background: var(--slate);
border-radius: 10px;
padding: 16px 18px;
overflow-x: auto;
}
.code pre {
font-family: var(--mono);
font-size: 12.5px;
line-height: 1.65;
color: #E8E6DE;
white-space: pre;
}
.code .kw { color: var(--clay); }
.code .str { color: var(--olive); }
.code .cm { color: var(--gray-500); }
.code .fn { color: #C9B98A; }
.code .add { background: rgba(120,140,93,.22); display: inline-block; width: 100%; }
.code .del { background: rgba(217,119,87,.18); display: inline-block; width: 100%; text-decoration: line-through; text-decoration-color: rgba(217,119,87,.6); }
/* ---------- review focus ---------- */
.focus { display: flex; flex-direction: column; gap: 14px; max-width: 760px; }
.focus .item {
background: var(--white);
border: 1.5px solid var(--gray-300);
border-radius: 10px;
padding: 16px 20px;
display: flex;
gap: 16px;
}
.focus .n {
font-family: var(--mono);
font-size: 13px;
font-weight: 600;
color: var(--white);
background: var(--clay);
width: 26px; height: 26px;
border-radius: 50%;
display: flex; align-items: center; justify-content: center;
flex-shrink: 0;
margin-top: 2px;
}
.focus .t { font-weight: 600; color: var(--slate); font-size: 15px; margin-bottom: 2px; }
.focus .d { font-size: 13.5px; color: var(--gray-500); }
.focus .d code { font-size: 12.5px; }
/* ---------- test plan ---------- */
.tests { display: flex; flex-direction: column; gap: 10px; max-width: 760px; }
.test {
display: flex;
align-items: flex-start;
gap: 12px;
background: var(--white);
border: 1.5px solid var(--gray-300);
border-radius: 10px;
padding: 13px 18px;
font-size: 14px;
}
.test .check {
width: 18px; height: 18px;
border-radius: 5px;
flex-shrink: 0;
margin-top: 2px;
border: 1.5px solid var(--gray-300);
}
.test.done .check {
background: var(--olive);
border-color: var(--olive);
position: relative;
}
.test.done .check::after {
content: '';
position: absolute;
left: 5px; top: 2px;
width: 5px; height: 9px;
border-right: 2px solid var(--white);
border-bottom: 2px solid var(--white);
transform: rotate(40deg);
}
.test .label { color: var(--slate); }
.test .note { color: var(--gray-500); font-size: 13px; margin-top: 2px; }
/* ---------- rollout ---------- */
.rollout { display: flex; gap: 0; max-width: 780px; }
@media (max-width: 720px) { .rollout { flex-direction: column; } }
.rollout .step {
flex: 1;
background: var(--white);
border: 1.5px solid var(--gray-300);
padding: 16px 18px;
position: relative;
}
.rollout .step:first-child { border-radius: 12px 0 0 12px; }
.rollout .step:last-child { border-radius: 0 12px 12px 0; }
.rollout .step + .step { border-left: none; }
@media (max-width: 720px) {
.rollout .step, .rollout .step:first-child, .rollout .step:last-child { border-radius: 12px; }
.rollout .step + .step { border-left: 1.5px solid var(--gray-300); margin-top: 10px; }
}
.rollout .step .pct {
font-family: var(--mono);
font-size: 22px;
font-weight: 600;
color: var(--clay);
margin-bottom: 6px;
}
.rollout .step .d { font-size: 13px; color: var(--gray-500); }
.rollout .step .when { font-family: var(--mono); font-size: 11px; color: var(--gray-500); text-transform: uppercase; letter-spacing: 0.05em; margin-bottom: 8px; }
</style>
</head>
<body>
<div class="page">
<header class="page-head">
<div class="eyebrow">Pull request · Acme</div>
<h1>#312 — Move notification delivery onto a queue</h1>
<div class="pr-meta">
<span class="stat"><strong>9</strong> files</span>
<span class="stat"><span class="add">+418</span> / <span class="del">−190</span></span>
<span class="stat">branch <strong>notify-queue</strong> → <strong>main</strong></span>
<span class="stat">author <strong>@priya</strong></span>
</div>
<div class="prompt-box">
<span class="label">Prompt</span>
Write up PR #312 for my reviewers. Explain the motivation, walk them
through the change file by file with the <em>why</em> for each, show
before/after behavior, and tell them exactly where to focus. They
haven't touched the notification code in six months.
</div>
</header>
<div class="layout">
<main>
<div class="tldr">
<div class="k">TL;DR</div>
<p>Notification sends were happening inline in the request path. Under load they added 200–800&nbsp;ms to mutation latency and silently dropped emails when the SMTP pool was exhausted. This PR moves delivery onto the existing <code>pg-boss</code> queue so the API returns immediately and failed sends retry with backoff.</p>
</div>
<!-- ======================= WHY ======================= -->
<section id="why">
<h2>Why</h2>
<p class="lede">When we added @mentions in comments last quarter, every mention started triggering up to three sends (in-app, email, Slack) inside the same transaction that saved the comment. That was fine at launch. It is not fine now that a single task update can fan out to forty watchers.</p>
<div class="ba">
<div class="panel">
<div class="k">Before</div>
<ul>
<li>Sends run inline in the mutation handler</li>
<li>SMTP timeout = 500 error for the <em>comment</em></li>
<li>No retries — a dropped email is gone</li>
<li>p99 on <code>comments.create</code>: <strong>1.4&nbsp;s</strong></li>
</ul>
</div>
<div class="panel after">
<div class="k">After</div>
<ul>
<li>Handler enqueues one job per recipient, returns</li>
<li>Worker retries 3× with exponential backoff</li>
<li>Dead-letter table for inspection after exhaustion</li>
<li>p99 on <code>comments.create</code>: <strong>180&nbsp;ms</strong> (staging)</li>
</ul>
</div>
</div>
</section>
<!-- ======================= FILE TOUR ======================= -->
<section id="tour">
<h2>File-by-file</h2>
<p class="lede">Ordered for reading, not alphabetically. Start at the worker — it's the new thing — then the enqueue call site, then the plumbing.</p>
<div class="file-tour">
<details class="file" open>
<summary>
<span class="chev"></span>
<span class="path">packages/notify/src/worker.ts</span>
<span class="badge new">new</span>
<span class="stat"><span class="add">+126</span></span>
</summary>
<div class="file-body">
<p class="why"><strong>The heart of the PR.</strong> A <code>pg-boss</code> subscriber that pulls <code>notify.deliver</code> jobs, resolves the user's channel preferences, and calls the right adapter. Retries are configured per-channel — email gets three attempts, Slack gets one because its API is already idempotent on our side.</p>
<div class="code"><pre>boss.<span class="fn">work</span>(<span class="str">'notify.deliver'</span>, { batchSize: 20 }, <span class="kw">async</span> (jobs) =&gt; {
<span class="kw">for</span> (<span class="kw">const</span> job <span class="kw">of</span> jobs) {
<span class="kw">const</span> { userId, event, channel } = job.data;
<span class="kw">const</span> prefs = <span class="kw">await</span> <span class="fn">getPrefs</span>(userId);
<span class="kw">if</span> (!prefs[channel]) <span class="kw">return</span>; <span class="cm">// user muted this channel</span>
<span class="kw">try</span> {
<span class="kw">await</span> adapters[channel].<span class="fn">send</span>(userId, event);
} <span class="kw">catch</span> (err) {
<span class="kw">if</span> (job.retryCount &gt;= MAX_RETRY[channel]) {
<span class="kw">await</span> <span class="fn">deadLetter</span>(job, err); <span class="cm">// don't throw — ack &amp; park</span>
<span class="kw">return</span>;
}
<span class="kw">throw</span> err; <span class="cm">// pg-boss reschedules</span>
}
}
});</pre></div>
</div>
</details>
<details class="file" open>
<summary>
<span class="chev"></span>
<span class="path">packages/api/src/routers/comments.ts</span>
<span class="badge mod">mod</span>
<span class="stat"><span class="add">+14</span> <span class="del">−62</span></span>
</summary>
<div class="file-body">
<p class="why"><strong>Where the win shows up.</strong> The mutation used to call <code>sendEmail</code>, <code>sendSlack</code>, and <code>createInApp</code> directly. Now it inserts the comment, computes recipients, and enqueues. The try/catch soup is gone.</p>
<div class="code"><pre> <span class="kw">const</span> comment = <span class="kw">await</span> db.comments.<span class="fn">insert</span>(input);
<span class="kw">const</span> recipients = <span class="kw">await</span> <span class="fn">resolveWatchers</span>(input.taskId, input.mentions);
<span class="del"> <span class="kw">for</span> (<span class="kw">const</span> r <span class="kw">of</span> recipients) {</span>
<span class="del"> <span class="kw">await</span> <span class="fn">sendEmail</span>(r, comment); <span class="cm">// blocked the response</span></span>
<span class="del"> <span class="kw">await</span> <span class="fn">sendSlack</span>(r, comment);</span>
<span class="del"> }</span>
<span class="add"> <span class="kw">await</span> boss.<span class="fn">insert</span>(recipients.flatMap((r) =&gt;</span>
<span class="add"> CHANNELS.map((ch) =&gt; ({</span>
<span class="add"> name: <span class="str">'notify.deliver'</span>,</span>
<span class="add"> data: { userId: r.id, channel: ch, event: toEvent(comment) },</span>
<span class="add"> singletonKey: <span class="str">`${comment.id}:${r.id}:${ch}`</span>, <span class="cm">// idempotent</span></span>
<span class="add"> }))));</span>
<span class="kw">return</span> comment;</pre></div>
</div>
</details>
<details class="file">
<summary>
<span class="chev"></span>
<span class="path">packages/db/migrations/0051_dead_letter.sql</span>
<span class="badge new">new</span>
<span class="stat"><span class="add">+22</span></span>
</summary>
<div class="file-body">
<p class="why">Table for jobs that exhaust their retries. Deliberately <em>not</em> auto-pruned — we want to look at these weekly until we trust the new path. Has the full job payload and the last error string.</p>
</div>
</details>
<details class="file">
<summary>
<span class="chev"></span>
<span class="path">packages/notify/src/adapters/{email,slack,inapp}.ts</span>
<span class="badge mod">mod</span>
<span class="stat"><span class="add">+88</span> <span class="del">−74</span></span>
</summary>
<div class="file-body">
<p class="why">Mostly moves. Each adapter now implements a shared <code>Adapter</code> interface and throws a typed <code>RetryableError</code> or <code>PermanentError</code> so the worker knows whether to retry. The email adapter also drops its internal retry loop — the queue owns retries now, double-retrying was how we got duplicate emails in April.</p>
</div>
</details>
<details class="file">
<summary>
<span class="chev"></span>
<span class="path">apps/worker/src/index.ts, infra/fly.toml</span>
<span class="badge mod">mod</span>
<span class="stat"><span class="add">+31</span> <span class="del">−4</span></span>
</summary>
<div class="file-body">
<p class="why">Registers the new subscriber in the existing worker process and bumps its concurrency from 5 → 20. No new deploy unit.</p>
</div>
</details>
<details class="file">
<summary>
<span class="chev"></span>
<span class="path">packages/notify/src/__tests__/worker.test.ts</span>
<span class="badge new">new</span>
<span class="stat"><span class="add">+137</span></span>
</summary>
<div class="file-body">
<p class="why">Covers the retry boundary, the dead-letter path, channel muting, and the singleton key dedupe. Uses a real pg-boss against the test database — we got burned last quarter when mocked queue tests passed but prod ordering broke.</p>
</div>
</details>
</div>
</section>
<!-- ======================= FOCUS ======================= -->
<section id="focus">
<h2>Where to focus your review</h2>
<div class="focus">
<div class="item">
<div class="n">1</div>
<div>
<div class="t">The retry / dead-letter boundary</div>
<div class="d"><code>worker.ts:31–44</code>. I catch, check <code>retryCount</code>, and either park or rethrow. If this logic is wrong we either retry forever or drop messages — the two failure modes this PR exists to fix.</div>
</div>
</div>
<div class="item">
<div class="n">2</div>
<div>
<div class="t">The singleton key</div>
<div class="d"><code>comments.ts:28</code>. <code>${commentId}:${userId}:${channel}</code> should make re-enqueues idempotent if the API handler retries. Sanity-check that this can't collide across tasks.</div>
</div>
</div>
<div class="item">
<div class="n">3</div>
<div>
<div class="t">What I deliberately did not do</div>
<div class="d">No per-user digest batching, no delivery receipts, no priority lanes. All of those layer on top of this cleanly; bundling them would make this unreviewable.</div>
</div>
</div>
</div>
</section>
<!-- ======================= TEST PLAN ======================= -->
<section id="tests">
<h2>Test plan</h2>
<div class="tests">
<div class="test done"><span class="check"></span><div><div class="label">Unit: retry → dead-letter path, channel mute, singleton dedupe</div><div class="note">packages/notify — 14 cases, real pg-boss on test db</div></div></div>
<div class="test done"><span class="check"></span><div><div class="label">Integration: create comment with 3 watchers, assert 9 jobs enqueued and drained</div></div></div>
<div class="test done"><span class="check"></span><div><div class="label">Staging load: 500 rps on comments.create for 10 min, p99 = 180 ms</div><div class="note">was 1.4 s before — dashboard linked in the PR description</div></div></div>
<div class="test"><span class="check"></span><div><div class="label">Manual: kill SMTP mid-burst, confirm jobs land in dead-letter and nothing 500s</div><div class="note">will do during the 10% ramp</div></div></div>
</div>
</section>
<!-- ======================= ROLLOUT ======================= -->
<section id="rollout">
<h2>Rollout</h2>
<p class="lede">Behind <code>notify_queue_v2</code>. The old inline path stays in the codebase, dead but dormant, for one release in case we need to flip back.</p>
<div class="rollout">
<div class="step"><div class="when">Day 0</div><div class="pct">internal</div><div class="d">Acme team only. Watch dead-letter table + worker error rate.</div></div>
<div class="step"><div class="when">Day 2</div><div class="pct">10%</div><div class="d">Random sample. Alert if dead-letter rate &gt; 0.5% of sends.</div></div>
<div class="step"><div class="when">Day 4</div><div class="pct">100%</div><div class="d">Ramp fully, delete the inline path in a follow-up PR next week.</div></div>
</div>
</section>
</main>
<nav class="toc">
<div class="t-head">In this PR</div>
<a href="#why">Why</a>
<a href="#tour">File-by-file</a>
<a href="#tour" class="sub">worker.ts</a>
<a href="#tour" class="sub">comments.ts</a>
<a href="#tour" class="sub">adapters</a>
<a href="#focus">Where to focus</a>
<a href="#tests">Test plan</a>
<a href="#rollout">Rollout</a>
</nav>
</div>
</div>
</body>
</html>