Skip to content

fix(web): keep review badges at their display size - #8314

Closed
MohtashamMurshid wants to merge 1 commit into
pingdotgg:mainfrom
MohtashamMurshid:fix/pr-markdown-image-sizing
Closed

MohtashamMurshid wants to merge 1 commit into
pingdotgg:mainfrom
MohtashamMurshid:fix/pr-markdown-image-sizing

fix(web): keep review badges at their display size

71d37cd
Select commit
Loading
Failed to load commit list.
MacroscopeApp / Macroscope - UI Consistency succeeded Aug 26, 2026 in 3m 21s

All clear

All clear

Details

Note

Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.


🔍 Agent activity — 11 tool calls

💭 I'm considering the effect of removing w-auto from the image size class—checking whether images without explicit width could get stretched by other global CSS rules, so I want to look at where this class is actually used in the code.

1. git_tools — CHAT_MARKDOWN_IMAGE_SIZE_CLASS_NAME

2. git_tools — img

💭 I'm wondering if removing w-auto could break something, since Tailwind's preflight already applies max-width: 100%; height: auto; to images by default in both v3 and v4, so that class might be redundant rather than essential. Should probably trace when w-auto was originally added to confirm it's safe to drop.

3. git_tools

4. browse_code — apps/web/src/components/ChatMarkdown.tsx

💭 I'm checking whether removing w-auto would break the workspace image variant, since it uses the same size class without an explicit width attribute, and Tailwind's preflight only sets max-width and auto height, not width. I want to make sure no other markdown image styling elsewhere relies on that class before removing it. I'm also double-checking that if an image has an explicit width/height attribute from raw HTML (like the picture/source test case), the max-width cap and h-auto still scale it proportionally, and confirming the sanitize schema allows those width/height attributes to pass through. The other spot to check is the workspace image class name — removing w-auto there shouldn't matter since replaced elements use intrinsic width by default with display:block regardless. I should double-check whether there's a base CSS rule forcing img width to 100% that I might've missed in an earlier truncated grep. Let me broaden the search across all CSS files including packages, and also verify class merge ordering doesn't matter here. The bigger risk is whether the rehype-sanitize schema strips width/height attributes on img/picture elements, which would break the test's assumption — worth confirming since that's a real correctness concern rather than just a style nit.

5. git_tools — picture|srcSet|srcset|"width"|"height"

6. git_tools — img|\bwidth\b

💭 I'm puzzled by the discrepancy in line numbers between the two greps at seemingly the same revision — the first showed lines around 2081-2094, but now it's 2213. Maybe the earlier search had a different match limit or context setting that shifted the reported range. I'm also checking whether raw HTML like picture/source tags actually renders — since the ChatMarkdown grep for srcset came up empty, there may be a sanitize schema elsewhere controlling allowed tags and attributes, which matters for whether the test's dark-mode media assertion would even pass. I'm weighing whether the width/height attributes still work fine alongside the Tailwind classes for object-contain and max-width, and considering edge cases like high-DPI button images sized via explicit width attributes.

7. git_tools — rehypeSanitize|defaultSchema|sanitizeSchema

💭 I'm trying to recall the specific attributes hast-util-sanitize's default schema permits for img tags versus the global wildcard attributes list, to figure out which ones apply broadly across elements. Now I'm checking whether picture and source tags are included among the allowed tag names in the default schema. I'm uncertain if source elements survive sanitization to keep that media attribute, but it seems like that's a detail I don't need to resolve myself since it's likely already been tested. More important is whether removing w-auto from that shared class actually matters for workspace images versus markdown images, since it's a class both consumers rely on. I should check for consistency by searching for other markdown renderers, like a PR review component, that might also use w-auto for images, to see if this pattern is already applied elsewhere.

8. git_tools — w-auto

9. git_tools — prose

💭 I'm confirming no other CSS applies width to markdown images, so w-auto isn't needed for the workspace image's block display inside .chat-markdown. I want to check the commit that introduced this class to see if it was intentional or tied to a specific test case.

10. git_tools — 77c9d1e

💭 I figure w-auto was just belt-and-braces, so removing it seems intentional. I'm also noting that the workspace image path already drops width/height attributes since ChatMarkdownWorkspaceImage doesn't forward them from markdown — that's a pre-existing asymmetry, not something this change worsens. The other paths look unaffected: the direct image path's className composition is untouched, and removing w-auto just means an oversized width attribute gets capped by max-width with height following automatically. Keeping h-auto means intrinsic aspect ratio wins over any explicit height attribute, which matches the stated intent — even if the requested height occasionally mismatches intrinsic proportions, it won't cause visible distortion. For a tall image constrained by both a width attribute and max-h-[30rem], object-contain letterboxes the result whether or not w-auto was present, so the before/after behavior is identical there too. I don't see other components using a similar w-auto sizing class that would need the same fix — the pull request markdown renderer likely reuses this same ChatMarkdown component anyway. Looking at the new test file, it renders images with a direct https path and cwd set, and its assertion just checks that the whole rendered markup never contains "w-auto" — that's a bit brittle as a blanket negative check, but it's a test-quality nitpick rather than a real rule violation, so probably not worth flagging. I also want to check whether this test mocks the same modules as the other ChatMarkdown test file, since that one uses heavier mocking while this one may render more directly. I'm also wondering whether the sanitize schema actually preserves srcSet on the source element — if it strips it, the picture/source structure the test asserts on wouldn't actually do anything, meaning the light-mode image would never show and the dark variant would always render regardless of theme. That would be a pre-existing sanitization gap, but since this diff's test explicitly asserts the picture markup as if it works, it arguably brings that gap into scope for this review. I'm trying to recall the full list of allowed tag names in the default schema, but I'm unsure whether 'picture' and 'source' are actually included—that may have been added in a later version. I should verify against the actual installed package version rather than relying on memory.

11. git_tools — hast-util-sanitize|rehype-sanitize

💭 I'm trying to recall the exact structure of hast-util-sanitize's default GitHub schema, including which tags and attributes it allows for elements like links and tables. I don't think there are real findings here — the CI tests cover this and the migration is tiny/visual-only. Thinking through whether removing w-auto changes behavior for the workspace image, I conclude it doesn't: width: auto is CSS's initial value anyway, so the class was effectively a no-op except for overriding HTML width attributes, meaning before/after rendering should be identical. Now I'm checking whether removing w-auto could let an HTML width attribute make an image render larger than before inside a constrained container, like a table cell with max-content sizing — but the max-w-[min(100%,30rem)] constraint should still cap it appropriately. For rasters with large intrinsic widths, the outcome ends up similar either way, but for smaller fixed-width images like button icons, this change actually gives a more correct, reduced size. I'm also noting a similar edge case with h-auto still overriding an image's height attribute. I'm weighing whether the retained h-auto counts as pre-existing behavior the patch makes directly relevant, since it sits on the same changed line as the width fix. Digging deeper, h-auto isn't protecting aspect ratio (that's already default) — it's specifically overriding HTML height attributes the same way the removed w-auto used to override width attributes, so for badges specifying both width and height, this asymmetry could still cause oversized rendering. Actually, Tailwind's preflight already sets height: auto on images to prevent distortion, so h-auto in the class is essentially redundant — removing it wouldn't restore height-attribute sizing anyway, meaning that edge case is already broken by preflight independent of this PR and shouldn't be flagged as a finding here. Confirming this is Tailwind v4 with its standard preflight rules setting max-width and height on img/video elements, so once w-auto is removed, the width attribute will be properly honored — the fix holds up. I want to double check that the direct rendering path still spreads through width and height props correctly. Since there's nothing left to flag, I'll conclude with no findings — the response should indicate everything is clear, formatted to match the expected JSON schema with success state and "All clear" as both title and summary.


Agent Credits: 96 credits