Skip to content

fix: preserve balanced Markdown links and own native speech lifetime - #44

Merged
armstrongsam25 merged 6 commits into
AstralDeep:mainfrom
xiasan1992:codex/issue-37-balanced-link-destinations
Oct 13, 2026
Merged

armstrongsam25 merged 6 commits into
AstralDeep:mainfrom
xiasan1992:codex/issue-37-balanced-link-destinations

Conversation

@xiasan1992

@xiasan1992 xiasan1992 commented Oct 3, 2026 •

Copy link
Copy Markdown

Fixes #37.

Xia San’s parser preserves balanced and escaped parentheses in Markdown destinations with bounded length and nesting. Cached competing code/link matches avoid repeated scans of unchanged suffixes while retaining HTML escaping, safe URL schemes, literal code spans and formatted link labels.

Current main is integrated at f44ff05. The prior Windows speech ownership production fix is already byte-identical to main; this PR retains its additional QObject ownership/destruction regressions. The extraction ledger preserves current-main inventory and binds the actual sanitizer bytes.

Original contribution by @xiasan1992. Original commits, PR author and existing co-author credit remain preserved.

Validation: all five exact-head hosted Python/web/Windows-source/Windows-package/aggregate checks pass, including the formerly failing full Windows lane. Python 3.11 focused verification passes 83 tests; all 75 changed sanitizer lines are covered. The full local source suite passes 4,381 tests and 33 subtests with six platform skips. Its single clean-wheel example failure also reproduces unchanged on main because a nested system-site-packages environment cannot find the dependency installed only in the parent virtualenv; hosted package qualification passes.

A fresh live component backend serves the exact candidate renderer to Chromium at 1280px and 320px. Ten checks per viewport verify balanced/nested/escaped/query destinations, literal code, malformed-input rejection, unsafe schemes, inert hostile labels, safe link attributes and formatted labels; screenshots inspected. This is isolated renderer qualification, not an authenticated deployed-client acceptance claim. The parser bytes are unchanged by the main merge.

No dependency, schema, UI vocabulary, protocol, consumer pin, deployment or release change is added by this completion.

@xiasan1992

Copy link
Copy Markdown
Author

The workflow for this fork PR was created but ended action_required with zero jobs: run 37147160855. Could a maintainer approve this run so the repository CI can execute? The current head has 612 passing Web-renderer tests, 100% changed-line coverage for sanitize.py, and a clean Ruff check locally.

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

@xiasan1992, CI needs fixes on 86b5f90.

  • python — Run python -m pytest -q -p no:cacheprovider --covastralprojection --covrote --covwebrender --covscripts.merge_xccov_line_coverage --covscripts.build_offline_assets --covscripts.bui (failure)
  • required — Require every owner gate (failure)

Please open the failed job logs, fix the reported CI issues, and push your changes to this PR. CI will run again automatically. Other checks may still be running.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

All applicable PR CI workflows passed for 54bec28.

Ready for maintainer code review.

@armstrongsam25 armstrongsam25 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed exact head 54bec28728355a617e13523e94cf3ef84bcfac9a. The five hosted checks pass, and the link destination bounds are useful, but the new token-selection loop introduces quadratic work over ordinary Markdown input.

At backend/webrender/sanitize.py:113-115, each code span calls _next_link(s, cursor) again over the full remaining suffix. For "`code` " * n there are no link starts, so every call searches the whole suffix. Independent deterministic instrumentation produced:

Code spans Input characters Link scans Total suffix characters searched
128 896 129 57,920
256 1,792 257 230,528
512 3,584 513 919,808

Doubling this normal input approximately quadruples the work. The 2,048-character destination and eight-level nesting limits do not bound repeated whole-input scans, leaving a denial-of-service path in the public sanitizer. Please retain the escape/URL/token-precedence behavior while advancing or caching the competing matchers so each suffix is scanned at most once, and add deterministic bounded-work tests for code-only and mixed input. This is a source defect independent of CI status.

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Issue context is present for 7a8e3c0a8f73e2bfed3b365a0ca0b564aff5cabd. Maintainer review still determines whether the change and its claims are supported.

@github-actions github-actions Bot added the triage:needs-context Please explain the problem, issue scope, and verification evidence. label Oct 6, 2026
armstrongsam25 and others added 2 commits October 6, 2026 11:55
Complete Xia San's balanced-link parser by retaining competing code and link matches until consumed. Add deterministic work and overlapping-token regressions and refresh source provenance.

Original contribution by @xiasan1992; the original commits and pull request remain intact.

Co-authored-by: Xia San <xiasan1992@gmail.com>
@armstrongsam25

armstrongsam25 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Thanks @xiasan1992 — your balanced-destination contribution, original commits and PR authorship remain intact. The Markdown completion includes co-author credit, and Fixes #37 preserves your automatic bounty eligibility after a qualifying main merge.

Published head 7a8e3c0a8f73e2bfed3b365a0ca0b564aff5cabd fixes bounded inline scanning and the native Qt speech ownership gap. Exact changed coverage passes all 75 sanitizer and three native speech lines. Real ownership/destruction tests, 180 adjacent speech tests, 74 BYO tests and all 29 source-replay/protocol/architecture tests pass.

The exact-head owning run is complete: Python/web/Windows-package pass; Windows source and the required aggregate fail. The Windows job reproduces the local access violation during application stylesheet replacement in result-presentation setup. Ubuntu passes 3,433 tests with one declared skip and 33 subtests.

Geometry passes 50 tests alone. Ordinary and full-collection-preserving prefixes each pass 1,426 tests with thirteen declared skips, but neither proves a root cause or qualifies the full Windows gate. The current-head review requests changes until the native fault is resolved and a corrected exact head passes. Failed evidence is retained and all required gates stay intact.

The original author branch is updated by normal fast-forward. This PR remains open, unapproved and unmerged; no release or new award is claimed.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

@xiasan1992, CI needs fixes on 1b0f619.

  • windows-tests — Run Windows source tests with changed-line coverage (cancelled)
  • required — Require every owner gate (failure)

Please open the failed job logs, fix the reported CI issues, and push your changes to this PR. CI will run again automatically. Other checks may still be running.

@armstrongsam25 armstrongsam25 changed the title fix: preserve balanced parentheses in Markdown links fix: preserve balanced Markdown links and own native speech lifetime Oct 6, 2026

@armstrongsam25 armstrongsam25 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The balanced-link and bounded-scanning repair is covered, and the native speech ownership correction passes real GUI-thread destruction/injected-ownership tests and its changed-line gate. Original author/history/credit and the #37 closing reference are preserved.

The required Windows source job on this exact head now reproduces the local access violation at astral_client.app.configure line 5369 (QApplication.setStyleSheet), called from the native style fixture as result-presentation tests begin. See run https://github.com/AstralDeep/AstralProjection/actions/runs/37498023707/job/112387484323. The owning Ubuntu job passes 3,433 tests with one declared skip and 33 subtests.

The 50 geometry tests pass alone; both ordinary and full-collection-preserving executed prefixes pass 1,426 tests with thirteen declared skips. Those diagnostics narrow the boundary but do not prove a root cause or qualify the full gate. Keep this PR blocked until the native fault is resolved and the corrected exact head passes the full required Windows job. Failed local/provider logs are retained; no gate has been relaxed or backend mocked to qualify the repair.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

@xiasan1992, CI needs fixes on 7a8e3c0.

  • windows-tests — Run Windows source tests with changed-line coverage (failure)
  • required — Require every owner gate (failure)

Please open the failed job logs, fix the reported CI issues, and push your changes to this PR. CI will run again automatically. Other checks may still be running.

@github-actions github-actions Bot removed the triage:needs-context Please explain the problem, issue scope, and verification evidence. label Oct 6, 2026

@armstrongsam25 armstrongsam25 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2026-10-11 core-repository review: held for repair/qualification at 7a8e3c0a8f73e2bfed3b365a0ca0b564aff5cabd.

Useful, credited balanced Markdown-link and native speech ownership work. Current exact head still fails required Windows source job with native QApplication.setStyleSheet access violation; conflicting with main. Do not call it low effort.

The existing changes-requested review on this same commit remains applicable.

The complete diff, existing discussion and current-head checks were reviewed. Useful partial work is retained; this review does not authorize bypassing a required gate.

@armstrongsam25

Copy link
Copy Markdown
Contributor

Reviewed 7a8e3c0a8f73e2bfed3b365a0ca0b564aff5cabd. The useful bounded Markdown parser and native speech ownership work should remain open. Main now includes 914b999's Qt screen-lifecycle repair, so update this branch from main, reconcile the provenance ledger against the resulting source bytes, and run the complete required Windows source job on the new exact head. Current merge conflict is confined to provenance; the previous native fault is not resolved until the combined head passes. Preserve contributor history and Fixes #37.

@armstrongsam25

armstrongsam25 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Integrated current main into the original branch at f44ff05. The Qt speech ownership implementation is already byte-identical to main; the PR retains its additional ownership regression tests. The provenance conflict preserves current-main records and binds the balanced-link sanitizer's actual bytes.

Reviewed the bounded parser and scan cache, escape/scheme handling and malformed/overlap regressions. Focused Python 3.11 verification passes 83 tests, with all 75 changed sanitizer lines covered. Full local source verification passes 4,381 tests and 33 subtests, with six platform skips; one unchanged clean-wheel example failure is being checked against main. Fresh hosted Python, web, full Windows source and packaging qualification is running. Merge is conditional on required gates and affected live behavior qualification; no gate bypass or rerun.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

All applicable PR CI workflows passed for f44ff05.

Ready for maintainer code review.

@armstrongsam25 armstrongsam25 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed exact repaired head f44ff05. Current-main merge resolves provenance and brings in the separately qualified native screen-lifecycle repair; speech ownership production bytes already match main. Parser escaping, bounded depth/length, cached scans and overlap handling pass deterministic regressions with 75/75 changed sanitizer lines covered. Live candidate renderer/Chromium smoke passes ten checks at each of 1280px and 320px. All five fresh hosted gates pass, including full Windows source and packaging, resolving the previous required-check block. The sole broad local installed-wheel failure reproduces identically on current main and does not affect the passing hosted gate.

@armstrongsam25
armstrongsam25 merged commit 6562fc9 into AstralDeep:main Oct 13, 2026
5 checks passed
@armstrongsam25

Copy link
Copy Markdown
Contributor

Merged normally after all five exact-head checks, including full Windows source/packaging, and live component renderer/browser verification passed. Thanks @xiasan1992 for the balanced-link parser contribution; your original PR, commits and co-author credit are preserved. Consumer adoption/deployment remains separate.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve balanced parentheses in rendered Markdown link destinations

2 participants