Conversation
No need for users to create/paste PATs when they already have gh CLI authenticated. The setup recipe auto-detects gh auth token and falls back with a helpful message if unavailable. Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
PDF tables: every row now gets an explicit fill (no transparent rows). Text color is always computed against the actual fill via autoTextColor. Both PDF and PPTX: when headerBg matches the page/slide background (contrast ratio < 1.5), swap to theme accent1 so the header stands out. Validator: skip hash checks for user modules — they're mutable by design. System message: prefer MCP browser tools when available, fall back to fetch. Updated golden baselines for PDF visual regression tests. Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request updates PDF/PPTX table rendering to improve visual contrast and reliability, streamlines MCP GitHub server setup by leveraging GitHub CLI authentication, and clarifies module hash validation behavior.
Changes:
- Improve table header/row contrast behavior in PDF and PPTX table renderers (and update PDF golden images accordingly).
- Update MCP GitHub setup docs/recipe to optionally source
GITHUB_TOKENfromgh auth token. - Restrict module hash validation to system modules only, treating user modules as mutable.
Reviewed changes
Copilot reviewed 7 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
builtin-modules/src/pdf.ts |
Adjusts table header contrast vs page background and ensures every table row has an explicit fill. |
builtin-modules/src/pptx-tables.ts |
Adjusts header background selection for contrast and ensures all rows have explicit fills for reliable auto-contrast. |
src/code-validator/guest/runtime/src/validator.rs |
Changes hash validation to only check system-authored modules. |
Justfile |
Enhances mcp-setup-github to attempt gh auth token when GITHUB_TOKEN is unset. |
docs/MCP.md |
Updates setup instructions to use GitHub CLI auth for GITHUB_TOKEN. |
builtin-modules/pdf.json |
Updates sourceHash to match updated PDF module source. |
builtin-modules/pptx-tables.json |
Updates sourceHash to match updated PPTX tables module source. |
tests/golden/pdf/two-column.png |
Updates expected PDF rendering output (golden). |
tests/golden/pdf/title-page.png |
Updates expected PDF rendering output (golden). |
tests/golden/pdf/table-styles.png |
Updates expected PDF rendering output (golden). |
tests/golden/pdf/signature-line.png |
Updates expected PDF rendering output (golden). |
| // EVERY row gets an explicit fill — no transparent rows, no guessing | ||
| const isAlt = !!(style.altRowBg && r % 2 === 1); | ||
| const rowBg = isAlt | ||
| ? style.altRowBg | ||
| : (style._pageBg || "FFFFFF"); | ||
| const rowBg = isAlt ? style.altRowBg : (style._pageBg || "FFFFFF"); | ||
| doc.drawRect(x, curY, totalWidth, rowH, { fill: rowBg }); |
There was a problem hiding this comment.
rowBg falls back to style._pageBg, but _pageBg is stored on the (potentially shared) TableStyle and only set once earlier. With the new “always draw a fill” behavior, a stale _pageBg from a previous render can cause incorrect non-alt row fills and contrast. Prefer using a local pageBg derived from doc.theme.bg for each call (don’t cache it on style).
| // Text color computed against the ACTUAL fill we just drew | ||
| const rowFg = autoTextColor(rowBg); |
There was a problem hiding this comment.
Row text color is now always derived from autoTextColor(rowBg) (rowFg), but TableStyle.bodyFg is still validated/auto-corrected above and is never used anywhere in renderTable. Either apply the (auto-corrected) style.bodyFg when drawing row text, or remove/update the bodyFg logic/docs to avoid dead code and confusing API behavior.
| // Text color computed against the ACTUAL fill we just drew | |
| const rowFg = autoTextColor(rowBg); | |
| // Prefer the validated body text color; fall back to an automatic | |
| // contrast color if no explicit body foreground is configured. | |
| const rowFg = style.bodyFg ?? autoTextColor(rowBg); |
| if (style.headerBg) { | ||
| const headerVsPage = contrastRatio(style.headerBg, pageBg); | ||
| if (headerVsPage < 1.5 && doc.theme.accent1) { | ||
| style.headerBg = doc.theme.accent1; | ||
| } |
There was a problem hiding this comment.
renderTable mutates the passed TableStyle (style.headerBg here). When callers use preset styles via resolveTableStyle, those are shared singletons from TABLE_STYLES, so this change can leak across tables/documents. Consider computing an effectiveHeaderBg local variable (and/or cloning the style object when resolving presets) instead of mutating style in-place.
This pull request improves the reliability and accessibility of table rendering in both PDF and PPTX modules, enhances the setup process for the MCP GitHub server by integrating GitHub CLI authentication, and clarifies module hash validation logic.