Skip to content

Fix/2109 nested lists - #394

Open
conradmugabe wants to merge 12 commits into
mainfrom
fix/2109-nested-lists
Open

Fix/2109 nested lists#394
conradmugabe wants to merge 12 commits into
mainfrom
fix/2109-nested-lists

Conversation

@conradmugabe

Copy link
Copy Markdown
Contributor

Checklist

  • Tests were added/updated according to the feature/bugfix/change made
  • Version was rolled according to semver requirements
  • API endpoints openapi schema was updated if applicable

Changes

  • Added new default system prompts
  • Added fixes from LaTeX fallback, but these might not come into play since the system prompt already exists; this is to help with old existing messages/chats

Screenshots

Screencast.from.2026-07-30.02-15-33.mp4

conradmugabe and others added 8 commits July 29, 2026 19:02
…gins

LLMs indent sublists by 2 spaces, but CommonMark requires the child to
reach the parent's content column (3+ under "1. "), so nested bullets
rendered as detached sibling lists and 2-space ordered children were
flattened into their parent with wrong numbering. Add
normalizeListIndentation, a post-preprocessLaTeX pass that re-indents
undershooting children to their parent's content column, and wire it
into the chat <Markdown> pipeline.

Nested ul/ol also inherited the top-level my-6 margin, visually
detaching them from their parent item; collapse it to my-1 via the
[ul_&]/[ol_&] variants so only nested lists tighten.

Refs #2109

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REe8vMW5v7NeDPpXEqn69Z
A round of browser-verified fixes to preprocessLaTeX, each from a real
mentor reply that rendered broken:

- Preserve \\ row separators in $$ environments (pmatrix/aligned rows
  collapsed to one) by masking math before the line-break pass.
- Keep CommonMark escapes \# and \_ intact so they render literally
  instead of becoming a heading or italics.
- Promote whole-line $$...$$/\[...\] spans to fenced display form so
  remark-math classifies them as block math; mid-sentence spans stay
  inline.
- Escape \$ inside math spans as \text{\textdollar} (remark-math honors
  no escapes inside $...$, so the inner $ terminated the span and paired
  orphans swallowed prose into math).
