Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c26567f
docs: add TUI renderer contract-hardening design
elkaix May 30, 2026
c839e66
docs: add TUI markdown+report contract-hardening implementation plan
elkaix May 30, 2026
d175f7a
test(tui): add shared markdown/report contract helpers
elkaix May 30, 2026
f4c6d7b
fix(tui): protect code-span pipes in markdown tables so LLM output re…
elkaix May 30, 2026
e75c5da
test(tui): guard table empty-header + long-cell data integrity
elkaix May 30, 2026
c94a175
test(tui): guard code-block border against inline-code color bleed
elkaix May 30, 2026
0a9d970
test(tui): guard streamed tables against mid-row commit
elkaix May 30, 2026
b962bde
test(tui): add stream offset-divergence and idempotency guards
elkaix May 30, 2026
906134b
docs(tui): record H2 non-reproduction result in design spec
elkaix May 30, 2026
7d6e8c2
test(tui): pin regex markdown table-repair behavior
elkaix May 30, 2026
9f01d00
test(tui): render reports from the real security-scan fixture
elkaix May 30, 2026
b91327e
fix(tui): extract report fences via AST so nested blocks aren't promoted
elkaix May 30, 2026
d76abcc
test(tui): forbid direct terminal writes in markdown/report renderers
elkaix May 30, 2026
cc391af
docs(tui): add markdown/report contract test registry
elkaix May 30, 2026
9e50132
fix(shell): restore blocking pre-start update prompt unwired in #63
elkaix May 30, 2026
ffbff64
fix: align todos and preserve agent language
elkaix May 30, 2026
deb0d81
fix(agent): harden reports and edit handling
elkaix May 30, 2026
70bf376
test(agent): align subagent prompt persistence
elkaix May 30, 2026
b8eec2c
test(tui): tighten markdown review guards
elkaix May 30, 2026
3b05a56
ci(coderabbit): pin Python naming to snake_case (PEP 8)
elkaix May 30, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ reviews:
- "!**/__pycache__/**"

path_instructions:
- path: "**/*.py"
instructions: |
Python uses snake_case for variables, functions, and methods per PEP 8; this is the
established convention across the entire codebase. Do NOT suggest renaming Python
identifiers to camelCase — such suggestions contradict the repo standard and are out of
scope. The camelCase convention applies only to TypeScript/JavaScript sources, never to
.py files.
- path: "src/pythinker_code/telemetry/**"
instructions: |
Flag any changes that could inadvertently capture PII, secrets, or sensitive user data.
Expand Down
136 changes: 136 additions & 0 deletions .pythinker/reports/validation-tui-renderer-contract-hardening.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# Validation Report: TUI Renderer Contract Hardening Review

## Verdict

The pasted review is **partially validated**. It correctly identifies a real edge-case mismatch in `_CODE_SPAN_RE` and a weak test assertion, but it overstates severity and contains at least one unvalidated/likely incorrect GFM/table-escape claim. I would not treat the pasted report's "2 high-priority issues" as release blockers.

## Scope and evidence

Validated against branch `feat/tui-renderer-contract-hardening` using targeted reads and commands.

Commands run:

