Skip to content

[CR-4][P1] Reconstruct PR overview, activity, checks, stack and preview cards #323

Open

Description

@BunsDev

Outcome

Reproduce the top-of-PR and scrolled-activity states R1/R2 in #319 with real PR information, safe Markdown, a truthful checks/stack sidebar and usable preview links. The page must remain useful when some information is missing, stale or inaccessible.

Parent: #319. Blocked by accepted artifacts: #321's account/revision-scoped read service and #322's workspace host; #320 supplies the reference/state contract. CR-5 can proceed in parallel. Produces overview/context selection for CR-6 and the action placement consumed by CR-7. This issue does not publish comments, request reviews or merge.

Context / entry points

Refresh current main, open PRs and AGENTS.md; audit baseline is cb33db5df00e235f68d36b5fac556dc30e0851f2. Use #320's exact source map and #322's accepted component interfaces. Reuse the existing safe Markdown implementation described in docs/status.md and #277, and existing menu/error components. Do not wait for all of #277 to close if its required renderer contract is already accepted.

Proposed files if no suitable equivalent exists: apps/desktop/src/renderer/components/code-review/PrOverview.tsx, PrActivity.tsx, PrSidebar.tsx, plus apps/desktop/e2e/code-review-overview.spec.ts. Keep shared read models in the #320 contract. These new paths are proposals.

Items to do

  • Add failing tests for R1/R2: title/status/author/branch metadata, description, changes totals, activity timeline, reviewers, checks and preview information. Include empty, partial and unavailable variants rather than hard-coded happy-path rows.
  • Render PR descriptions/comments with the existing safe Markdown path. Keep raw HTML inert; validate links and use the existing remote-image/privacy policy. PR text, SVG, filenames and bot comments must not execute scripts, acquire IPC access or silently load arbitrary remote resources.
  • Build a paginated activity timeline with stable event IDs, explicit source/type and deterministic ordering. Distinguish commits, issue comments and submitted reviews; avoid double-counting a review appearing in multiple collections. Preserve the reader's position during refresh and expose remaining/failed pages.
  • Match the compact right sidebar and expandable check rows. Show pending/failed/cancelled/neutral/skipped/unknown states and the revision/freshness of their evidence. Only describe a collection as empty when the read is complete; do not infer merge permission from the number of green checks.
  • Render a numbered PR stack only from an accepted, documented relationship source. Distinguish explicit stack metadata from an inferred branch relationship; label inference and uncertainty. Do not infer dependencies from adjacent issue numbers or PR prose. Handle missing/inaccessible members, branching and cycles without inventing an order. No stack rebasing or mutation in this slice.
  • Show deployment previews with provenance: a provider deployment/check result is different from a URL found in a comment. Mark comment-derived information accordingly. Validate external destinations and open them only on user action using the existing safe external-navigation path. Do not embed a privileged preview or add a new Vercel account integration solely to match the image.
  • Place the activity comment composer and header edit/reviewer controls behind the capabilities/actions supplied later by CR-7. Keep the PR-question composer owned by CR-6. Specify explicit props/callbacks instead of duplicating draft stores or inventing successful submissions.
  • Match the measured text width, whitespace, card density and floating-composer clearance. Preserve independent scrolling and focus on refresh. Include accessible check text, timeline headings and keyboard-operable expanders.

Acceptance criteria

  • AC1: R1 and R2 are reached by selecting and scrolling the same PR; metadata and activity use the real read adapter or clearly identified deterministic fixtures.
  • AC2: pagination/refresh produces no duplicate activity, lost scroll position or cross-PR response bleed. Partial data never becomes a misleading zero count.
  • AC3: stale/unknown checks, inferred stack links and comment-derived deployment previews remain visibly distinct from authoritative current evidence.
  • AC4: hostile Markdown/HTML, unsafe URL schemes and misleading preview links cannot execute content, access credentials or trigger navigation without user action.
  • AC5: long titles, large descriptions and narrow windows remain readable; the composer does not cover the final activity item or keyboard focus.
  • AC6: all visible write controls are either connected to a verified CR-7 capability or explicitly unavailable. This PR performs no GitHub mutations.

Validation / handoff

Add adapter/model tests for timeline deduplication and relationship ambiguity; exercise the actual Electron renderer with Playwright for R1/R2 and malicious-content fixtures. Run pnpm build && pnpm test and pnpm test:e2e. Record exact SHA, commands, reviewed screenshots, source/completeness labels and remaining authenticated/manual gates in docs/reviews/ and docs/status.md. Do not publish real private PR content in fixtures or evidence. Sources: #319, Electron security, GitHub REST practices.

