From f5b6701fbd308e240ca6de8e14176798c2350401 Mon Sep 17 00:00:00 2001 From: Kenneth Date: Sun, 7 Jun 2026 21:01:44 +0100 Subject: [PATCH] feat: add md img rendering support --- .../issues.pull_request.PR_kwDOAgent47.json | 2 +- .../issues.pull_request.PR_kwDODesign31.json | 2 +- .../issues.pull_request.PR_kwDOInfra19.json | 2 +- .../issues.pull_request.PR_kwDONovem84.json | 2 +- .../issues.pull_request.PR_kwDONovem85.json | 2 +- .../issues.pull_request.PR_kwDOSprint62.json | 2 +- src/api/mock.rs | 63 +++++++++++++++++++ src/component/markdown.rs | 52 ++++++++++++--- src/component/rich_text.rs | 27 +++++++- 9 files changed, 135 insertions(+), 19 deletions(-) diff --git a/fixtures/github/issues.pull_request.PR_kwDOAgent47.json b/fixtures/github/issues.pull_request.PR_kwDOAgent47.json index a44f63c..d444247 100644 --- a/fixtures/github/issues.pull_request.PR_kwDOAgent47.json +++ b/fixtures/github/issues.pull_request.PR_kwDOAgent47.json @@ -14,5 +14,5 @@ "head_branch_name": "feat/worker-context-envelope", "head_repo_slug": "kennethnym/agent-tooling", "head_ref": "4a8df12be732c0f9e5d194cd2af7430c0d2fb8d4", - "body": "## Goal\n\nSplit context loading from execution workers so delegation stays predictable while this pull request is still in draft.\n\n### Why\n- workers should receive a compact payload\n- prompt packing should be testable without spawning a worker\n- retry policy should stay in one place\n\n### Boundaries\n| Boundary | Responsibility |\n| --- | --- |\n| `ContextLoader` | Hydrate repository and file context |\n| `PromptAssembler` | Build compact worker payloads |\n| `WorkerRunner` | Apply retry policy and collect results |\n\n### Proposed flow\n1. Load repository context once.\n2. Normalize file excerpts and metadata.\n3. Hand workers a stable execution envelope.\n\n```text\nContextLoader -> PromptAssembler -> WorkerRunner\n```\n\n> Draft status stays until we decide whether token counts belong in the worker response.\n\n### Questions\n- Should `ContextLoader` expose cache hit metrics?\n- Should worker retries carry the same prompt hash?\n- Add a regression test for interrupted workers" + "body": "## Goal\n\nSplit context loading from execution workers so delegation stays predictable while this pull request is still in draft. The payload chip ![Worker payload badge](https://placehold.co/24x24.png?text=P) appears inline wherever a worker receives context.\n\n### Why\n- workers should receive a compact payload\n- prompt packing should be testable without spawning a worker\n- retry policy should stay in one place\n\n### Boundaries\n| Boundary | Responsibility |\n| --- | --- |\n| `ContextLoader` | Hydrate repository and file context |\n| `PromptAssembler` | Build compact worker payloads |\n| `WorkerRunner` | Apply retry policy and collect results |\n\n### Proposed flow\n1. Load repository context once.\n2. Normalize file excerpts and metadata.\n3. Hand workers a stable execution envelope.\n\n![Worker context envelope moving from loader to assembler to runner](https://placehold.co/960x540.png?text=Worker+Context+Envelope)\n\n```text\nContextLoader -> PromptAssembler -> WorkerRunner\n```\n\n> Draft status stays until we decide whether token counts belong in the worker response.\n\n### Questions\n- Should `ContextLoader` expose cache hit metrics?\n- Should worker retries carry the same prompt hash?\n- Add a regression test for interrupted workers" } diff --git a/fixtures/github/issues.pull_request.PR_kwDODesign31.json b/fixtures/github/issues.pull_request.PR_kwDODesign31.json index b0acede..2f8fedb 100644 --- a/fixtures/github/issues.pull_request.PR_kwDODesign31.json +++ b/fixtures/github/issues.pull_request.PR_kwDODesign31.json @@ -14,5 +14,5 @@ "head_branch_name": "chore/dashboard-spacing-scale", "head_repo_slug": "kennethnym/design-notes", "head_ref": "5b0cf338ec46d581af0d582da6427a3dfbce9018", - "body": "## Summary\n\nTightens the dashboard spacing scale before the next visual refresh.\n\n### Updated tokens\n- `space.3` for compact sidebar gaps\n- `space.5` for section rhythm\n- `space.8` for page-level separation\n\n| Surface | Before | After |\n| --- | --- | --- |\n| Sidebar section gap | `space.6` | `space.5` |\n| Filter row padding | `space.4` | `space.3` |\n| Dashboard gutter | `space.7` | `space.6` |\n\n### Review notes\n- verify heading baselines still align with list content\n- compare 1280px and 1440px screenshots side by side\n- revisit compact mode once the nav collapse lands\n\n**Design intent:** make dense screens feel more deliberate without looking cramped." + "body": "## Summary\n\nTightens the dashboard spacing scale before the next visual refresh.\n\n### Updated tokens\n- `space.3` for compact sidebar gaps\n- `space.5` for section rhythm\n- `space.8` for page-level separation\n\n| Surface | Before | After |\n| --- | --- | --- |\n| Sidebar section gap | `space.6` | `space.5` |\n| Filter row padding | `space.4` | `space.3` |\n| Dashboard gutter | `space.7` | `space.6` |\n\n### Comparison\n![Dashboard spacing comparison between old and tightened token scale](https://placehold.co/960x540.png?text=Spacing+Scale)\n\n### Review notes\n- verify heading baselines still align with list content\n- compare 1280px and 1440px screenshots side by side\n- revisit compact mode once the nav collapse lands\n\n**Design intent:** make dense screens feel more deliberate ![Spacing approved badge](https://placehold.co/24x24.png?text=A) without looking cramped." } diff --git a/fixtures/github/issues.pull_request.PR_kwDOInfra19.json b/fixtures/github/issues.pull_request.PR_kwDOInfra19.json index 58a3329..ba748f2 100644 --- a/fixtures/github/issues.pull_request.PR_kwDOInfra19.json +++ b/fixtures/github/issues.pull_request.PR_kwDOInfra19.json @@ -14,5 +14,5 @@ "head_branch_name": "docs/manual-failover-steps", "head_repo_slug": "kennethnym/infra-scripts", "head_ref": "6fd11baf0d9d53d18f6d7b7dc265d9b09e6f4217", - "body": "## Context\n\nDocuments the manual failover sequence for the staging stack while the automated recovery path is still unstable.\n\n### Draft runbook\n1. Put the primary deployment in maintenance mode.\n2. Promote the standby database.\n3. Repoint the app workers.\n4. Warm the cache before reopening traffic.\n\n| Step | Owner | State |\n| --- | --- | --- |\n| Promote standby | SRE | Drafted |\n| Repoint workers | App platform | Drafted |\n| DNS validation | Release lead | Pending |\n\n```bash\n./scripts/failover promote-standby --env staging\n./scripts/failover repoint-workers --env staging\n./scripts/failover verify --env staging\n```\n\n> This pull request was closed because the final DNS validation steps were still changing underneath the runbook.\n\n### Remaining gaps\n- secrets rotation is still manual\n- rollback screenshots are missing\n- add the final post-cutover checklist" + "body": "## Context\n\nDocuments the manual failover sequence for the staging stack while the automated recovery path is still unstable. The standby marker ![Standby promoted badge](https://placehold.co/24x24.png?text=S) appears inline with the promotion step.\n\n### Draft runbook\n1. Put the primary deployment in maintenance mode.\n2. Promote the standby database.\n3. Repoint the app workers.\n4. Warm the cache before reopening traffic.\n\n| Step | Owner | State |\n| --- | --- | --- |\n| Promote standby | SRE | Drafted |\n| Repoint workers | App platform | Drafted |\n| DNS validation | Release lead | Pending |\n\n### Runbook diagram\n![Manual failover runbook sequence with standby promotion](https://placehold.co/960x540.png?text=Manual+Failover+Runbook)\n\n```bash\n./scripts/failover promote-standby --env staging\n./scripts/failover repoint-workers --env staging\n./scripts/failover verify --env staging\n```\n\n> This pull request was closed because the final DNS validation steps were still changing underneath the runbook.\n\n### Remaining gaps\n- secrets rotation is still manual\n- rollback screenshots are missing\n- add the final post-cutover checklist" } diff --git a/fixtures/github/issues.pull_request.PR_kwDONovem84.json b/fixtures/github/issues.pull_request.PR_kwDONovem84.json index e71d84b..7f9c4da 100644 --- a/fixtures/github/issues.pull_request.PR_kwDONovem84.json +++ b/fixtures/github/issues.pull_request.PR_kwDONovem84.json @@ -14,5 +14,5 @@ "head_branch_name": "feat/cached-issue-pane", "head_repo_slug": "kennethnym/novem", "head_ref": "2bc41de7731b9ef48f7d64ee9f0d5f497dbe0a51", - "body": "## Summary\n\nHydrates the dashboard issue pane from cached query state so selection and scroll position stay stable during refetches.\n\n### Rendering coverage\n- headings\n- bullet lists\n- inline code like `use_query`\n- tables\n\n### Implementation sketch\n```rust\nlet cached = query_store.read(key);\nlet selection = cached.and_then(|data| data.selected_issue_id.clone());\n```\n\n| Case | Expected behavior |\n| --- | --- |\n| Cache hit | Keep the current selection pinned |\n| Cache miss | Fall back to the first visible item |\n| Refetch in flight | Preserve scroll position |\n\n### Follow-up\n- mirror the same cache behavior in the pull request detail pane\n- add a smoke test around keyboard navigation during refetch\n\nSee also the [query store](src/query.rs) integration notes." + "body": "## Summary\n\nHydrates the dashboard issue pane from cached query state so selection and scroll position stay stable during refetches. The active-row chip ![Selection stable badge](https://placehold.co/24x24.png?text=OK) stays visible beside the selected issue.\n\n### Rendering coverage\n- headings\n- bullet lists\n- inline code like `use_query`\n- tables\n\n### Implementation sketch\n```rust\nlet cached = query_store.read(key);\nlet selection = cached.and_then(|data| data.selected_issue_id.clone());\n```\n\n| Case | Expected behavior |\n| --- | --- |\n| Cache hit | Keep the current selection pinned |\n| Cache miss | Fall back to the first visible item |\n| Refetch in flight | Preserve scroll position |\n\n### Preview\n![Dashboard issue pane preserving selection during refetch](https://placehold.co/960x540.png?text=Dashboard+Issue+Pane)\n\n### Follow-up\n- mirror the same cache behavior in the pull request detail pane\n- add a smoke test around keyboard navigation during refetch\n\nSee also the [query store](src/query.rs) integration notes." } diff --git a/fixtures/github/issues.pull_request.PR_kwDONovem85.json b/fixtures/github/issues.pull_request.PR_kwDONovem85.json index a59dff3..6830575 100644 --- a/fixtures/github/issues.pull_request.PR_kwDONovem85.json +++ b/fixtures/github/issues.pull_request.PR_kwDONovem85.json @@ -14,5 +14,5 @@ "head_branch_name": "feat/cached-repo-picker", "head_repo_slug": "kennethnym/novem", "head_ref": "13af7d0b48a6ce0b22d48c9b6c1c78dfcd94e6a0", - "body": "## Summary\n\nIntroduces a cached repository query so the titlebar picker can switch context without hitting GitHub on every open.\n\n### Why\n- reduces flicker while the picker opens\n- keeps recent repositories visible during short reconnects\n- avoids duplicate requests when the titlebar rerenders\n\n### Cache rules\n- explicit refresh invalidates the cached list\n- fresh network data still wins when available\n- empty responses should not overwrite a warm cache\n\n**Fast path:** render the warm cache immediately.\n*Background refresh* still reconciles stale rows.\n~~Empty refreshes~~ should never clear visible repositories.\n\n| Cache path | Expected behavior |\n| --- | --- |\n| Warm cache | Render repositories before the refresh finishes |\n| Refresh success | Replace cached rows with fresh network data |\n| Empty response | Keep the previous warm cache intact |\n\n```text\nopen picker -> read cache -> render immediately -> refresh in background\n```\n\n### Follow-up\n1. Measure cache hit rate in debug builds.\n2. Add eviction telemetry.\n3. Consider persisting the last successful repository list across launches." + "body": "## Summary\n\nIntroduces a cached repository query so the titlebar picker can switch context without hitting GitHub on every open.\n\n### Why\n- reduces flicker while the picker opens\n- keeps recent repositories visible during short reconnects\n- avoids duplicate requests when the titlebar rerenders\n\n### Cache rules\n- explicit refresh invalidates the cached list\n- fresh network data still wins when available\n- empty responses should not overwrite a warm cache\n\n**Fast path:** render the warm cache immediately with ![Warm cache badge](https://placehold.co/24x24.png?text=W) beside cached rows.\n*Background refresh* still reconciles stale rows.\n~~Empty refreshes~~ should never clear visible repositories.\n\n| Cache path | Expected behavior |\n| --- | --- |\n| Warm cache | Render repositories before the refresh finishes |\n| Refresh success | Replace cached rows with fresh network data |\n| Empty response | Keep the previous warm cache intact |\n\n### Screenshot\n![Repository picker showing warm cache while refresh runs](https://placehold.co/960x540.png?text=Repo+Picker+Cache)\n\n```text\nopen picker -> read cache -> render immediately -> refresh in background\n```\n\n### Follow-up\n1. Measure cache hit rate in debug builds.\n2. Add eviction telemetry.\n3. Consider persisting the last successful repository list across launches." } diff --git a/fixtures/github/issues.pull_request.PR_kwDOSprint62.json b/fixtures/github/issues.pull_request.PR_kwDOSprint62.json index 10a37d7..3e3776b 100644 --- a/fixtures/github/issues.pull_request.PR_kwDOSprint62.json +++ b/fixtures/github/issues.pull_request.PR_kwDOSprint62.json @@ -14,5 +14,5 @@ "head_branch_name": "feat/release-handoff-checklist", "head_repo_slug": "kennethnym/sprint-planner", "head_ref": "be7a8114a57f3e9d214cb9af457c10fd6c5a0b21", - "body": "## Release handoff checklist\n\nAdds the release checklist views and closes the loop for the May rollout.\n\n### Included\n- launch readiness checklist for QA, docs, and release engineering\n- handoff status badges in the weekly planner\n- empty-state copy for weeks without a scheduled release\n\n| Stage | Owner | Status |\n| --- | --- | --- |\n| QA sign-off | `@mariahops` | Done |\n| Docs publish | `@rorycraft` | Done |\n| Release window confirm | `@kennethnym` | Done |\n\n### Verification\n1. Open a release week and confirm checklist sections render in order.\n2. Mark each handoff item complete and confirm the summary badge updates.\n3. Review the planner on a narrow viewport.\n\n> The merged version intentionally keeps the checklist readable even when one section has no pending items.\n\n- QA sign-off state is visible\n- Docs handoff state is visible\n- Add screenshot coverage for the compact layout" + "body": "## Release handoff checklist\n\nAdds the release checklist views and closes the loop for the May rollout, with the done marker ![Release done badge](https://placehold.co/24x24.png?text=D) shown inline beside completed stages.\n\n### Included\n- launch readiness checklist for QA, docs, and release engineering\n- handoff status badges in the weekly planner\n- empty-state copy for weeks without a scheduled release\n\n| Stage | Owner | Status |\n| --- | --- | --- |\n| QA sign-off | `@mariahops` | Done |\n| Docs publish | `@rorycraft` | Done |\n| Release window confirm | `@kennethnym` | Done |\n\n### Screenshot\n![Weekly planner release checklist with all handoff items complete](https://placehold.co/960x540.png?text=Release+Checklist)\n\n### Verification\n1. Open a release week and confirm checklist sections render in order.\n2. Mark each handoff item complete and confirm the summary badge updates.\n3. Review the planner on a narrow viewport.\n\n> The merged version intentionally keeps the checklist readable even when one section has no pending items.\n\n- QA sign-off state is visible\n- Docs handoff state is visible\n- Add screenshot coverage for the compact layout" } diff --git a/src/api/mock.rs b/src/api/mock.rs index 1a25eb7..69b07da 100644 --- a/src/api/mock.rs +++ b/src/api/mock.rs @@ -161,6 +161,39 @@ mod tests { assert!(body.contains("~~Empty refreshes~~")); } + fn assert_markdown_image(body: &str, alt: &str) { + let image_marker = format!("![{alt}](https://placehold.co/"); + assert!( + body.contains(&image_marker), + "pull request markdown fixture should contain image alt text {alt:?}" + ); + } + + fn assert_markdown_inline_image(body: &str, alt: &str) { + let image_marker = format!("![{alt}](https://placehold.co/24x24.png"); + let line = body + .lines() + .find(|line| line.contains(&image_marker)) + .unwrap_or_else(|| { + panic!("pull request markdown fixture should contain inline image {alt:?}") + }); + + let start = line.find(&image_marker).unwrap(); + let end = line[start..] + .find(')') + .map(|offset| start + offset + 1) + .expect("inline image should have a closing paren"); + + assert!( + !line[..start].trim().is_empty(), + "inline image {alt:?} should have text before it" + ); + assert!( + !line[end..].trim().is_empty(), + "inline image {alt:?} should have text after it" + ); + } + #[test] fn list_pull_request_fixtures_parse_with_current_filter_strings() { let authored = list_pull_requests(Some("author:@me state:open"), 1) @@ -191,6 +224,11 @@ mod tests { assert_eq!(merged.state, issues::PullRequestState::Merged); assert_markdown_table(&merged.body, "| Stage | Owner | Status |"); + assert_markdown_image( + &merged.body, + "Weekly planner release checklist with all handoff items complete", + ); + assert_markdown_inline_image(&merged.body, "Release done badge"); assert_eq!( merged.author.as_ref().map(|author| author.login.as_ref()), Some("rorycraft") @@ -210,6 +248,11 @@ mod tests { .contains("./scripts/failover promote-standby") ); assert_markdown_table(&documented_failover.body, "| Step | Owner | State |"); + assert_markdown_image( + &documented_failover.body, + "Manual failover runbook sequence with standby promotion", + ); + assert_markdown_inline_image(&documented_failover.body, "Standby promoted badge"); assert_eq!( documented_failover .author @@ -228,6 +271,11 @@ mod tests { ); assert!(dashboard_markdown.body.contains("```rust")); assert_markdown_table(&dashboard_markdown.body, "| Case | Expected behavior |"); + assert_markdown_image( + &dashboard_markdown.body, + "Dashboard issue pane preserving selection during refetch", + ); + assert_markdown_inline_image(&dashboard_markdown.body, "Selection stable badge"); assert_eq!(dashboard_markdown.base_branch_name.as_ref(), "main"); assert_eq!( dashboard_markdown.head_branch_name.as_ref(), @@ -257,6 +305,11 @@ mod tests { "| Cache path | Expected behavior |", ); assert_markdown_emphasis(&cached_repo_picker.body); + assert_markdown_image( + &cached_repo_picker.body, + "Repository picker showing warm cache while refresh runs", + ); + assert_markdown_inline_image(&cached_repo_picker.body, "Warm cache badge"); assert_eq!(cached_repo_picker.base_branch_name.as_ref(), "main"); assert_eq!( cached_repo_picker.head_branch_name.as_ref(), @@ -282,6 +335,11 @@ mod tests { Some("leaferiksen") ); assert_markdown_table(&worker_split.body, "| Boundary | Responsibility |"); + assert_markdown_image( + &worker_split.body, + "Worker context envelope moving from loader to assembler to runner", + ); + assert_markdown_inline_image(&worker_split.body, "Worker payload badge"); assert_eq!(worker_split.base_branch_name.as_ref(), "main"); assert_eq!( worker_split.head_branch_name.as_ref(), @@ -299,6 +357,11 @@ mod tests { Some("mariahops") ); assert_markdown_table(&spacing_tokens.body, "| Surface | Before | After |"); + assert_markdown_image( + &spacing_tokens.body, + "Dashboard spacing comparison between old and tightened token scale", + ); + assert_markdown_inline_image(&spacing_tokens.body, "Spacing approved badge"); assert_eq!(spacing_tokens.base_branch_name.as_ref(), "main"); assert_eq!( spacing_tokens.head_branch_name.as_ref(), diff --git a/src/component/markdown.rs b/src/component/markdown.rs index a018b45..c10f5a9 100644 --- a/src/component/markdown.rs +++ b/src/component/markdown.rs @@ -172,8 +172,6 @@ impl MarkdownText { theme: &theme::Theme, parent_style: Option, ) { - let node_start_byte = cursor.node().start_byte(); - let style = parent_style.unwrap_or_default(); cursor.goto_first_child(); @@ -181,13 +179,6 @@ impl MarkdownText { loop { let node = cursor.node(); - macro_rules! node_range { - () => { - (node.start_byte() - node_start_byte - byte_offset) - ..(node.end_byte() - node_start_byte - byte_offset) - }; - } - match node.kind_id() { | MARKDOWN_KIND_ID_TEXT => { let start = node.start_byte() + byte_offset; @@ -264,8 +255,49 @@ impl MarkdownText { cursor.goto_parent(); } + | MARKDOWN_KIND_ID_IMAGE => { + cursor.goto_first_child(); + + let (caption, src) = + if cursor.node().kind_id() == MARKDOWN_KIND_ID_LINK_DESTINATION { + // this image node has a source with no caption + (None, cursor.node().utf8_text(content.as_ref()).ok()) + } else { + debug_assert!( + cursor.node().kind_id() == MARKDOWN_KIND_ID_IMAGE_DESCRIPTION + ); + + let caption = cursor.node().utf8_text(content.as_ref()).ok(); + if cursor.goto_next_sibling() { + debug_assert!( + cursor.node().kind_id() == MARKDOWN_KIND_ID_LINK_DESTINATION + ); + (caption, cursor.node().utf8_text(content.as_ref()).ok()) + } else { + (None, None) + } + }; + + match (caption, src) { + | (_, None) => { + // if no src is specified for the image node + // skip it entirely + } + + | (caption, Some(src)) => { + builder.push_image(caption, src.to_owned()); + } + } + + cursor.goto_parent(); + } + | _ => { - // extend here to support more styles + println!( + "rich text not implemented for node {} id {}", + node.kind(), + node.kind_id() + ); } }; diff --git a/src/component/rich_text.rs b/src/component/rich_text.rs index 4baa389..59f15f0 100644 --- a/src/component/rich_text.rs +++ b/src/component/rich_text.rs @@ -1,6 +1,6 @@ use std::rc::Rc; -use gpui::{IntoElement, ParentElement, Styled, div, px}; +use gpui::{ParentElement, Styled, StyledImage, div, img, px}; use crate::{app, util::syntax_highlight}; @@ -45,7 +45,7 @@ enum RichTextElement { }, Image { src: gpui::SharedString, - description: gpui::SharedString, + caption: Option, }, } @@ -89,6 +89,14 @@ impl RichTextContentBuilder { }); } + pub(crate) fn push_image(&mut self, caption: Option<&str>, src: String) { + let start = self.raw_content.len(); + self.annotations.push(Annotation::Image { + src: src.into(), + range: start..start, + }) + } + pub(crate) fn build(&self) -> RichTextContent { let mut text_start = 0; let mut text_end = 0; @@ -129,6 +137,12 @@ impl RichTextContentBuilder { link_i_offset, }); + elements.push(RichTextElement::Image { + src: src.clone(), + // todo: add image caption support + caption: None, + }); + highlights.clear(); link_ranges.clear(); link_i_offset = links.len(); @@ -207,7 +221,14 @@ impl gpui::RenderOnce for RichText { ) } } - | RichTextElement::Image { src, description } => todo!(), + | RichTextElement::Image { src, .. } => { + div().min_w_0().min_h_0().max_w_full().max_h_80().child( + img(src.clone()) + .w_full() + .max_h_80() + .object_fit(gpui::ObjectFit::ScaleDown), + ) + } }); div()