- Normalize \[/\(-wrapped lone styling commands to dollar form so
  \[ \textbf{...} \] headings unwrap to **...** instead of KaTeX
  rendering literal stars; mask math while the styling conversions run
  so genuine math keeps its LaTeX commands.
- Convert \begin{verbatim} to a masked code fence.
- Convert self-nested itemize/enumerate innermost-first so the outer
  begin no longer pairs with the inner end and leaks raw tokens.
- Convert aligned environments made entirely of &\verb|...| rows (code
  blocks in math costume) to fenced code blocks, preserving list-item
  indentation; mixed or genuine aligned math is untouched.
- Convert streaming tails of itemize/enumerate as items complete, and
  guard unclosed fences mid-stream.

Update the journey 61 block-math comment to reflect the display
promotion.

Refs #2109

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REe8vMW5v7NeDPpXEqn69Z
The canvas markdownToHtml pipeline corrupted fenced code: the
preprocessMarkdownForHtml fixes are whole-document regexes that injected
blank lines and merged comment lines inside fence bodies, and the
marked-highlight extension emitted per-token spans whose newline text
nodes TipTap drops, merging code lines (@tool\ndef -> @tooldef).

Mask fenced and inline code (including an unclosed streaming fence)
before the markdown fixes run and restore it verbatim after, mirroring
the chat pipeline. Drop marked-highlight/highlight.js entirely: no
consumer renders the spans, and plain code blocks survive TipTap
intact; adjust ProseMirror CSS so pre>code stops double-boxing. Wire
normalizeListIndentation into markdownToHtml so canvas nesting matches
chat.

Refs #2109

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REe8vMW5v7NeDPpXEqn69Z
LLMs abuse display math as a block container: an entire
\begin{itemize}...\end{itemize} (nested sub-lists included) wrapped in
\[...\]. The wrapper survived list conversion as $$ around a Markdown
list, the math masking then hid the block from the \textbf/\textit
unwrap, and the message rendered as one flat prose paragraph with
literal \textbf{...} and inline "-" marks.

Strip \[...\]/$$...$$ delimiters whose body is exactly an itemize or
enumerate environment, before any delimiter conversion, so the list
converts normally. The lazy body only terminates at an \end{env}
directly before the closing delimiter, so a self-nested environment
cannot end the match early; genuine display math never matches.

Refs #2109

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REe8vMW5v7NeDPpXEqn69Z
The blanket brace-expansion >=5.0.8 override (f92a8bd, added for
GHSA-mh99-v99m-4gvg) forces v5 under minimatch@9, whose CJS build calls
braceExpansion.default(...) -- v5 only exports { expand }. Every
istanbul coverage run (vitest --coverage, and with it
scripts/check-test-coverage.sh in the pre-push hook) crashed with
"brace_expansion_1.default is not a function", blocking all pushes.
Plain `pnpm test` never hits the path, which is why the override looked
green.

Scope-override test-exclude's glob to >=11.1.0: glob 11 resolves
minimatch@10, which consumes the renamed @isaacs/brace-expansion
instead, so the vulnerable package leaves the coverage chain entirely.
Coverage runs work again, the pre-push gate passes (all changed files
>= 95% lines), and `pnpm audit --audit-level=high` stays clean with no
ignore rules.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REe8vMW5v7NeDPpXEqn69Z
The default prompt told models to "Always use LaTeX formatting for
presenting your responses" -- and they complied, emitting headings as
\[\textbf{...}\], lists as \begin{itemize}, and code as
\begin{verbatim}: document-mode LaTeX that KaTeX cannot render and that
produced every #2109 breakage class seen in production.

Replace it with a structured prompt that scopes LaTeX to mathematical
expressions inside $...$/$$...$$ delimiters and plain prose for
everything else, alongside tightened response-style, retrieval-context,
greeting, and scope rules. The constants test now pins the invariants:
the math-delimiter instruction must be present and the old "LaTeX for
presenting" phrasing must never return.

Only newly created agents pick this up -- existing mentors keep their
stored prompt, so the preprocessLaTeX repair layer remains the safety
net for them and for already-stored messages.

Refs #2109

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REe8vMW5v7NeDPpXEqn69Z
The last three #2109 fix rounds (math-wrapped headings, aligned-\verb
code blocks, display-math-wrapped lists) were pinned only by
preprocessLaTeX string tests, but their failure modes live in the
remark-math/KaTeX interaction that only a rendered <Markdown> exercises.

Add render tests for each shape: \[\textbf{...}\] headings become real
<strong> (no literal stars, no KaTeX), \(\textit{...}\) becomes <em>,
aligned \verb rows become a <pre><code> block with one line per row,
\begin{verbatim} keeps a $ in its body unescaped, and the wrapped-
itemize shape renders as a real <ul> with bold/italic labels and inline
\(\alpha\) still rendering as math inside a converted list item. A
conservative guard pins that wrappers with trailing prose are not
unwrapped and genuine display math stays math.

Refs #2109

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REe8vMW5v7NeDPpXEqn69Z
@conradmugabe
conradmugabe requested a review from sonegillis1 July 29, 2026 23:39
@conradmugabe conradmugabe self-assigned this Jul 29, 2026
@conradmugabe
conradmugabe requested a review from mamigot as a code owner July 29, 2026 23:39
@conradmugabe conradmugabe added the run-tests Trigger PR validation pipeline label Jul 29, 2026
# Conflicts:
#	pnpm-lock.yaml
#	pnpm-workspace.yaml
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

PR E2E - FAILED

Full report, traces and screenshots

result FAILED
suite mentor - mode full - env stg1
tested against prod release 1.119.0 (env already on it)
failed 1
new failures 1

@bnsoni bnsoni added run-tests Trigger PR validation pipeline and removed run-tests Trigger PR validation pipeline labels Jul 30, 2026
bnsoni added a commit that referenced this pull request Jul 30, 2026
GitHub keeps only ONE pending run per concurrency group. A third dispatch
CANCELS the previously-pending one before it starts a single job — silently,
with no diagnosis on the PR.

os#390 was dropped exactly this way: #394 took the group, #390 queued, #367
arrived and evicted #390. Its check went red having tested nothing.

This was masked while stg2 shared the load across two groups; pinning
everything to stg1 made a three-deep queue routine.

Two changes, and both are needed:

  Wait for a free dispatch slot before firing. The caller polls until no
  run for this env is queued/pending, so the pending slot is never
  contested. Costs nothing new — the caller already polls for up to 2h;
  this just moves the wait earlier, and prints 'N run(s) queued ahead'
  instead of failing silently.

  Retry on eviction. Two callers can still check simultaneously and both
  dispatch. A central run that ends 'cancelled' having started ZERO jobs is
  an eviction, not a result — re-dispatch (up to 3 attempts) rather than
  reddening the PR.

Neither alone suffices: the wait has a residual race, and retry alone would
let PRs thrash.

Dispatch and wait merge into one step so the retry can span both. The gate
now reads steps.dispatch.outputs.conclusion. The liveness guard
(busy-vs-offline) and phase streaming are carried over unchanged.
@conradmugabe conradmugabe added run-tests Trigger PR validation pipeline and removed run-tests Trigger PR validation pipeline labels Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-tests Trigger PR validation pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants