Render GFM tables in message and comment bodies (#405) - #560
Closed
jeremy wants to merge 1 commit into
Closed
Conversation
Register goldmark's table extension in mdConverter so Markdown tables convert to <table> HTML instead of leaking as raw pipe rows. Use the align-attribute cell method rather than the default inline style, since BC3's sanitizer whitelists the align attribute but strips style, so column alignment survives sanitization. BC3's WrapTablesFilter supplies the figure wrapper, so a bare <table> is correct. Because the CLI can now author tables, guard against destroying them: - IsMarkdown gains AST-based table detection so a table-only chat message is sent as HTML rather than verbatim pipes. - IsHTML recognizes <table> so a plain table response is routed through the display converter instead of leaking raw markup to the reader. - The TUI in-place editors (message body, comment, to-do description) fail closed on table-bearing content: HTMLToMarkdown has no table handling, so they refuse to open rather than flatten the table on resubmit. Round-trip editing waits on server-side Markdown (BC3 #11986). Document the table boundary in the Basecamp skill.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes #405 by enabling GitHub Flavored Markdown (GFM) table rendering in message/comment bodies, ensuring table-only content is correctly detected as Markdown/HTML, and preventing the TUI from “round-tripping” table content through HTML→Markdown in-place editors in a way that would silently destroy table structure.
Changes:
- Register Goldmark’s table extension (configured to emit
alignattributes) so Markdown tables render as<table>HTML and preserve column alignment through BC3 sanitization. - Improve content-type heuristics: detect Markdown tables via AST parsing and treat
<table>as HTML for display routing. - Add fail-closed TUI edit guards (with tests) that block in-place editing when HTML contains tables, avoiding destructive re-submits; document the boundary in the Basecamp skill.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/basecamp/SKILL.md | Documents table support and the TUI edit boundary for table-bearing content. |
| internal/tui/workspace/widget/composer_test.go | Adds coverage ensuring table-only submissions aren’t misclassified as plain text. |
| internal/tui/workspace/views/todos.go | Blocks in-place editing of to-do descriptions containing HTML tables. |
| internal/tui/workspace/views/todos_test.go | Tests the fail-closed edit guard for table-bearing descriptions. |
| internal/tui/workspace/views/detail.go | Blocks in-place editing for message bodies and comments containing HTML tables. |
| internal/tui/workspace/views/detail_test.go | Tests table edit guards for both message bodies and comments. |
| internal/richtext/richtext.go | Adds table rendering + table-aware Markdown/HTML detection and introduces HasTableHTML. |
| internal/richtext/richtext_test.go | Adds regression tests for table rendering/alignment and detection helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| return false | ||
| return hasMarkdownTable(s) |
Comment on lines
+1395
to
+1397
| // reTableHTML matches a real <table> tag (open tag or self-closing), distinct | ||
| // from the Markdown table detector. Used to gate the fail-closed TUI edit paths. | ||
| var reTableHTML = regexp.MustCompile(`(?i)<table[\s>]`) |
Member
Author
1 task
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.
Fixes #405.
GFM Markdown tables in message/comment bodies rendered as raw pipe text instead of
<table>HTML — every other Markdown construct converted, tables were the exception.mdConverterregistered onlyextension.Strikethrough, so pipe rows parsed as plain paragraphs.Changes
extension.NewTable(WithTableCellAlignMethod(TableCellAlignAttribute)). This emits the whitelistedalignattribute rather than the defaultstyle="text-align:…", which BC3's sanitizer strips (onlycolor/background-colorCSS survive; thealignattribute is whitelisted — verified inbc3 app/helpers/content_pipeline/sanitization.rb). So GFM column alignment survives sanitization. Bare<table>is correct — BC3'sWrapTablesFiltersupplies the<figure class="lexxy-content__table-wrapper">wrapper.IsMarkdowngains AST-based table detection (parses with the table extension, walks foreast.KindTable) so a table-only chat message is sent as HTML, not raw pipes.IsHTMLrecognizes<table>so a plain BC3 table response routes through the display converter instead of leaking raw markup to the reader.richtext.HasTableHTML; the TUI in-place editors (message body, comment, to-do description) refuse to open table-bearing content rather than flatten it —HTMLToMarkdownhas no table handling, so entering edit mode and resubmitting would strip the table. Round-trip editing waits on server-side Markdown (BC3 #11986).Relationship to #498
@savtrip's #498 makes the one-line extension registration. This PR carries the align-attribute correction (so column alignment isn't lost to the sanitizer) plus the surrounding work that keeps the CLI from destroying tables now that it can author them. The align-attribute fix was also offered to #498 as an inline suggestion; this PR is the home for the broader AST-detection + display-routing + edit-guard + tests changes that don't fit an inline suggestion.
Testing
bin/cifully green (fmt, vet, lint, unit, e2e, surface, skill drift, provenance, mod tidy). New tests cover the #405 golden repro, column alignment,IsMarkdown/IsHTML/HasTableHTMLtable cases, the composer table-only regression, and the fail-closed edit guards.Summary by cubic
Render GFM tables in message and comment bodies as real HTML tables and preserve column alignment through BC3 sanitization. Also detect and display tables correctly, and block in‑place edits that would strip them. Fixes #405.
goldmarktable extension using align attributes instead of inline styles; emit bare<table>(BC3 wraps it).IsHTMLnow recognizes<table>and routes BC3 table responses through the display converter.Written for commit 601b272. Summary will update on new commits.