Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1
My Work: To-do first, structured PR-summary cards, retroactive resync#11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
cf887ff
docs(spec): My Work reorder + structured cards + retroactive summary …
AndresL230 c79120d
feat(shared): parseStructuredSummary — What changed/Why markdown conv…
AndresL230 af5f344
feat(summarize): require the What changed/Why structured markdown con…
AndresL230 de744f9
feat(mywork): render structured What changed/Why PR summaries as labe…
AndresL230 cb20694
test(mywork): strengthen prActivityCard structured-summary tests to d…
AndresL230 55b11db
feat(mywork): reorder To-do above Previous activity; richer To-do cards
AndresL230 279583e
feat(backfill): retroactively resync PR summaries to the structured f…
AndresL230 2fa5ae9
fix(shared): tolerate a leading AI preamble before the What changed m…
AndresL230 278e035
docs(plan): My Work reorder + structured cards + retroactive summary …
AndresL230 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
955 changes: 955 additions & 0 deletions
955 docs/superpowers/plans/2026-07-03-mywork-restructure-plan.md
Large diffs are not rendered by default.
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
140 changes: 140 additions & 0 deletions
140 docs/superpowers/specs/2026-07-03-mywork-restructure-design.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| # My Work: reorder, structured cards, retroactive summary resync | ||
| Date: 2026-07-03 | ||
| ## Problem | ||
| The My Work screen (`web/src/render.ts`, `myWorkView`) shows two sections: "Previous | ||
| activity" (merged/closed PRs) above "To-do" (open assigned issues). Two things are off: | ||
| 1. To-do — the actionable list — renders below the read-only activity history. It | ||
| should be on top. | ||
| 2. Both sections are thin on structure. `todoCard` is a single truncated line with no | ||
| room for a full title or an updated-at signal. `prActivityCard`'s body is one | ||
| undifferentiated AI-generated prose blob — no labeled fields. | ||
| Separately, "Sync GitHub" (the admin-only backfill button) only looks at PRs closed in | ||
| the last 14 days, and only (re)generates a PR's summary when its event is *newly* | ||
| captured — an already-captured PR's summary is never touched again. That means | ||
| existing PRs would stay on the old unstructured prose format forever, even after this | ||
| change ships. | ||
| ## Design | ||
| ### 1. Reorder (`web/src/render.ts`, `myWorkView`) | ||
| Swap section composition from `${hero}${activity}${todo}` to `${hero}${todo}${activity}`. | ||
| Section labels ("To-do", "Previous activity") stay as-is. | ||
| ### 2. To-do card restructure (`todoCard`) | ||
| From a single-line truncated row to a two-line card, still one `<a class="cnpy-card">`: | ||
| - Row 1: priority badge + `#number` + title. Title wraps up to 2 lines (CSS | ||
| line-clamp) instead of single-line ellipsis truncation. | ||
| - Row 2: up to 3 labels (cap unchanged, keeps the card bounded) + a right-aligned | ||
| relative "updated" timestamp from `t.updatedAt` (captured today, currently unused | ||
| in the card). | ||
| ### 3. Structured PR summaries | ||
| **New shared module `shared/prSummary.ts`** — the single source of truth for | ||
| recognizing the structured-summary convention, imported by both the Worker and the | ||
| web build: | ||
| ```ts | ||
| export interface StructuredPrSummary { | ||
| what: string; | ||
| why: string | null; | ||
| } | ||
| export function parseStructuredSummary(raw: string): StructuredPrSummary | null; | ||
| ``` | ||
| It matches markdown of the shape: | ||
| ``` | ||
| **What changed:** <1-2 factual sentences> | ||
| **Why:** <1 sentence — omitted entirely when no rationale is stated> | ||
| ``` | ||
| Returns `null` when the text doesn't match (old-style prose, the deterministic | ||
| excerpt fallback, or a malformed AI response) — callers treat `null` as "render/treat | ||
| as plain prose," never as an error. | ||
| **Backend (`src/tools/summarize.ts`)** — `workersAiSummarizer`'s system prompt | ||
| changes to require exactly that two-field shape (omitting the `**Why:**` line when | ||
| the PR body states no rationale), instead of "2-3 short sentences." No signature | ||
| change. `excerptSummary` (the deterministic no-AI fallback) is untouched — it keeps | ||
| producing plain prose, which is intentional: a parse miss just falls back to today's | ||
| rendering, never a broken UI. `pr_summaries.summary` stays a single `TEXT` column; no | ||
| migration — the structure lives in the markdown convention, not the schema. | ||
| **Frontend (`web/src/render.ts`, `prActivityCard`)** — the summary body now branches | ||
| on `parseStructuredSummary(pr.summary)`: | ||
| - Matched → two labeled rows ("What changed", and "Why" only when present), each a | ||
| small uppercase caption (scaled-down `MW_LABEL` idiom) above its markdown body. | ||
| - Not matched → today's single prose block, unchanged. | ||
| ### 4. Retroactive resync (`src/tools/backfill.ts`, `runBackfill`) | ||
| Two independent changes to `runBackfill`: | ||
| **a. Full PR history.** Remove the `DAYS_BACK`/cutoff logic on the closed-PR fetch — | ||
| paginate through every closed PR the repo has, not just the last 14 days. The open- | ||
| issues fetch is untouched (it already has no window). | ||
| **b. Decouple summary (re)generation from event-capture outcome.** Today, | ||
| `storePrSummary` only runs inside `if (res.outcome === "written")` — i.e., only for | ||
| brand-new events. Change: for every PR in the fetched list (regardless of whether its | ||
| event was newly captured or already existed), look up its existing `pr_summaries` row | ||
| and call `storePrSummary` unless `parseStructuredSummary(existing.summary) !== null` | ||
| (i.e., skip only when it's already in the new structured format). `ev.raw` (built | ||
| before `ingestEvent` runs) always has the PR's title/body available regardless of the | ||
| event's write outcome, so no extra DB read is needed to get the summarizer input — | ||
| only one extra read (existing `pr_summaries` row) to decide skip-or-regenerate. | ||
| **c. Observability.** `BackfillResult` gains `summarized: number` — incremented each | ||
| time `storePrSummary` actually runs (not skipped). Threaded through: | ||
| - `src/routes.ts` `/admin/backfill` response (already returns the whole result object, | ||
| no route change beyond the type flowing through). | ||
| - `web/src/api.ts` `adminBackfill()` return type. | ||
| - `web/src/main.ts`'s flash message: `Synced: ${r.captured} captured, ${r.unchanged} | ||
| unchanged, ${r.summarized} summaries updated`. | ||
| - `web/src/render.ts`'s button title: "Fetch recent GitHub PRs + issues" → | ||
| "Fetch all GitHub PRs + issues" (no longer just recent). | ||
| **Accepted limitation:** unbounded full-history pagination plus one AI call per | ||
| un-migrated PR has no hard cap or resumable cursor. Fine at this project's current | ||
| size (a first Sync after this ships does a one-time migration of every existing PR to | ||
| the structured format; later clicks are cheap since already-structured PRs are | ||
| skipped). Would need a real bound/cursor if the repo's PR history grows a lot — | ||
| explicitly not building that now (YAGNI). | ||
| ## Testing | ||
| - `test/render.mywork.test.ts` (pure, no DOM): `todoCard` shows `updatedAt` and | ||
| doesn't collapse a long title to one truncated line; `prActivityCard` renders two | ||
| labeled rows for a structured summary (with and without a `why`), and falls back to | ||
| the existing prose rendering for a non-conforming summary. | ||
| - New test file (or colocated in `test/summarize.test.ts`) for | ||
| `shared/prSummary.ts`'s `parseStructuredSummary`: matches the two-field shape, | ||
| matches "What changed" only (no "Why" line), returns `null` for old-style prose and | ||
| for empty/malformed input. | ||
| - `test/backfill.test.ts`: the existing "oldPr excluded by the 14-day window" | ||
| assertion is now wrong on purpose and must be updated to reflect full-history | ||
| fetch. New cases: a PR with an existing non-structured summary gets re-summarized | ||
| on a second run (`summarized` increments, `storePrSummary`/summarizer called | ||
| again); a PR with an existing structured summary is skipped on a second run | ||
| (`summarized` stays 0 for it, summarizer not called again — assert via a | ||
| call-counting stub summarizer). | ||
| - No new migration, so `test/apply-migrations.ts` is untouched. | ||
| ## Out of scope | ||
| - No change to the live webhook capture path (`src/webhook.ts`) — a real-time merge | ||
| event only ever fires once, so the "already captured, skip" question doesn't arise | ||
| there. | ||
| - No change to issue capture/backfill scope (already unbounded). | ||
| - No resumable/bounded backfill cursor for large repos (see accepted limitation | ||
| above). | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // Recognizes the "What changed / Why" markdown convention emitted by the PR | ||
| // summarizer (src/tools/summarize.ts) so both the Worker (the backfill's | ||
| // already-structured check, src/tools/backfill.ts) and the web build (card | ||
| // rendering, web/src/render.ts) agree on what counts as a structured summary. | ||
| // No schema change backs this — pr_summaries.summary stays a single markdown | ||
| // TEXT column; the structure lives in this convention, not a stored shape, so | ||
| // old prose summaries and the deterministic excerpt fallback degrade | ||
| // gracefully to a `null` parse instead of erroring. | ||
| export interface StructuredPrSummary { | ||
| what: string; | ||
| why: string | null; | ||
| } | ||
| const STRUCTURED_RE = /\*\*What changed:\*\*\s*([\s\S]*?)(?:\s*\*\*Why:\*\*\s*([\s\S]*))?$/i; | ||
| /** Parses "**What changed:** ... **Why:** ..." out of a PR summary's markdown. | ||
| * Returns null when the text doesn't match (old-style prose, the excerpt | ||
| * fallback, or a malformed AI response) — callers treat null as "render/treat | ||
| * as plain prose," never as an error. */ | ||
| export function parseStructuredSummary(raw: string): StructuredPrSummary | null { | ||
| const m = raw.match(STRUCTURED_RE); | ||
| if (!m) return null; | ||
| const what = m[1].trim(); | ||
| if (!what) return null; | ||
| const why = m[2]?.trim() || null; | ||
| return { what, why }; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tag the example fence so markdownlint stays green.
The bare fenced block here triggers MD040. Add a language hint (
md/markdown) or convert it to a quoted example so the spec stays lint-clean.✏️ Suggested fix
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Source: Linters/SAST tools