```bash
git rev-list --count main..HEAD
git diff --name-only main...HEAD | wc -l
git diff --shortstat main...HEAD
uv run pytest tests/ui_and_conv/test_md_table_contract.py tests/ui_and_conv/test_md_stream_idempotency.py -q
uv run pytest tests/ui_and_conv -q
uv run python - <<'PY'
from markdown_it import MarkdownIt
from pythinker_code.ui.shell.components.markdown import _escape_code_span_pipes

md = MarkdownIt('commonmark').enable('table')
for label, row in [
('mismatched closing run', '| `a | b`` | rest |'),
('double slash before pipe', '| `a \\\\| b` | rest |'),
]:
src = '| Expr | Meaning |\n| --- | --- |\n' + row + '\n'
cells = []
for token in md.parse(src):
if token.type == 'inline':
cells.append((token.content, [(c.type, c.content) for c in (token.children or [])]))
print(label)
print('escaped-row:', repr(_escape_code_span_pipes(row)))
print('markdown-it inline cells:', cells)
PY
```

Results:

- Branch metadata from `main...HEAD`: **16 commits**, **24 files changed**, `1966 insertions(+), 51 deletions(-)`. This does **not** match the pasted report's "15 commits / 16 files" claim.
- Targeted tests: `15 passed`.
- Full `tests/ui_and_conv`: `1305 passed`.

## Finding validation

### 1. `_CODE_SPAN_RE` mismatched backtick behavior

Status: **Validated as an edge case, severity overstated.**

Evidence:

- `src/pythinker_code/ui/shell/components/markdown.py` defines `_CODE_SPAN_RE = re.compile(r"(?P<ticks>`+)(?P<body>.*?)(?P=ticks)")`.
- For `| `a | b`` | rest |`, `_escape_code_span_pipes` returns `| `a \| b`` | rest |`.
- `markdown-it-py` without the pre-escape parses the row cells as text fragments `('`a')` and `('b``')`, not a balanced code span.

Interpretation:

The observation is technically real: the regex can treat the first backtick of a longer closing run as the equal-length closer. However, this input is already malformed, and the normalizer is explicitly a repair/tolerance path for LLM-produced table rows. The product decision is whether to enforce strict GFM code-span boundaries or keep forgiving repair behavior.

Recommended action:

- Add a characterization test for mismatched backtick runs.
- Decide and document the intended policy:
- strict GFM: do not escape pipes in mismatched runs; or
- tolerant LLM repair: keep current behavior and test it as intentional.

Suggested severity: **Low/Medium**, not High.

### 2. Double-backslash before pipe claim

Status: **Not validated; likely incorrect for the current parser contract.**

Evidence:

For row `| `a \\| b` | rest |`:

- `_escape_code_span_pipes` returns it unchanged.
- `markdown-it-py` parses a valid table row with cells:
- `code_inline` content: `a \| b`
- second cell: `rest`
- The table structure remains intact.

Interpretation:

The pasted report assumes even/odd backslash semantics that do not match the observed `markdown-it-py` table behavior used by this code path. The current implementation's simple negative lookbehind preserves table structure for this case. The proposed regex change could alter literal backslash rendering and should not be applied without a concrete failing renderer test.

Recommended action:

- Do **not** treat this as a confirmed bug.
- If this edge matters, first add a renderer-level characterization test for the desired source-to-rendered output.

Suggested severity: **None / advisory only**.

### 3. `test_table_with_piped_inline_code_keeps_columns` assertion precision

Status: **Validated, but low severity.**

Evidence:

The test in `tests/ui_and_conv/test_md_table_contract.py` checks only that `bitwise or`, `plain`, and `text` appear in rendered output. Those checks are useful but do not strongly prove table structure survived.

Recommended action:

Strengthen with a structural assertion that the header/data relationship survives, or with a lower-level normalized-markdown/token assertion. Keep it simple; avoid brittle visual-layout assertions.

Suggested severity: **Low**.

### 4. Parameterizing test strings

Status: **Valid nit, not a defect.**

This is maintainability advice only. Current explicit tests are readable and acceptable.

Suggested severity: **Nit / optional**.

### 5. Stream intermediate-state assertions

Status: **Not validated as useful.**

The existing stream test already validates exact reassembly and no duplicated rendered rows. The suggested `1 <= len(committed) <= 10` check is arbitrary and may become brittle if commit-boundary heuristics change without user-visible regression.

Recommended action:

Do not add the suggested slice-count assertion. If stronger coverage is needed, assert a named invariant tied to user-visible behavior, not an arbitrary count.

## Recommended next actions

1. Correct the review metadata: current branch evidence is 16 commits / 24 files from `main...HEAD`.
2. Add one characterization test for mismatched backtick runs in `_escape_code_span_pipes`.
3. Optionally strengthen `test_table_with_piped_inline_code_keeps_columns` with a non-brittle structural assertion.
4. Do not implement the pasted report's double-backslash regex recommendation unless a failing renderer-level test proves the desired behavior.

## Notes

The graphify knowledge graph may be stale because files changed in this session; validation above used targeted raw-file reads and deterministic commands rather than relying on the graph for modified areas.
Loading
Loading