Lua M2 §6: placement + format matrix cases - #247
Conversation
The coverage the plan wanted to target via §5 is unreachable (pandoc's embedded
Lua does not honour luacov), so add the matrix cases by inspection instead:
- figure placement: wrap-outer and margin both take the OUTER wrapfigure edge
({o}), where only wrap-inner was tested ({i}); and the \Needspace guard +
column width scale with the measure (half -> 0.5\linewidth/13, third ->
0.3333/9) with a 0.4\linewidth/11 fallback for a width outside the vocabulary.
- set-pieces: cascade/verse/tail degrade to a clean native line block in
markdown/plain/docx (the non-web branch), with no html divs or latex machinery
leaking in -- the plain-text edition shows them as verse.
Behaviour unchanged; these are new assertions only. docs/LUA-QUALITY-PLAN.md §6.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NMBFSnRjob6Cmz1p4cwyCQ
📝 WalkthroughWalkthroughThe pull request adds integration tests for figure placement and width scaling, plus a Pandoc test for clean Markdown rendering of ChangesFigure placement coverage
Set-piece rendering coverage
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/test_set_pieces.py`:
- Around line 81-85: Update the set-piece test loop in tests/test_set_pieces.py
to map _CASCADE, _VERSE, and _TAIL to their expected markdown line sequences,
extract the rendered line-block content, and compare it exactly with the mapped
expectations. Preserve the existing artifact-negative assertions, including
checks for unlisted markup, while ensuring _VERSE’s empty stanza and all source
lines are validated.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5fc9dd45-91cb-4018-9dad-2be9b8e7fe7b
📒 Files selected for processing (2)
tests/test_figure_placement.pytests/test_set_pieces.py
| for md in (_CASCADE, _VERSE, _TAIL): | ||
| out = _render(md, to="markdown") | ||
| assert "| " in out # a pandoc line block | ||
| assert 'class="' not in out and "padding-left" not in out and "margin-left" not in out | ||
| assert "wrapfigure" not in out and "\\hspace" not in out and "PressVerse" not in out |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching test_set_pieces.py:\n'
fd -a 'test_set_pieces.py' . || true
file="$(fd 'test_set_pieces.py' . | head -n 1 || true)"
if [ -n "${file:-}" ]; then
printf '\nStat:\n'
wc -l "$file"
printf '\nRelevant lines:\n'
sed -n '1,140p' "$file" | nl -ba
fi
printf '\nSearch render/context identifiers:\n'
rg -n "_CASCADE|_VERSE|_TAIL|_render|test_|set_pieces" -S . || trueRepository: clintecker/press
Length of output: 328
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = next(Path('.').rglob('test_set_pieces.py'), None)
print("path", p)
if p:
txt=p.read_text()
print("contains _CASCADE:", "_CASCADE" in txt)
PYRepository: clintecker/press
Length of output: 208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="tests/test_set_pieces.py"
printf 'tests/test_set_pieces.py:\n'
awk '{printf "%5d %s\n", NR, $0}' "$file"
printf '\nSearch fixture/render definitions:\n'
rg -n "_CASCADE|_VERSE|_TAIL|_render|PressVerse|cascade|verse|tail|set_pieces|SetPiece|setpieces" -S . --glob '!*/.git/**' || trueRepository: clintecker/press
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
src = Path("tests/test_set_pieces.py").read_text()
def assert_pos(pattern, start=1):
idx = src.find(pattern, start)
print(f"{pattern!r}: {'FOUND at index ' + str(idx) if idx >= 0 else 'NOT FOUND'}")
return idx
# Exact marker checks from the relevant assertions.
assert_pos("| ")
assert_pos('class="')
assert_pos('padding-left')
assert_pos('margin-left')
assert_pos('"wrapfigure"')
assert_pos('"\\hspace"')
assert_pos('"PressVerse"')
# Programmatic extraction of assertion bodies for tests around lines 76-85.
PY
python3 - <<'PY'
import ast
from pathlib import Path
tree = ast.parse(Path("tests/test_set_pieces.py").read_text())
for node in tree.body:
if isinstance(node, ast.FunctionDef) and node.name == "test_constructs_degrade_to_a_clean_line_block_in_plaintext":
print(ast.unparse(node))
PYRepository: clintecker/press
Length of output: 793
Assert the expected markdown lines for each set-piece.
assert "| " in out only detects one line-block prefix and ignores missing source lines, _VERSE’s empty stanza, and unlisted markup. Map _CASCADE, _VERSE, and _TAIL to their expected line sequences and compare the extracted line-block content; keep the artifact-negative checks as additional guards.
🤖 Prompt for 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.
In `@tests/test_set_pieces.py` around lines 81 - 85, Update the set-piece test
loop in tests/test_set_pieces.py to map _CASCADE, _VERSE, and _TAIL to their
expected markdown line sequences, extract the rendered line-block content, and
compare it exactly with the mapped expectations. Preserve the existing
artifact-negative assertions, including checks for unlisted markup, while
ensuring _VERSE’s empty stanza and all source lines are validated.
Adds the untested filter branches by inspection (luacov can't measure filter coverage): figure wrap-outer/margin (outer {o} edge) + the \Needspace guard/width scaling with the measure and its fallback; set-pieces cascade/verse/tail degrading to a clean line block in plain-text (the non-web branch). New assertions only, behaviour unchanged. docs/LUA-QUALITY-PLAN.md §6.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NMBFSnRjob6Cmz1p4cwyCQ
Summary by CodeRabbit
cascade,verse, andtailrender as clean Markdown line blocks without unwanted formatting artifacts.