dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

test: preserve ambiguous markdown fence boundaries (#3622)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-09-12 16:29:59 +01:00
committed by GitHub
parent b6571a9854
commit 61e86b40e7
4 changed files with 682 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/bare-markdown-wrapper/analysis.md
+172
View File
@@ -0,0 +1,172 @@
# Bare-wrapper ambiguity analysis
## Finding
No content-only heuristic can guarantee both requested outcomes for every valid
Markdown message. A bare code block can contain headings and tagged-looking fence
lines as literal text. Those characters do not encode the author's wrapper intent.
The work remains blocked. A thematic-break boundary does not establish wrapper
intent: the same bounded range can contain two complete independent blocks. No
safe content-only discriminator is available for this task.
## Source evidence
- `apps/web/lib/markdown/normalize-cache.ts`:
`strengthenMarkdownWrapperFence` rejects a bare first opener through
`MARKDOWN_WRAPPER_OPEN_RE`.
- The same helper uses `firstNonBlankLineIndex` and `lastNonBlankLineIndex`.
Introductory and trailing prose also prevent detection.
- `markdownWrapperInnerFenceInfo` is a heuristic scan, not evidence of author intent.
`FENCE_OPENER_LINE_RE` can match text inside an already open code block.
- A boundary search without an enclosing-fence parser also sees rules and fence
candidates inside a valid five-backtick code block.
- `apps/web/components/shared/memoized-markdown.tsx` passes
`normalizeCached(content)` directly to `ReactMarkdown`.
- Existing tests preserve Markdown samples followed by separate blocks and
preserve tagged wrappers with trailing prose outside the wrapper.
## Compact reproduction
Construct the raw string from the following lines. `BT3` means exactly three
backticks, and `BT3go` means those backticks immediately followed by `go`.
`EMPTY` means an empty line. These labels are not part of the input.
```text
Copy everything between the lines.
EMPTY
---
EMPTY
BT3
Open a pull request.
## Why
BT3go
func first() {}
BT3
More prose.
BT3go
func second() {}
BT3
Final prose.
BT3
EMPTY
---
EMPTY
That is self-contained.
```
Desired interpretation: one raw Markdown code block between the rules.
Observed interpretation: three code blocks. The first ends after `func first()`.
The second contains `func second()`. The third consumes the final rule and outro.
The pure transform leaves this input unchanged. This establishes both the
reported symptom and the missing interior-range behavior.
## Counterexample A: two complete legitimate blocks
```text
BT3
## Fence syntax
BT3go
func first() {}
BT3
This paragraph separates two examples.
BT3
func second() {}
BT3
```
The first block shows a Markdown syntax fragment, including a heading and a
literal tagged opener. It ends at the first pure fence after `func first()`.
The paragraph is normal text. The second bare block contains `func second()`.
Both blocks have explicit closing fences.
This input satisfies all three suggested signals:
1. Its first and last nonblank lines are matching bare fences.
2. A same-length tagged opener and pure closer occur strictly between them.
3. It contains an ATX heading and prose between code samples.
Strengthening the outermost fences merges two intended code blocks and their
separating paragraph. Adding the reproduction's introduction and rules does not
remove this ambiguity from an interior-range detector.
## Counterexample A2: rule-bounded independent blocks
```text
---
BT3
## Fence syntax
BT3go
func first() {}
BT3
This paragraph separates two examples.
BT3
func second() {}
BT3
---
```
This input has the proposed thematic-break boundaries, but it is also two
complete bare code blocks separated by a paragraph. The first code block contains
the literal heading and tagged opener. Strengthening the first and last fences
would merge both blocks and the paragraph, so the boundary is not a safe repair
signal.
A valid five-backtick text block can contain the complete compact reproduction as
literal content. A raw-line scan would find its rules and inner fences and rewrite
bytes that Markdown already treats as code text.
## Counterexample B: requiring two tagged pairs
```text
BT3
## Fence syntax
BT3go
func first() {}
BT3
More prose.
BT3go
func second() {}
BT3
Final prose.
BT3
```
Under GFM, this is a bare syntax sample, a separate Go block, and an empty final
code block. A fence without an explicit closer extends to the end of the input.
Such input is valid Markdown, including during streaming.
The alternative intended interpretation is one wrapper containing two tagged
examples. Both interpretations use exactly the same bytes. Requiring two tagged
pairs or prose between them cannot distinguish those intentions.
Headings in these examples are meaningful literal documentation. They are not
evidence that sibling blocks are nonsensical. Natural-language introduction text
and horizontal rules also occur in legitimate example documents.
## Read-only verification
Installed `normalizeMarkdown`, React, and `react-markdown` produced the following
results on 2026-09-11:
| Input | Normalizer changes bytes | Rendered code blocks |
| --- | --- | --- |
| Compact reproduction | No | 3 |
| Counterexample A | No | 2 |
| Counterexample B | No | 3 |
| Two independent tagged Go blocks | No | 2 |
For counterexample A, the emitted HTML has a paragraph between two `pre`
elements. The first code text contains the literal heading and tagged opener.
Counterexample B and the independent Go case also ran with all production remark
plugins: GFM, breaks, and gemoji.
## Disposition
The proposed boundary contract is rejected. Counterexample A2 proves that it
violates the preservation guarantee, and the five-backtick case proves that a
boundary scan can rewrite literal code content. The normalizer therefore keeps
the existing tagged-wrapper behavior and leaves bare wrapper candidates unchanged.
The work order remains blocked until a producer marker, parser-level intent
signal, or other product decision supplies information that raw Markdown lacks.
docs/plans/bare-markdown-wrapper/plan.md
+172
View File
@@ -0,0 +1,172 @@
---
created: 2026-09-11
status: blocked
requirements:
- REQ-UI-COMMENT-MARKDOWN-001
system_design:
- ../../specs/ui/system-design/comment-markdown.md
legacy_specs: []
---
# Implementation Plan: Bare Markdown Wrappers
## Overview
This package records the requested repair, the counterexamples that make raw
Markdown wrapper intent ambiguous, and the resulting blocked implementation.
The existing normalizer remains unchanged for bare wrapper candidates. A future
implementation needs a producer marker, parser-level intent signal, or another
accepted product contract before it can safely repair the reported document.
The [analysis](analysis.md) contains concrete counterexamples, renderer evidence,
and the written-analysis fallback.
## Requirement and design reconciliation
UI owns reusable Markdown presentation independently of task or agent state.
The existing [requirements](../../specs/ui/requirements/comment-markdown.md)
define GFM rendering under `AC-UI-COMMENT-MARKDOWN-001.1` and `.2`.
The existing [design](../../specs/ui/system-design/comment-markdown.md) names
`MemoizedMarkdown` as the shared renderer.
The existing requirements and design define tagged whole-message wrapper repair,
but they do not define a safe raw-Markdown signal for a bare wrapper. The proposed
thematic-break contract was rejected because it merges valid independent blocks
and can rewrite literal content inside an enclosing code block. No new acceptance
criterion or system-design behavior is recorded until that product decision is
resolved.
## Scope
### In scope
- Record the ambiguity and renderer evidence in the analysis.
- Add regression coverage for the exact preservation cases.
- Keep the existing normalizer behavior until a safe contract is accepted.
### Out of scope
- Any bare-wrapper repair without an explicit intent signal.
- Automatic changes to message producers, renderer APIs, or stored messages.
- New parser dependencies, cache keys, cache limits, or telemetry.
- Changes to tagged-wrapper eligibility or existing trailing-prose behavior.
- Commits, pushes, PRs, and delegated implementation.
## Technical approach
`normalizeMarkdown` calls `strengthenMarkdownWrapperFence` before its glued-close
repair. Tagged wrappers keep their existing whole-message eligibility. Bare
wrapper candidates remain unchanged because the raw input does not identify
whether matching fences are a wrapper or separate Markdown blocks.
The supplied reproduction has a bare opener and surrounding prose. The normalizer
must preserve its current Markdown interpretation until a product-level signal
resolves the ambiguity.
Keep any future repair inside `apps/web/lib/markdown/normalize-cache.ts`. Reuse the
existing tagged-wrapper path and do not weaken separate-block preservation. If a
future contract is accepted, it must preserve indentation, surrounding prose,
interior text, and newline behavior.
`normalizeCached` remains a raw-input-string LRU with a hard cap of 500 entries.
No task, session, message, or viewport information enters its key.
## ASCII UI preview
UI-01: Existing chat message, desktop and phone. The requested grouping remains
blocked because the raw input has two valid interpretations.
The count and content of code blocks are structural requirements, not pixel sizes.
```text
Current interpretation Requested but unresolved
Intro and rule Intro and rule
[code: first portion] [one code block:
Middle prose heading + literal fences
[code: second Go example] both Go examples + prose]
Final prose Rule and outro
[code: rule and outro]
```
The same ambiguity applies on desktop and phone. This utility introduces no
controls, navigation, touch behavior, scroll ownership, or viewport branches. The
mobile-parity normalization exception permits targeted renderer coverage in the
existing unit test file. No new Playwright file is planned.
## Tests
All permanent cases belong in `apps/web/lib/markdown/normalize-cache.test.ts`.
The tests preserve the ambiguous inputs and prove that valid code content remains
literal.
| Requested behavior | Evidence |
| --- | --- |
| Bare wrapper candidate with two tagged Go blocks and surrounding prose | Byte-identical normalization and rendered Markdown boundaries remain unchanged |
| Two independent tagged Go blocks | Byte-identical normalization and exactly two rendered code blocks with prose between |
| Existing tagged Markdown wrappers | Existing suite passes, including sample boundaries and trailing-prose exclusions |
| Glued closing fences | Standalone `func f() {}` plus glued fence, nested glued closes, and glued wrapper closes retain existing results |
| Idempotence | Normalize the two-region bare fixture twice and compare with the unchanged first result |
| Raw-key bounded LRU | Existing hit, miss, overflow, and recency tests pass; assert cap equals 500 and distinct raw inputs remain distinct cache entries |
| Ambiguous syntax samples | Rule-bounded independent blocks, five-backtick literal content, and variants remain byte-identical |
Also cover CRLF, leading/trailing blank lines, indentation from zero to three
spaces, longer runs, missing closes, and bare blocks without nested tags.
Reuse fixture constants to respect duplicate-string limits.
### Rendered verification
Use React `createElement`, `react-dom/server`, and `react-markdown` with
`remark-gfm`, `remark-breaks`, and `remark-gemoji` inside the existing `.test.ts`.
Assert code text and surrounding elements, not only fence counts.
This is a parser/render integration check, not a claim of full browser E2E coverage.
It exercises the viewport-independent transformation used by desktop and phone.
## Work orders
- [ ] [Task 01: Resolve bare-wrapper eligibility](task-01-resolve-wrapper-eligibility.md) (`blocked`)
There is one sequential work order.
## Verification results
Read-only renderer probes on 2026-09-11 found:
- Compact reproduction: normalization unchanged, three code blocks.
- Counterexample A: normalization unchanged, two separate code blocks.
- Two independent tagged Go blocks: normalization unchanged, two code blocks.
- Two nested-looking pairs: normalization unchanged, three code blocks.
These probes used installed React and react-markdown packages. The latter two
also used all three production remark plugins. Permanent regression tests cover
the preserved interpretations in the existing normalizer test file.
Documentation validation on 2026-09-11:
- `rtk proxy python3 scripts/lint-spec-files.test.py`: 36 tests passed.
- `rtk proxy python3 scripts/lint-spec-files.py --all`: all specification files passed.
- `rtk git diff --check -- docs/plans/bare-markdown-wrapper`: passed.
- `rtk git status --short -- docs/plans/bare-markdown-wrapper`: new package present, untracked.
Verification after restoring the safe boundary on 2026-09-12:
- `pnpm --filter @kandev/web test -- --run lib/markdown/normalize-cache.test.ts`: 52 tests passed.
- `pnpm --filter @kandev/web lint`: passed with zero warnings.
- `pnpm --filter @kandev/web typecheck`: passed.
- `pnpm --filter @kandev/web build:vite`: passed. Vite emitted existing chunk-size and ineffective dynamic-import warnings.
- `python3 scripts/lint-spec-files.test.py`: 36 tests passed.
- `python3 scripts/lint-spec-files.py --all`: all specification files passed.
- `git diff --check`: passed.
The full `pnpm --filter @kandev/web test` suite was started with the configured
worker budget but produced no terminal report after 13 minutes, so it was
stopped. The focused suite completed independently with a captured exit code of
zero.
The package remains uncommitted and blocked pending a product-level intent signal.
## Risks and unresolved decision
The thematic-break contract cannot be accepted as a compatibility boundary. A
message with the same bytes can represent one wrapper, two independent blocks, or
literal content inside a larger fence. Correct longer outer fences or an explicit
producer marker remain possible future solutions, but producer changes remain
outside this task.
docs/plans/bare-markdown-wrapper/task-01-resolve-wrapper-eligibility.md
+121
View File
@@ -0,0 +1,121 @@
---
id: "01-resolve-wrapper-eligibility"
title: "Resolve bare-wrapper eligibility"
status: blocked
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-UI-COMMENT-MARKDOWN-001
acceptance_criteria:
- AC-UI-COMMENT-MARKDOWN-001.1
- AC-UI-COMMENT-MARKDOWN-001.2
system_design:
- ../../specs/ui/system-design/comment-markdown.md
---
# Task 01: Resolve bare-wrapper eligibility
## Summary
Resolve whether a bare wrapper can be repaired safely. The current analysis shows
that raw Markdown does not carry enough intent information, so the work order
records the preservation fallback and remains blocked.
## In scope
- Use the [analysis](analysis.md) to preserve the ambiguous cases.
- Add the exact rule-bounded, enclosing-fence, and multi-region regressions.
- Keep the existing normalizer behavior until a safe contract is accepted.
## Out of scope
- Adding a heuristic that narrows the separate-block preservation guarantee.
- Defining a new requirement or system-design contract without a product decision.
- Changing cache semantics, Markdown plugins, or renderer components.
- New permanent test files or dependency changes.
## Acceptance
1. The analysis records why the reported input is ambiguous and names the missing
intent signal.
2. The normalizer leaves the exact negative cases byte-identical, including valid
code content enclosed by a five-backtick fence and two disjoint regions.
3. The plan and work order remain blocked until a product-level contract makes a
safe repair possible.
## ASCII UI preview
UI-01 uses the [shared desktop/phone preview](plan.md#ascii-ui-preview).
```text
Requested: intro -> one raw document code block -> outro
Current safe behavior: first code block -> prose -> second code block
```
Content grouping supports `AC-UI-COMMENT-MARKDOWN-001.1` and `.2`.
No layout or mobile interaction changes are proposed.
## Verification
Run documentation commands from the repository root after revising the package:
```bash
rtk proxy python3 scripts/lint-spec-files.test.py
rtk proxy python3 scripts/lint-spec-files.py --all
rtk git diff --check -- docs/specs docs/plans/bare-markdown-wrapper
rtk git status --short -- docs/plans/bare-markdown-wrapper
```
Use TDD in the existing test file. First demonstrate each unsafe repair candidate
fails its preservation regression. Then run this complete block from the repository
root:
```bash
(cd apps && rtk pnpm install --frozen-lockfile)
(cd apps && rtk pnpm --filter @kandev/web test -- normalize-cache)
(cd apps && rtk pnpm --filter @kandev/web lint)
rtk git diff --check
```
Installation is required once in a fresh worktree. Do not override the configured
Vitest worker budget or its `NODE_ENV=test` setting. Changed-line warnings fail
acceptance. Preserve the supplied frontend complexity and size limits.
## Files in this package
- `docs/plans/bare-markdown-wrapper/analysis.md`
- `docs/plans/bare-markdown-wrapper/plan.md`
- `docs/plans/bare-markdown-wrapper/task-01-resolve-wrapper-eligibility.md`
- `apps/web/lib/markdown/normalize-cache.test.ts`
## Dependencies
An accepted producer marker, parser-level intent signal, or product decision is
required. No dependency work order exists.
## Risks
The existing inner scan cannot establish author intent even inside the proposed
boundary. Avoid changing existing tagged-wrapper behavior, and preserve separate
bare blocks plus literal content inside enclosing fences.
## Parallelism
`sequential`
## Inputs
- [Requirements](../../specs/ui/requirements/comment-markdown.md)
- [System design](../../specs/ui/system-design/comment-markdown.md)
- `apps/web/lib/markdown/normalize-cache.ts`
- [Counterexamples and source evidence](analysis.md)
- Existing normalizer helpers and tests, especially wrapper unchanged boundaries.
- The user's explicit implementation request and the written-analysis fallback.
## Results
The rule-delimited repair was rejected after the exact preservation counterexample
and enclosing-fence case failed. The production normalizer remains unchanged for
bare candidates. Preservation regressions and final validation results are recorded
in the plan; the work order is blocked pending a safe intent signal.