Skip to content

fix(docs): preserve source text through syntax highlighting - #971

Closed
rudycelekli wants to merge 3 commits into
openclaw:mainfrom
rudycelekli:fix/peekaboo-docs-code-preservation-20261005
Closed

rudycelekli wants to merge 3 commits into
openclaw:mainfrom
rudycelekli:fix/peekaboo-docs-code-preservation-20261005

Conversation

@rudycelekli

Copy link
Copy Markdown
Contributor

Summary

The syntax highlighter used U+E000–U+F8FF as hidden replacement tokens. Actual private-use characters in code were silently deleted or replaced, and blocks containing more than 6,400 highlighted matches exceeded that token range. The builder still reported success while changing the text users copy.

Keep highlighted fragments separate from raw source text, applying later patterns only to unhighlighted fragments. This removes token collisions and the fixed token limit while retaining the existing span classes and escaping. Include the new producer regressions in test:docs-site/test:safe.

Observed production behavior

Actual docs builder entrypoint with owned Markdown fixtures:

Shell source: echo <U+E000>path<U+F8FF>
before rendered code: echo path
after rendered code: exact original source

7,000-string JSON fixture:
before builder exit: 0; copyable JSON source preserved: false
after builder exit: 0; copyable JSON source preserved: true

Shipped docs build: exit 0; 70 HTML pages emitted

Validation

  • node --test tests/docs-site-code-preservation.test.mjs tests/docs-site-toc.test.mjs — five tests passed.
  • Controls retain highlighting and exact source text for quoted strings, comments, flags, numbers, shell paths, YAML keys, Swift, JavaScript and JSON.
  • node scripts/build-docs-site.mjs — passed.
  • git diff --check — passed.
  • Full Swift/UI automation is not claimed for this docs-only change; macOS CI remains the broader gate.

AI assistance

Prepared with Codex assistance; owned production fixtures and focused regressions verify the change.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@rudycelekli
rudycelekli requested a review from a team as a code owner October 5, 2026 14:47
@clawsweeper

clawsweeper Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@cursor

cursor Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Docs-site rendering and test wiring only; no runtime, auth, or data-path changes.

Overview
Fixes the docs site builder so copyable code blocks match the original source after syntax highlighting.

The highlighter no longer substitutes matches with private-use Unicode placeholders (which could strip real characters in paths/YAML and break past ~6,400 tokens). It now tracks highlighted fragments separately from plain text in build-docs-site.mjs, applying each pattern only to unhighlighted segments while keeping the same hl-* span classes.

Adds tests/docs-site-code-preservation.test.mjs (private-use chars, large JSON blocks, multi-pass shell/JS/Swift/JSON) and broadens test:docs-site to tests/docs-site-*.test.mjs so the regressions run in test:safe.

Reviewed by Cursor Bugbot for commit f5e962d. Bugbot is set up for automated code reviews on this repo. Configure here.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 5, 2026
@clawsweeper

clawsweeper Bot commented Oct 5, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed October 5, 2026, 10:51 AM ET / 14:51 UTC.

ClawSweeper review

What this changes

The PR replaces hidden syntax-highlighting placeholders with separate text and HTML fragments, adds source-preservation regressions, and includes them in the docs test command.

Merge readiness

✅ Ready for maintainer review

The fix remains necessary: current main and v4.8.0 retain the reported text-corruption mechanism. The introduced patch is focused, has relevant production-builder proof, and has no actionable correctness findings.

Priority: P2
Reviewed head: 86e5caf0af9d353041dbf3e6df8e389292b67db5

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused repair with relevant production-builder observations, regression coverage, and no blocking findings.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The captured before/after output exercises the changed docs builder through its actual entrypoint with private-use shell text and 7,000 JSON strings, observing exact source preservation after the fix. Focused producer tests supplement that proof. No stored-data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The captured before/after output exercises the changed docs builder through its actual entrypoint with private-use shell text and 7,000 JSON strings, observing exact source preservation after the fix. Focused producer tests supplement that proof. No stored-data contract changes.
Evidence reviewed 7 items Introduced change: The pinned merge-base-to-head diff changes only the docs builder, its package test command, and a new regression file. Source fragments remain escaped; generated spans are excluded from subsequent pattern passes.
Current-main defect remains: Current main still generates placeholders using String.fromCharCode(0xe000 + idx) and restores only U+E000–U+F8FF. Literal characters in that range collide with the stash, and matches beyond its 6,400 characters escape restoration.
Latest-release check: Inspection of v4.8.0 shows the same placeholder generation and restoration implementation, so the latest supplied release does not already contain this repair.
Findings None None.
Security None None.

How this fits together

Peekaboo’s documentation builder converts Markdown into the HTML published by GitHub Pages. Its syntax highlighter formats fenced code blocks that readers view and copy.

flowchart LR
  A[Markdown documentation] --> B[Extract fenced code]
  B --> C[Choose language patterns]
  C --> D[Separate text and highlighted fragments]
  D --> E[Escape source and render spans]
  E --> F[Published HTML code blocks]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production +33/-31; tests +57/-0; package command +1/-1 The small production growth replaces collision-prone placeholders and is accompanied by focused producer regressions.

Technical review

Best possible solution:

Keep highlighted HTML separate from source text so published code remains copyable without character collisions or a fixed match limit.

Do we have a high-confidence way to reproduce the issue?

Yes: current-main source establishes collisions for private-use characters and failed restoration beyond 6,400 matches. The contributor supplies before/after builder observations; this review did not execute the builder.

Is this the best way to solve the issue?

Yes: separating generated spans from raw text directly removes both failure mechanisms while retaining HTML escaping and existing language dispatch.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 91cd87e79bf0.

Labels

Label changes:

  • add P2: This repairs corrupted copyable documentation examples with a limited docs-rendering blast radius.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The captured before/after output exercises the changed docs builder through its actual entrypoint with private-use shell text and 7,000 JSON strings, observing exact source preservation after the fix. Focused producer tests supplement that proof. No stored-data contract changes.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured before/after output exercises the changed docs builder through its actual entrypoint with private-use shell text and 7,000 JSON strings, observing exact source preservation after the fix. Focused producer tests supplement that proof. No stored-data contract changes.

Label justifications:

  • P2: This repairs corrupted copyable documentation examples with a limited docs-rendering blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured before/after output exercises the changed docs builder through its actual entrypoint with private-use shell text and 7,000 JSON strings, observing exact source preservation after the fix. Focused producer tests supplement that proof. No stored-data contract changes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured before/after output exercises the changed docs builder through its actual entrypoint with private-use shell text and 7,000 JSON strings, observing exact source preservation after the fix. Focused producer tests supplement that proof. No stored-data contract changes.

Evidence

What I checked:

  • Introduced change: The pinned merge-base-to-head diff changes only the docs builder, its package test command, and a new regression file. Source fragments remain escaped; generated spans are excluded from subsequent pattern passes. (scripts/build-docs-site.mjs:729, 86e5caf0af9d)
  • Current-main defect remains: Current main still generates placeholders using String.fromCharCode(0xe000 + idx) and restores only U+E000–U+F8FF. Literal characters in that range collide with the stash, and matches beyond its 6,400 characters escape restoration. (scripts/build-docs-site.mjs:727, 91cd87e79bf0)
  • Latest-release check: Inspection of v4.8.0 shows the same placeholder generation and restoration implementation, so the latest supplied release does not already contain this repair. (scripts/build-docs-site.mjs, 4d43dc9d80cd)
  • After-fix production-builder proof: The captured PR body reports running node scripts/build-docs-site.mjs against owned Markdown fixtures: private-use shell characters are preserved after the fix, and a 7,000-string JSON block changes from non-preserved to preserved source. It also reports a successful normal docs build emitting 70 HTML pages. This exercises the changed static HTML producer rather than an unrelated startup check. (scripts/build-docs-site.mjs:347, 86e5caf0af9d)
  • Focused regression coverage: The new tests invoke the actual builder with temporary Markdown inputs and compare decoded code text after removing only renderer-owned span wrappers. They cover private-use characters, 7,000 JSON strings, literal markup, and ordered highlighting controls. The package command includes both docs-site test files. (tests/docs-site-code-preservation.test.mjs:11, 86e5caf0af9d)
  • Routing history and inspection limits: Available main-branch history repeatedly identifies Peter Steinberger on the docs builder, including documentation-hub work and its latest recorded security cleanup. Full blame and older blob inspection failed because required promisor objects could not be fetched; no exact source-line introduction is asserted. GitHub canonical-search requests were also blocked. These limitations do not prevent inspection of the introduced diff, current implementation, or release implementation. No builds or artifact-producing tests were executed, and git status remained clean. (scripts/build-docs-site.mjs, 1cd64648f458)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete

steipete commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

The owner consolidation is #992, combining the five related rendering repairs after independently reproducing them on current main. Its 24 renderer tests, metadata tests, lint, full site build, and P0–P2 review pass. It also includes actual Chrome before/after screenshots using identical synthetic input. This original will be closed as superseded once the combined candidate completes CI and lands; contributor credit is retained.

@clawsweeper

clawsweeper Bot commented Oct 7, 2026

Copy link
Copy Markdown

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(docs): preserve source text through syntax highlighting This is item 1/1 in the current shard. Shard 0/1.

This temporary status tracks the active review worker. The completed review will appear in the durable ClawSweeper review comment.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@steipete

steipete commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Thanks for identifying source-text corruption during highlighting. Main reproduces the token/Unicode collisions. Owner PR #992 uses fragments instead of reserved placeholders, with exact decoded-text regressions and browser proof. Closing this duplicate while that successor completes its landing gates.

@steipete steipete closed this Oct 7, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f5e962d. Configure here.

});
result.push(fragment.slice(previous));
return result;
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anchors bind to fragments not lines

Low Severity

highlightFragments runs each pattern on leftover source slices, so ^ and $ now mean fragment edges rather than the original line. Mid-line shell comments stop at an earlier quoted match, and later passes can restyle the remainder. Copyable text stays intact; only span assignment changes.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f5e962d. Configure here.

steipete added a commit that referenced this pull request Oct 7, 2026
Consolidate the reproduced CRLF metadata, entity/link, heading identity, EOF fence, and highlighter token defects from #939, #931, #940, #969, and #971. Keep highlighted fragments separate from source text and retain the shared renderer gate.

Reconcile the Unreleased note and menu-preparation documentation with current main. All 25 docs-site regressions pass; independent Codex review is clean through P2.

Co-authored-by: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com>
steipete added a commit that referenced this pull request Oct 7, 2026
Documentation rendering lost literal code text, leaked CRLF front matter into articles, double-escaped TOC text and link queries, reused heading anchors, and discarded fenced code at EOF. This consolidates the verified fixes from #939, #931, #940, #969 and #971, with credit to @rudycelekli.

The highlighter now keeps rendered fragments separate from source text, with no reserved source characters or 6,400-token limit. The page-wide heading allocator preserves natural anchors and assigns unique duplicate/fallback IDs. Front matter is normalized before extraction; renderer-owned entities are decoded once; link suffixes remain intact; EOF flushes the pending fence. Normal macOS CI now runs the complete shared renderer gate.

Preserve the existing regression and platform proof from the PR. Reconcile with current main and retain the Unreleased changelog. Independent Codex review is clean through P2, and the final exact-head CI checks pass.

Co-authored-by: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants