feat(richtext): add GitHub Markdown <-> AgilePlace HTML translation layer#77
Merged
Conversation
Lays the foundation for the GitHub Markdown <-> AgilePlace HTML translator (#64): module docstring embedding the supported-subset and degradation tables, the escape-char constant sets, and the shared pure sanitizers both translation directions will depend on -- _escape_html_text, _escape_markdown_text/_unescape_markdown_text (a true inverse pair, keyed on at_line_start so structural markers like '#'/'-'/'>' only escape when they'd actually open a line), and _sanitize_href (closes tab/space/newline scheme-obfuscation bypasses on javascript:/data: hrefs). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds _MarkdownWalker (stdlib html.parser) translating AgilePlace's observed HTML subset -- p, br, strong, em, u, code, pre>code -- into Markdown, plus the public leankit_html_to_markdown() entry point. A format_stack tracks pending inline-format closers so an unclosed <strong>/<em>/<code> force-closes at end of input instead of leaving a dangling marker. <u> and any tag outside the current vocabulary take a unified degrade path (tag dropped, text kept); <script>/<style> content is suppressed rather than leaked as prose. Tests pin content conservation across the vocabulary, literal-newline fidelity inside pre>code (exact, no escaping), the str-boundary TypeError, totality over malformed/fuzzed HTML, and whitelist closure (no raw tag syntax or script/style content leaks into the Markdown output). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…>MD walker Extends _MarkdownWalker with h1-h6, ul/ol/li (via an immutable _ListFrame stack that indents and increments per nesting level), <a href> (validated through the shared _sanitize_href, degrading to plain text on a rejected scheme), and <s>/<del>/<strike> (unified onto the existing format-marker force-close path). Heading close now explicitly emits a blank-line block separator, fixing the run-on omission where text immediately following a heading (no intervening <p>) landed on the same Markdown line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_parse_blocks does a line-oriented scan (fenced code, ATX headings, '-'/'N.' list items, blank-line paragraphs) and _render_block_html folds each list_item against the running list_stack's top instead of rendering it in isolation -- the spike proved per-block isolation flattens nesting and resets ordered-list numbering. markdown_to_leankit_html threads a new list_stack through the fold (never mutates the caller's), closing every open list before a non-list block and at EOF. Inline content (bold, links, code spans) is still escaped plain text pending a later task. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wires _render_inline_html + _parse_inline_span into the MD->HTML block layer so bold/italic/strikethrough/links/code spans within a block are now rendered instead of passed through as escaped plain text. Adds a balanced bracket/paren matcher for link syntax (nearest-occurrence matching mis-paired nested links), a bounded-window delimiter scanner to keep pathological unmatched-delimiter input linear rather than quadratic, and a depth cap (_MAX_INLINE_DEPTH) on nested spans. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rip fixed point
Directly-nested strong/em spans with no separating text (e.g. <strong><em>x</em>
</strong>) concatenated into an ambiguous run of literal '*' characters
("***x***") that the nearest-delimiter-match MD->HTML parser mis-split, dropping
or misplacing content on the return trip. The HTML->MD walker now emits GFM's
alternate underscore delimiter ("__"/"_") for whichever of strong/em opens
directly against another star-based marker, and the MD->HTML parser recognizes
both spellings -- sidestepping the ambiguity instead of requiring a full
CommonMark delimiter-run algorithm. Pins the round-trip fixed point for a
combined document (heading + nested list + link + code + bold/italic/strike)
and for directly-adjacent nested inline formats up to three levels deep.
Splits richtext.py (pushed to 840 lines by this fix, over the 800-line hard
cap) into _richtext_shared.py (data shapes and sanitizers both directions use),
_richtext_html_to_md.py, and _richtext_md_to_html.py, with richtext.py now a
thin re-exporting facade so `import richtext` still exposes the full public and
test-facing surface.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…udget guard Closes out the richtext.py bidirectional translator work with a systematic, parametrized pass over the remaining risk surface: every unclosed-delimiter degradation cell from the module docstring's table (not just bold), a regex-based tag-name whitelist-closure check in both directions (versus the prior ad hoc leaked-substring checks), a broader unicode/control-char totality battery, and an explicit TypeError sweep naming None/int/bytes. Also adds a hard-cap/soft-target line-count regression guard over the three richtext modules, recording _richtext_md_to_html.py's 443-line soft-target overage as a reviewed, intentional flag rather than a silent budget drift. All 57 new cases pass against the existing implementation -- no production code changes were needed, confirming the prior six tasks' TDD cycles already satisfied these invariants. Full suite: 697 passed, zero regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_render_list_item opened one <ul>/<ol> container per skipped Markdown
indent level on a >1-level indent jump (e.g. a first item that starts
already indented, or a stray deep indent mid-list) but only ever
emitted a matching <li> for the deepest level, so _close_list_frame_html
later closed every frame with a </li> regardless -- producing
unbalanced HTML. Clamp the target depth to at most one level deeper
than what's currently open so every opened container is always paired
with exactly one <li>.
_try_parse_link spliced a link's href straight from the source slice
between the balanced parens into the emitted <a href> without ever
unescaping it, even though _find_balanced_close treats a backslash
before any character (not just the syntax-ambiguous set
_unescape_markdown_text targets) as non-structural specifically so a
literal '(' or ')' can be embedded in a URL. A backslash-escaped paren
in an href therefore survived into the emitted attribute verbatim,
corrupting the link target. Add a href-scoped unescape pass that
strips any backslash-escaped character generically, matching the
balance scanner's own escape handling.
Also repairs three test-quality issues found in review: a tautological
round-trip assertion that could never fail regardless of
markdown_to_leankit_html's correctness, an assertion that pinned Python
str immutability rather than any real behavior, and a private-helper
test rewritten to exercise the public boundary so it survives
refactors. Adds an exact-value pin for mixed-type nested list
round-trips (ordered-inside-bullet and its inverse) in both
directions, previously covered only by a fixed-point self-consistency
check that a consistently-wrong renderer could still satisfy.
Splits the now 1000+ line test_richtext.py into four direction-scoped
files (shared helpers, HTML->MD, MD->HTML, combined round-trip/
adversarial-battery) so each stays under the repo's 400-line soft
target; the original file exceeded the 800-line hard cap.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… items
Two round-trip bugs found in code review:
- HTML->Markdown spliced an <a href> value into `](href)` without
escaping '(' / ')' / '\\', so a href containing an unmatched literal
paren produced Markdown that MD->HTML's balanced-paren scan for the
link's closing paren could not correctly close.
- A <br> (or any newline) inside <li> text emitted a bare '\n' with no
continuation marker; _parse_blocks didn't recognize the following
physical line as part of the same list item, so it detached into a
sibling paragraph on the next translation pass -- breaking the
round-trip fixed-point invariant and, for <ol>, resetting the
numbering of subsequent items.
Also closes a test-coverage gap: the adversarial-battery whitelist-
closure sweep checked HTML output tag names but never href attribute
values, so a regression in _sanitize_href's scheme allowlist/
comparison could leak a live javascript:/vbscript: href through the
one legitimately-whitelisted <a> tag undetected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesRichtext translation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant richtext
participant leankit_html_to_markdown
participant markdown_to_leankit_html
participant _sanitize_href
Caller->>richtext: Call conversion entry point
richtext->>leankit_html_to_markdown: Convert AgilePlace HTML
leankit_html_to_markdown->>_sanitize_href: Validate link href
_sanitize_href-->>leankit_html_to_markdown: Sanitized href or None
leankit_html_to_markdown-->>richtext: Markdown result
richtext->>markdown_to_leankit_html: Convert Markdown
markdown_to_leankit_html->>_sanitize_href: Validate link href
_sanitize_href-->>markdown_to_leankit_html: Sanitized href or None
markdown_to_leankit_html-->>richtext: AgilePlace HTML result
richtext-->>Caller: Return converted string
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
…git runs A NUL-delimited digit run longer than CPython's int-string-conversion limit (4300 digits, 3.11+) made _reinsert_code_spans' unguarded int(match.group(1)) raise ValueError, violating the module's hard never-raises-over-arbitrary-content invariant. Guard the conversion and degrade to untouched text, exactly as an out-of-range index already does. Found by the draft-phase Codex adversarial review of PR #77. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
Draft-phase Codex (GPT-5.5, xhigh) adversarial reviewRan an adversarial security review of both translation directions, plus a 100k-iteration random totality fuzz (passed) and targeted round-trip probes. Three candidate issues surfaced; dispositions below. Fixed[P2] Totality violation: oversized code-span placeholder digit run raises Declined — out of the supported-subset scope, and confirmed non-injectionBoth were verified to produce no live markup: MD→HTML HTML-escapes all content before any tag-producing substitution, so neither can inject. Both are correctness / fixed-point issues on inputs outside issue #64's documented supported vocabulary. Recommending a separate follow-up issue rather than expanding this PR's scope (per one-PR-one-change). [P2] Code-span content containing backticks does not round-trip — [P1→downgraded] HTML→MD emits unescaped Both public entry points remain total over the 100k-iteration fuzz after the fix. |
thewrz
marked this pull request as ready for review
July 22, 2026 15:08
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@_richtext_md_to_html.py`:
- Around line 389-393: The final _unescape_markdown_text pass in the
markdown-to-HTML conversion must not modify rendered href values. Update the
flow around _render_inline_run and _reinsert_code_spans to protect rendered link
fragments or finalized hrefs before unescaping, then restore them verbatim
afterward, preserving existing text unescaping and code-span behavior. Ensure
href backslashes remain unchanged when preceding any _UNESCAPABLE_CHARS
character.
In `@tests/test_richtext_shared.py`:
- Line 119: Update UNICODE_SAMPLE in tests/test_richtext_shared.py:119,
tests/test_richtext_html_to_md.py:21, and tests/test_richtext_md_to_html.py:31
to replace the raw trailing zero-width characters with explicit \u200b, \u200c,
and \u200d escapes, preserving the resulting string content while satisfying
Ruff PLE2515.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 026a4056-3154-4607-9f4f-3bcd02c71180
📒 Files selected for processing (8)
_richtext_html_to_md.py_richtext_md_to_html.py_richtext_shared.pyrichtext.pytests/test_richtext_html_to_md.pytests/test_richtext_md_to_html.pytests/test_richtext_roundtrip.pytests/test_richtext_shared.py
…scape pass The final _unescape_markdown_text pass in _render_inline_html ran over the whole rendered string, including <a href> attribute values, silently stripping an href backslash that preceded any _UNESCAPABLE_CHARS character (e.g. '\.' became '.') and breaking the HTML->MD->HTML fixed point for such links. Double each backslash in the finalized href before splicing it into the attribute -- the unescape pass's exact pre-image -- so hrefs survive verbatim while text content keeps its unescape. Pinned by direct '\.'/'\#' href tests and new round-trip cases. Also replace the raw zero-width characters in the three UNICODE_SAMPLE test literals with explicit unicode escapes (Ruff PLE2515); byte content is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Why
AgilePlace renders card descriptions and comments as an HTML subset; GitHub renders issue bodies and comments as GFM Markdown. Both the description sync (#65) and comment sync (#66) need a shared, bidirectional translator -- this issue is their common prerequisite.
What
Adds a stdlib-only
richtext.pymodule (with_richtext_md_to_html.py,_richtext_html_to_md.py,_richtext_shared.pyimplementation details) exposingmarkdown_to_leankit_html()andleankit_html_to_markdown(), covering the observed AgilePlace HTML vocabulary: paragraphs, line breaks, bold, italic, underline, strikethrough, inline code, code blocks, headings, lists, and links.html.parser; unknown/unsupported tags degrade to text rather than crashing or leaking markup.Design decisions
none -- spec was unambiguous.
Testing
tests/test_richtext_html_to_md.py,tests/test_richtext_md_to_html.py,tests/test_richtext_shared.py)tests/test_richtext_roundtrip.py)284 tests passing locally (
python3 -m pytest tests/test_richtext_*.py -q).🤖 Co-authored by Claude Sonnet 5. Closes #64
Summary by CodeRabbit