Long-running goal prompt

Implement CR-4 for #319 in TypeSafeAI/modex after reading the live parent, #320/#321/#322 artifacts, current source and scoped AGENTS.md. Verify worktree ownership and current main. Reconstruct R1/R2 through the existing workspace using real read models and the existing safe Markdown renderer. Start with failing tests for timeline pagination, stale checks, ambiguous stack relationships and unsafe preview links. Keep every claim tied to its source/revision; missing data is not zero and inferred relationships are not authoritative. Reuse existing components, preserve scroll/focus and match approved geometry with synthetic content. Do not create another Markdown parser, draft store, Vercel integration or write service. Do not publish, review, merge or run foreign PR code. Implement small tested slices, visually inspect normal/narrow/error states, and checkpoint full SHA, files, exact test results, evidence and one next action. Finish with a signed reviewable PR and explicit limits; stop on missing authority or an unresolved producer contract, not on unrelated issue closure.

Follow-up / resume prompt

Resume CR-4 from current issue discussions, accepted producer artifacts and the actual owned-worktree diff. Rerun the overview/activity smoke test before trusting prior progress. Select one remaining acceptance item; inspect source provenance, pagination and revision state before changing presentation. Preserve other agents' workspace and draft/action ownership. Never replace unknown data with screenshot values or refresh goldens without review. Record what is verified by fixtures versus authenticated GitHub, remaining manual gates and the next action. Do not merge or publish anything.

Independent review prompt

Review R1/R2 with complete, partial, stale and hostile input. Check that stack ordering and deployment status have honest provenance, that checks do not imply merge permission, and that pagination preserves event identity and scroll position. Verify raw content stays inert and every unavailable write control says so. Compare actual rendered evidence to the approved references and report acceptance-specific findings.

Activity

  1. BunsDev commented on Oct 20, 2026

    @BunsDev
    ContributorAuthor

    The CR-1 contract is on main as 43d5b9c (#334). Bind to these files at that commit:

    • apps/desktop/src/shared/code-review.ts (the contract)
    • apps/desktop/test/fixtures/code-review.ts (synthetic fixtures)
    • docs/design/code-review.md (source map, references, inventory, open decisions)

    For CR-4:

    • summarizeChecks gives the verdict. Pending is never passing, and only the newest run counts.
    • The R2 and R3 fixtures and the PrOverview, PrActivity and PrStack types are in the contract.
    • The design page proposes two typography tokens for this slice: a ~22 px PR title and a ~24 px prose pitch.

    Coven-Harness: Claude Code / claude-opus-5-5

  2. mariscate commented on Oct 22, 2026

    @mariscate

    Refreshed goal prompts — 2026-10-22 (main at 0e674e7)

    Where it stands: not started; this is next in Gate A, alongside #324. Everything it needs is on main:

    • Reads: the overview, activity (cursor), checks (revision) and stack operations in apps/desktop/src/shared/code-review.ts, served by src/main/engine/code-review/read-service.ts.
    • Contract: summarizeChecks, the PrActivity union (commit, opened, comment, review, event, deployment) and PrSummary.stackHint.
    • Fixtures: R1–R3 plus the FAULTS.
    • The host: Integrate Code Review inbox and workspace with main-owned PR reads #367. Its placeholder for this slice is in renderer/code-review/PrPane.tsx.
    • Markdown: full Markdown in replies shipped in v0.3.0. Reuse renderer/components/Markdown.tsx.
    • Avatars (C16/C81): still Val's decision, so show logins only.

    Long-running goal prompt

    Implement CR-4 (#323) in TypeSafeAI/modex. Read AGENTS.md, #319, this issue, docs/design/code-review.md (inventory, typography tokens, open decisions) and the shared contract, then verify main's SHA. Create your own worktree (scripts/worktree.sh new cr4-overview). Replace PrPane's overview placeholder with an overview built on the existing host. Put new components in apps/desktop/src/renderer/code-review/ beside the host, not a new components/code-review folder. Start with red tests against the R1/R2/R3 fixtures and FAULTS: activity pagination with stable ids and no duplicate reviews, partial or failed pages that never show zero, checks verdicts from summarizeChecks (pending is never passing, only the newest run counts, old-head runs marked), stackHint/inferred stacks labelled as inferred, deployment rows with provenance, and hostile Markdown/HTML/URLs left inert. Render text with the existing Markdown component and open links through the existing safe external path. Every write control (comment box, edit, reviewers, state menu) shows as unavailable via capabilities(), owned by #326; the PR question composer belongs to #325. Keep scroll and focus across refresh, and keep the floating composer clear of the last item. Run pnpm build && pnpm test && pnpm test:e2e. Review screenshots of normal, narrow, stale and error states. Record evidence in docs/reviews and update docs/status.md. Open one signed PR and don't merge without Val's go-ahead.
    

    Follow-up / resume prompt

    Resume CR-4. Confirm you own the worktree, fetch main, and read this issue's last checkpoint and your PR's review comments. Re-run the overview/activity e2e and the contract tests before trusting earlier results. If #324 landed meanwhile, merge main and resolve any PrPane conflict while keeping both slices' behaviour. Continue with the next unproven AC: pagination and scroll, honest stale/inferred data, or inert content. Don't change goldens without reviewing them. Post a checkpoint here: SHA, files, commands and results, fixture vs authenticated evidence, and one next action.
    
  3. BunsDev commented on Oct 23, 2026

    @BunsDev
    ContributorAuthor

    Refreshed goal prompts — 2026-10-23 (main at 203a350)

    On main already:

    • the reads, contract, fixtures and host this slice builds on (CR-1/2/3);
    • the overview placeholder in renderer/code-review/PrPane.tsx.

    The overview itself has not landed.

    In review: #393 (cr4-overview, head 52375d4, base 68860a0; the implementation is in signed commit 485391a).

    What #393 adds:

    • PR metadata and a safe Markdown description;
    • paginated, filterable activity with stable ids;
    • grouped commits;
    • compact checks that use summarizeChecks;
    • reviewers;
    • stacks labelled as inferred, with ambiguous orderings refused;
    • current-head deployment cards read from GitHub's deployment/status endpoints;
    • unavailable write controls, with callback slots left for CR-7.

    Evidence (2026-10-23-cr4-overview.md):

    • pnpm build && pnpm test: 2,020 passed, 16 skipped. One earlier rerun failed six terminal-cleanup cases; they pass on their own, and the cause is not identified.
    • pnpm test:e2e: 400 desktop passed, 7 skipped.
    • The overview spec passed 9 of 9.
    • An authenticated read-only service smoke ran against Define the Code Review read contract and fixtures (CR-1) #334.
    • CI is green and the PR merges cleanly.

    Still open:

    • an independent review of feat(code-review): add PR overview, activity and evidence cards #393 on GitHub, and Val's go-ahead to merge;
    • the R1/R2 comparison against the originals, which are missing from the manifest's paths;
    • authenticated rendering in Electron, which wasn't exercised;
    • live account switching;
    • human VoiceOver;
    • avatars (C16/C81), which are still Val's decision, so the overview shows logins only.

    CR-5's unpushed cr5-remote-diffs branch also changes PrPane.tsx, CodeReviewView.tsx and code-review.css. Whichever lands second merges main.

    Long-running goal prompt

    Land CR-4 (#323) in TypeSafeAI/modex. The implementation is PR #393 (branch cr4-overview); do not start over.
    
    Read AGENTS.md, #319, this issue, docs/design/code-review.md and #393's evidence doc (docs/reviews/2026-10-23-cr4-overview.md), then verify main's SHA. Work in the cr4-overview worktree only if you own it; otherwise claim it with scripts/worktree.sh claim, or ask its owner.
    
    1. Get an independent review of #393 and fix important findings with red tests first: activity pagination and refresh, stale and inferred data never shown as fresh or empty, inert hostile Markdown, HTML and URLs, unavailable write controls through capabilities(), and focus/scroll across refresh.
    2. If CR-5 lands first, merge main and reconcile PrPane, CodeReviewView and code-review.css so that both slices' behaviour holds.
    3. With Val's go-ahead, run an authenticated Electron render of the overview, separate from the service smoke.
    4. Hand CR-8 (#327) the head SHA so it can add the R2 journey step.
    
    Run pnpm build && pnpm test && pnpm test:e2e. Investigate the terminal-cleanup failures if they recur. Update the evidence doc and docs/status.md. Use signed commits, and don't merge without Val's go-ahead.
    

    Follow-up / resume prompt

    Resume CR-4.
    1. Confirm you own cr4-overview, fetch main, and read #393's review comments and this issue's last checkpoint.
    2. Re-run e2e/code-review-overview.spec.ts and the contract tests on the current head.
    3. If main moved (CR-5 or anything touching PrPane), merge it and re-run the full suite.
    4. Fix the next open review finding, or the weakest AC.
    Don't change goldens without reviewing them. Post a checkpoint here: SHA, commands and results, fixture vs authenticated evidence, what waits on Val, and one next action.
    
  4. self-assigned this
    on Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions