Skip to content

perf(mobile): scan HTML code ranges once during Markdown conversion - #21278

Open
luke2x wants to merge 6 commits into
pingdotgg:mainfrom
luke2x:perf/mobile-html-code-ranges
Open

luke2x wants to merge 6 commits into
pingdotgg:mainfrom
luke2x:perf/mobile-html-code-ranges

Conversation

@luke2x

@luke2x luke2x commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The mobile HTML-to-Markdown helper checks every HTML tag against codeRanges.some(...), restarting at the first code range for each tag. Long content containing many inline code examples and HTML image embeds makes that preservation check quadratic, before Markdown parsing or native text layout begins.

Change

Advance one range cursor as the HTML replacement callbacks move forward through source offsets. Each expired code range is visited once. Tags inside code remain literal; real HTML images and wrappers outside code retain their existing conversion.

Scope and approval

A small implementation-only performance fix to one mobile Markdown helper: seven production lines. It preserves the current HTML/code conversion behavior and qualifies for the focused small-fix exception. It does not change rendering defaults or claim to solve the separate native text-mount stalls in #13925.

Verification

  • vp test run apps/mobile/src/lib/markdownHtml.test.ts apps/mobile/src/lib/markdownMedia.test.ts — 28 passed. New cases cover many alternating literal/converted image tags, conversion after the last range, and overlapping ranges following an unclosed fence. Existing HTML entity, attribute, code-fence, screenshot, and media cases pass.
  • Mobile typecheck and formatting passed. Focused lint had no errors; two existing array-reverse warnings remain in other functions in the file.

Performance comparison

Workload / metric Before After Observed difference
3,000 code ranges / 6,000 image tags — median 64.355666 ms 2.788375 ms 95.67% less helper time
Same workload — p95 71.968875 ms 3.798083 ms Lower tail latency in this fixture
Converted output Baseline string Byte-for-byte identical string Equality asserted before timing

The synthetic document alternates literal <img> examples inside inline code and real image embeds outside it. Ordered tag offsets allow the preservation check to advance through expired code ranges once, instead of rescanning from the first range for every tag. Existing parsing and conversion rules remain responsible for recognizing the ranges and producing Markdown.

Reproduce the measurements

Benchmark script · all raw samples · method, workloads, environment and full commit refs

The script extracts the actual helper source at the named Git commits, strips TypeScript, asserts before/after output equality, warms both versions five times, then takes 30 alternating samples per side. GC runs outside timing. Inputs are reused. Recorded environment: Node v26.10.0 / V8, macOS arm64, Apple M5. The reported median is the upper middle sample; p95 is the recorded 29th ordered sample out of 30.

Exact command and benchmark revisions

Download the linked script and run it from a checkout containing these commits (fetch the three luke2x/t3code performance branches if needed):

node --expose-gc performance-bench.mjs \
  d9eb2b61c3899f1ade376fd00f1ebb7fd11cf47e \
  98cc2b434fc6d35824009ade2ceffe78bf8d76fe \
  374b83c181db3d2a608060cab842a3f398773dbd \
  59090b82c32f369acf99add8ecb17f2884b8ee6f

Arguments are baseline, tree fix, preview fix, and HTML fix. Subsequent main merge commits did not alter any of the three benchmarked helper files; this was checked with Git diffs. The source revisions keep the original measurement reproducible.

Measurement limits

These are synthetic production-helper CPU measurements on Node/V8. They exclude caller rendering, native Hermes, native layout, scrolling frames, and heap measurements. No screenshots or native recording were collected because there is no visual-output change being claimed and this host has no native device/simulator available. The focused tests establish output behavior; these timings establish the cost of the isolated helper for the stated fixtures.

CI and review status

As of the 19 October 2026 audit, CI on 8d36084e11 passes: 20 successful checks and 7 skipped, with no failing or pending checks. The branch also merges cleanly against main at 07d38b8d17. Earlier shared settings-mock and web test-dependency failures are historical; they are not current blockers. CI success does not replace maintainer review or establish native performance measurements.

Macroscope approved the implementation at its original reviewed commit; subsequent heads include merges from main. Approval at that SHA is not a claim that all checks or other reviewers have approved the latest head.

Implemented and tested with OpenAI Codex (gpt-6.1-sol) in T3 Code. Review follow-ups by DeepSeek V4.1 Flash 1017.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T19:59:52.639040Z 59090b8 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 18, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 59090b8

Macroscope's review found this PR approvable — This is a small, localized mobile performance optimization that preserves existing Markdown/HTML conversion behavior and adds focused regression coverage. It introduces no product-default, schema, deployment, security, billing, or static-analysis changes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between c871258 and 63eac02.

Files selected for processing: 2
  • apps/mobile/src/lib/markdownDocument.ts
  • apps/mobile/src/lib/markdownHtml.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

markdownHtmlTags now advances through ordered code ranges while filtering HTML tags. Tags inside code ranges remain excluded. Tests cover HTML image conversion among inline-code ranges and literal tags after an unclosed fence.

Changes: Markdown HTML conversion

Layer / File(s) Summary
Code-range-aware conversion
apps/mobile/src/lib/markdownDocument.ts, apps/mobile/src/lib/markdownHtml.test.ts
markdownHtmlTags advances through ordered code ranges while filtering tags. Tests check HTML image conversion, inline-code preservation, and literal tags after an unclosed fence.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related PRs

  • pingdotgg/t3code#24263: Rewrites the same mobile HTML-to-Markdown conversion path and adds conversion tests.

Suggested reviewers: flamboh, juliusmarminge

Merge Risk: Minimal · up to 63eac

No actionable merge-blocking risk was identified in the changed Markdown conversion path.

Architecture risk: Low · up to 0bad2

The change affects 1 system.

Changed systems: apps/mobile

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/mobile (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/mobile/src/lib/markdownDocument.ts: markdownHtmlContent now advances a range index as tag offsets pass code-range ends and preserves tags when they fall within the current range, replacing the prior per-tag scan of all code ranges.
  • observed — Modified behavior in apps/mobile/src/lib/markdownHtml.test.ts: Added a test covering 100 lines of inline-code and HTML-image markup: inline tags remain literal, image tags are converted, and trailing text after <br> is separated by a Markdown line break.
  • observed — Modified behavior in apps/mobile/src/lib/markdownHtml.test.ts: Added a test asserting that tags after an unclosed HTML fence remain literal, including tags inside a later inline-code range; preceding <div> markup is converted.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the main change: a performance fix in the mobile Markdown conversion helper that scans HTML code ranges once instead of rescanning them for each tag. It follows the conventional commit…
Description check Passed The description follows the template. It has Problem, Change, Scope and approval, and Verification sections, and it fills each one with specific content. The Scope section explains why the change qual…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: a performance fix in the mobile Markdown conversion helper that scans HTML code ranges once instead of rescanning them for each tag. It follows the conventional commit format and is specific enough for a reviewer scanning history.

Full details: Description check

Explanation

The description follows the template. It has Problem, Change, Scope and approval, and Verification sections, and it fills each one with specific content. The Scope section explains why the change qualifies as a small focused fix without a prior issue. The Verification section lists the focused test command and results, a benchmark table, and the measurement limits. The description is long and includes CI and review status details that the template does not request, but the required information is present. The Scope section links no triaged issue or maintainer approval, and it cites issue #13925 only as a separate concern. The template asks for explicit approval for scope, or a clear explanation of why a small obvious fix needs none. The author gives that explanation, so the section is acceptable. The Verification section also says no screenshots were collected, which is reasonable because the change has no visual output. The reproduction steps depend on external links and a commit list, which a reviewer must check.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@luke2x

luke2x commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

Note

Written by deepseek v4.1 flash 1017 on behalf of luke.

Updated this branch with the latest main. It was 23 commits behind, so I merged main into perf/mobile-html-code-ranges, which brings the PR fully up to date. The merge was clean with no conflicts, and the PR diff is otherwise unchanged. CI will re-run against the new base.

@luke2x

luke2x commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Note

Written by gpt 6.1 sol on behalf of Luke

Resolved the conflict with current main in c871258. Main now filters parsed HTML tags before converting links, so I moved the forward-only code-range cursor into that filter. This preserves main’s anchor/link handling, table-cell handling, and escaping while keeping this PR’s linear scan across code ranges.

vp run check:changed upstream/main passed formatting, lint, both knip checks, mobile/scripts typechecks, and all 10 HTML tests. The separate HTML/media run passed all 22 tests, including the many-range and unclosed-fence regressions. Lint reports existing reverse-array warnings. The earlier benchmark numbers were not rerun against this newer Markdown implementation; no new native performance measurement is claimed.

@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:XS 0-9 changed lines (additions + deletions). labels Oct 24, 2026
@luke2x

luke2x commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Note

Written by gpt 6.1 sol on behalf of Luke

Resolved the conflict with main in 63eac02. Both this PR's HTML code-range regressions and main's file-embed tests are retained.

Verification against d1f7018: all 36 Markdown HTML/file-embed tests passed, as did both affected package typechecks, formatting, lint, and the first dependency-analysis pass. Checks ran sequentially with one test worker and a 1 GB heap cap. The second dependency-analysis pass remains deferred to CI; native device behavior was not reverified.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant