fix(chat): honour column alignment in markdown tables#5591
Open
BrunooMoniz wants to merge 1 commit into
Open
Conversation
The pipe-table pass in mdToHtml discarded the GFM separator row's alignment markers and wrote a hard-coded `text-align: left` on every cell, so `:---:` (centre) and `---:` (right) columns rendered exactly like a left column. Parse the separator row once, map each column (`:---` left, `:---:` center, `---:` right, plain `---` keeps the left default), and apply that alignment to the column's header and body cells. Columns without a marker keep the previous left default, so existing tables render unchanged. Adds node-driven regression tests covering left/centre/right and the default-left case.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
The chat markdown renderer (
mdToHtmlinstatic/js/markdown.js) parsed GFM pipe tables but discarded the separator row's alignment markers, writing a hard-codedtext-align: lefton every<th>/<td>. As a result a:---:(centre) or---:(right) column rendered exactly like a left column, so numeric columns and centred labels never lined up the way the Markdown asked. This PR parses the separator row once, computes a per-column alignment (:---left,:---:centre,---:right, and a plain---keeps the existing left default), and applies that alignment to each column's header and body cells. Columns with no alignment marker keep the previous left default, so existing tables render unchanged — only:---:and---:columns move. The change is limited to the table pass; it reuses the separator-row detection already present and the cell's existing inline style (no new CSS variables, colours, fonts, or classes).Target branch
dev, notmain. All PRs land indev;mainis curated by the maintainer at each release. If your PR is onmainby accident, click "Edit" on this PR and change the base.Linked Issue
Fixes #5590
Type of Change
Checklist
devdocker compose uporuvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.How to Test
Steps a reviewer can reproduce (full app):
docker compose up -d --build, open the chat.dev: every column is left-aligned —PriorityandScoreignore the:---:/---:markers. With this branch:Priorityis centred andScoreis right-aligned,Featurestays left. Inspecting the cells in DevTools showstext-align: center/righton the aligned columns.What was verified for this change:
dev(before) vs this branch (after) — see the screenshots below. The computedtext-alignon the header cells is["left","left","left"]before and["left","center","right"]after.node --check static/js/markdown.js— passes.python -m pytest tests/test_markdown_rendering_js.py— 15 passed (2 new alignment cases plus the existing table / list / thinking / autolink / math contracts).python -m pytest(full suite) — no new failures versus a cleandevbaseline (4620 passed / 5 skipped, i.e. the baseline 4618 passed / 5 skipped plus the 2 new cases).Files changed:
static/js/markdown.js— the pipe-table pass now derives per-column alignment from the separator row and applies it to<th>/<td>instead of a hard-codedtext-align: left. Columns without a marker keep the left default.tests/test_markdown_rendering_js.py— new cases asserting:---/:---:/---:map to left / centre / right on both header and body cells, and that a plain---column stays left.Visual / UI changes — REQUIRED if you touched anything that renders
Anything that changes what the UI looks like — buttons, icons, padding, colors, fonts, spacing, layout, CSS, HTML, SVG, or any
static/js/module that draws to the DOM — needs all of the following.mdToHtml) andstatic/style.cssin Chromium (desktop, 1280px) and WebKit (mobile, 390px) at the default dark theme, before (currentdev) vs after (this branch).--red,--fg,--bg,--card,--border, etc.) — do not introduce new color values, font sizes, or spacing units.static/index.html) or plain text.Fira Code) for primary UI text. Don't override.Screenshots / clips
Desktop — before (current
dev)Desktop — after (this PR)
Mobile — before (current
dev)Mobile — after (this PR)