Skip to content

fix(render): count multi-line <div> opening tags in dj-root boundary scan (#1749)#1750

Merged
johnrtipton merged 2 commits into
mainfrom
fix/render-full-template-multiline-div-boundary
Jun 8, 2026
Merged

fix(render): count multi-line <div> opening tags in dj-root boundary scan (#1749)#1750
johnrtipton merged 2 commits into
mainfrom
fix/render-full-template-multiline-div-boundary

Conversation

@johnrtipton

Copy link
Copy Markdown
Contributor

Closes #1749.

Problem

render_full_template finds the dj-root region's closing </div> by counting <div> depth, but the opening-tag check only matched the exact byte forms "<div " and "<div>". A multi-line opening tag<div\n class="..."> / <div\t...> — was not counted as an open. Each missed open under-counted depth, so a later </div> drove depth to 0 early: the dj-root region closed before its real end, the full rendered view was spliced in place of the truncated region, and the tail of the page rendered outside <div dj-root> (and duplicated).

Because dj-navigate swaps only the [dj-root] subtree on SPA navigation, that ejected content was never cleared → it leaked onto the next page. Reproduced on djust.org /examples/ (demo sections authored as <div\n class="demo-section">): navigating examples→home left all demos on the home page. Single-line <div content (e.g. home) was unaffected; only triggers on a fresh GET (the SPA-mount path puts content cleanly inside dj-root). Same family as #1746.

Fix

Match <div followed by any tag-boundary char (whitespace, >, /) — one predicate in render_full_template's boundary scan (python/djust/mixins/template.py).

Verification

  • Empirically on djust.org /examples/: 484 real <div opens, only 481 counted (3 <div\n missed) → demos ejected. After fix: all 17 demos inside dj-root; examples→home navigation clean (no leak, correct title/dj-view).
  • New test_render_full_template_multiline_div_boundary.py (2 cases) with gate-off proof: reverting the predicate makes the tail marker appear 2× (ejected/duplicated) → test fails; with the fix it appears once, inside dj-root. Control: single-line <div> content stays green.
  • Full Python suite: +2 passing tests; the 17 test_theme_tags_rust_engine_1721 failures in make test-python are pre-existing test-ordering pollution (identical on clean origin/main; pass in isolation; CI runs tests in separate jobs).

Follow-up (separate, not in this PR)

dj-navigate SPA nav doesn't update page chrome outside dj-root (active-nav highlight, document.title, the dj-root dj-view attribute). Tracking separately — relates to ADR-021.

🤖 Generated with Claude Code

johnrtipton and others added 2 commits June 7, 2026 19:56
…scan (#1749)

render_full_template locates the dj-root region's closing </div> by counting
<div> depth, but the opening-tag check only matched the exact forms "<div " and
"<div>". A multi-line opening tag ("<div\n  class=...>" / "<div\t...") was not
counted, so each missed open under-counted depth and a later </div> closed the
dj-root region EARLY. The full rendered view was then spliced in place of the
truncated region, leaving the tail of the page OUTSIDE <div dj-root> (and
duplicated). Because dj-navigate swaps only the [dj-root] subtree, that ejected
content was never cleared on navigation (leaked onto the next page).

Fix: match "<div" followed by any tag-boundary char (whitespace, '>', '/').

Adds test_render_full_template_multiline_div_boundary.py with gate-off proof
(fails on the old 2-form check: tail marker appears twice). Same family as #1746.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@johnrtipton

Copy link
Copy Markdown
Contributor Author

Stage-7 Adversarial Review — PR #1750 (fix #1749)

Verdict: APPROVE (with one 🟡 latent-mirror-bug finding tracked as a follow-up, not a merge blocker)

The fix is correct, minimal, and the regression test is genuinely behavior-coupled (proven by an independent gate-off). The one substantive finding is a latent close-side symmetry gap that this PR does not introduce and does not worsen — it is the symmetric twin of the bug being fixed and exists identically in a sibling counter. Flagging per the #1646 parallel-path-drift canon, but it is lower-probability than the open-side bug and out of scope for this fix.


Mandatory checks

  • 🟢 Stale-base (Full HTML update path mangles page rendering #250): BEHIND=0. git log origin/main..HEAD shows exactly the 2 PR commits (c5e73afa fix+test, 8f00f452 CHANGELOG). Reviewing the program that will merge.
  • 🟢 Two-commit shape: c5e73afa = template.py + new test only, NO CHANGELOG. 8f00f452 = CHANGELOG only. Correct boundary.
  • 🟢 Three-dot diff: CHANGELOG.md (+4), template.py (+15/-1), new test (+191). Scoped.

Predicate correctness (empirically replicated, not eyeballed)

🟢 Standalone replication of the new open-check over crafted strings confirmed:

  • COUNTS all real opens: <div>, <div (space), <div\n, <div\t, <div\r, <div/, and bare <div at EOF.
  • Does NOT count </div> as an open (shell_html[i:i+4] at the < of </div> is </di<div).
  • Does NOT count <divider / <div-foo / <division (char after <div is i/-, not in the boundary set " \t\r\n>/").

🟡 Close-side mirror bug — LATENT (python/djust/mixins/template.py:997)

The OPEN check now tolerates whitespace; the CLOSE check is still hardcoded shell_html[i:i+6] == "</div>". I traced the Rust renderer to determine whether a whitespace-bearing close tag (</div >, </div\n>) can ever reach shell_html:

  • The Rust template engine (crates/djust_templates/src/renderer.rs) is a Django-style text-passthrough engine, not an HTML DOM serializer. Node::Text(text) => Ok(text.clone()) (renderer.rs:387) emits authored HTML — including close tags — verbatim. The only place a </div> is synthesized is renderer.rs:759 (the ReactComponent mount-point node), which emits the canonical no-whitespace form.
  • Empirically confirmed by rendering '<div dj-root>\n <div\n class="inner">hi</div >\n</div>TAIL' through the real RustLiveView(src).render(): the output preserves </div > byte-for-byte. Newline-before-> (</div\n>) is likewise passed through verbatim.
  • Failure mode (simulated against the real boundary loop): a template whose inner div uses </div > and whose dj-root closes with canonical </div> drives the loop to FALLBACK_DEPTH_NEVER_ZERO — the inner <div open counts (depth→2) but its </div > close is not matched (depth stays 2), so the real outer </div> only reaches depth 1 and the loop exhausts → returns the un-normalized shell (the Initial SSR render skips the comment/whitespace normalization that render_with_diff applies → first-hydration morphChildren re-render (flash) #1737 first-hydration-flash class).

Determination: NOT safe — a latent mirror bug exists. It is the symmetric twin of #1749. It is lower-probability (close tags rarely carry whitespace before >, and the synthesized React close tag is always canonical), so it does not block this PR, but per #1646 it should be filed as a follow-up. The cleanest structural cure is below.

🟡 Parallel-path-drift (#1646)

grep confirms template.py:993 is the only hand-rolled character-scan div-depth counter. BUT there is a second, regex-based div-boundary scanner: TemplateMixin._find_closing_div_pos (template.py:775), used by 5 call sites (:451, the three _extract_liveview* helpers, and :855). It:

  • uses re.search(r"<div\b", ...) for opens — \b already tolerates multi-line opens, so it never had the open-side bug;
  • uses re.search(r"</div>", ...) for closes — same hardcoded close form → same latent whitespace-close gap as the fixed counter.

So the close-side gap is consistent across both scanners (both latently broken on </div >), and the open-side handling is now consistent too. Recommendation for the follow-up: rather than maintaining two counters, have render_full_template delegate to _find_closing_div_pos (it already handles balanced nesting and {% if/else %} branch restoration, which the hand-rolled loop does not). The hand-rolled duplicate at :993 is the kind of "N correct copies" #1646 warns against. Not in scope for this fix; noting for the class.

Gate-off (independently run) — test is NOT tautological 🟢

Reverted ONLY the predicate to the old 2-form check, re-ran:

Restored the fix; working tree clean. The test exercises the real render_full_template path end-to-end (Rust render → boundary scan → HTMLParser containment assertion), not a synthetic loop copy — high reproduction fidelity.

Pollution claim 🟢

test_theme_tags_rust_engine_1721.py passes in isolation (17 passed). The 17 full-suite failures are an ordering/isolation artifact, pre-existing and unrelated. This PR edits only the dj-root boundary loop in render_full_template; it cannot causally affect theme tag rendering.

No-regression 🟢

test_render_full_template_djroot_substring_1746.py + test_ssr_render_normalization_1737.py + tests/unit/test_template_inheritance.py: 31 passed.


Summary

Correct, well-tested, well-scoped fix. APPROVE. Recommend a follow-up issue for the close-side whitespace-tolerance gap (present in both template.py:997 and _find_closing_div_pos:804), ideally resolved by collapsing the two div-boundary scanners into _find_closing_div_pos.

🤖 Generated with Claude Code

@johnrtipton johnrtipton merged commit f6745f8 into main Jun 8, 2026
13 checks passed
@johnrtipton johnrtipton deleted the fix/render-full-template-multiline-div-boundary branch June 8, 2026 00:05
@johnrtipton

Copy link
Copy Markdown
Contributor Author

Retrospective — PR #1750 (#1749)

Task: fix render_full_template ejecting page content outside [dj-root] for multi-line <div> tags (leaked across dj-navigate).

Quality: 5/5

Tight one-predicate fix, root-caused empirically against a live downstream symptom, gate-off-proven test, systemic remainder tracked.

What went well

What didn't

  • Long investigation before the fix. Multiple wrong turns (nested <main>, service-worker cache, the client mount handler — a passing JS test that was later removed) because the browser MCP couldn't reach the affected tab and the byte-level <div> depth count was fooled by <script>/code-sample text. Resolved once an HTML-parser-based (reliable) containment check and the live <div-open census were used.
  • Pre-existing test pollution surfaced. make test-python fails 17 test_theme_tags_rust_engine_1721 cases by ordering (pass in isolation; identical on clean main; CI unaffected). Not this PR's bug — worth a separate pollution-isolation issue.

Verified

Follow-ups

RETRO_COMPLETE

johnrtipton added a commit that referenced this pull request Jun 8, 2026
…dening)

Drains #1751 (close-side </div> + scanner consolidation) and #1752 items 2-3
into v1.0.3; records #1747/#1750/#1748/#1753 already merged toward it.

Audit-bypass-reason: docs-only ROADMAP update via pipeline-drain skill (no retro needed)
johnrtipton added a commit that referenced this pull request Jun 8, 2026
…rs (#1751) (#1754)

* fix(render): close-side </div> whitespace tolerance + consolidate dj-root scanners (#1751)

Completes the #1749/#1750 dj-root-boundary fix class.

- `_find_closing_div_pos` (the shared scanner, 6 call sites) hardcoded the
  close tag as `</div>`, missing `</div >` / `</div\n>`. That is the close-side
  twin of #1749's open-side under-count: a whitespace close over-counts depth so
  the close is never found. Now matches `</div\s*>` (`close_match.end()` consumes
  the full tag, so splice points stay correct).
- Replaced `render_full_template`'s separate hand-rolled depth loop (its open
  side was patched in #1750) with a call to `_find_closing_div_pos`. The helper
  is multi-line-safe on the open side (`<div\b`) and now whitespace-tolerant on
  the close side; the rendered shell has no `{% %}` tags so the helper's
  if/else handling is inert. Removing the duplicate scanner closes the
  parallel-path-drift (#1646) that let the open-side bug exist in one copy only.

Adds close-side gate-off test (reverting `</div\s*>`→`</div>` makes the
whitespace test fail); existing multi-line + render-path suites stay green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(changelog): record #1751 close-side </div> tolerance + scanner consolidation

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: render_full_template ejects page content outside dj-root for multi-line <div> tags (leaks across dj-navigate)

1 participant