Repository navigation
update: tests & performance - #2
Merged
Merged
Conversation
CHANGELOG.md was removed in 5d62317 but the test that runs the release guard against the repository's own files still read it unconditionally, so `npm test` died with ENOENT in CI. The guard's contract for a missing changelog is refusal (it cannot know whether a break is pending), so the oracle now expects exactly that when the file is absent, and keeps the BREAKING-vs-tag equivalence when it is present. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q6HFJQt8QvipUNrVnP6Wia
… deprecated alias The tarball mobile-react-app vendors (0.12.0-dev.a1884d4) carried two entry files and three exports entries this branch never had: src/engine.ts and src/stream.ts behind `./engine` and `./stream`, and `./node` for the addon loader, with native/node and scripts/build-node-addon.mjs in `files` so a consumer's jest can build the engine out of node_modules. The app imports all three subpaths, so a release cut from this branch as it stood would not resolve in it. Both barrels are star-free and never reach the view layer; src/index.test.ts pins that, so neither can grow a react-native import. `classifyBlock` stays at the root as a deprecated alias of `classifyTopLevelBlock` — same signature, and the binding is still the new name, so the prop/function collision the rename fixed does not come back. That turns the BREAKING entry into a deprecation. The changelog counted nineteen removed internals; ten were ever on the published root, and the entry now names them and where they moved. verify:pack resolves the three subpaths and loads the two dist barrels under both conditions. The stream barrel re-exports bindRunTextEvents, whose module imports react at load, so it is a PEER_DEPENDENT entry loaded after `react` is linked into the unpacked package; the Node-safe entries still load before any peer exists, which keeps proving they need none. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Semibold loses bold on older Android
- Restored the pre-API 28 StyleSpan cutoff to weight >= 600 so themed semibold still gets synthetic bold.
- ✅ Fixed: Link bounds ignore text scroll
- Stopped re-adding scrollX/scrollY so virtual-view bounds stay in the TextView parent space that ExploreByTouchHelper expects.
Or push these changes by commenting:
@cursor push 497b06abbf
Preview (497b06abbf)
diff --git a/android/src/main/java/com/selectablemarkdown/RunAccessibility.kt b/android/src/main/java/com/selectablemarkdown/RunAccessibility.kt
--- a/android/src/main/java/com/selectablemarkdown/RunAccessibility.kt
+++ b/android/src/main/java/com/selectablemarkdown/RunAccessibility.kt
@@ -220,7 +220,6 @@
return
}
node.contentDescription = textOf(range)
- bounds.offset(textView.scrollX, textView.scrollY)
node.setBoundsInParent(bounds)
if (range.pressable != null) {
node.className = "android.widget.TextView"
diff --git a/android/src/main/java/com/selectablemarkdown/RunAttributedText.kt b/android/src/main/java/com/selectablemarkdown/RunAttributedText.kt
--- a/android/src/main/java/com/selectablemarkdown/RunAttributedText.kt
+++ b/android/src/main/java/com/selectablemarkdown/RunAttributedText.kt
@@ -270,7 +270,7 @@
attribute.fontWeight?.let { declared ->
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
out.setSpan(RunFontWeightSpan(declared), start, end, flags)
- } else if (declared >= 700) {
+ } else if (declared >= 600) {
out.setSpan(StyleSpan(Typeface.BOLD), start, end, flags)
}
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 172da90. Configure here.
| out.setSpan(RunFontWeightSpan(weight), start, end, flags) | ||
| } else if (weight >= 600) { | ||
| out.setSpan(RunFontWeightSpan(declared), start, end, flags) | ||
| } else if (declared >= 700) { |
There was a problem hiding this comment.
Semibold loses bold on older Android
Medium Severity
The pre-API 28 StyleSpan path now treats only weights >= 700 as bold, so 600 (semibold) renders regular. That contradicts the CSS 600 cutoff still documented on parseFontWeight and in theme.ts. Defaults use 700 and stay fine; themed 600 on API 24–27 does not.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 172da90. Configure here.
| return | ||
| } | ||
| node.contentDescription = textOf(range) | ||
| bounds.offset(textView.scrollX, textView.scrollY) |
There was a problem hiding this comment.
Link bounds ignore text scroll
Low Severity
boundsFor already converts layout coordinates into the TextView’s visible space by subtracting scrollX/scrollY. Offsetting those bounds back by the same scroll before setBoundsInParent puts virtual-view rectangles in content space. ExploreByTouchHelper then maps them to screen with only getLocationOnScreen, so a scrolled run’s TalkBack/keyboard highlight misses the glyphs.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 172da90. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Medium Risk
Changes span release automation, new CI performance gates, and native Android selection/accessibility/install contracts that consumers and integrators must align with.
Overview
CI and release now enforce performance budgets (
bench:pathological,bench:streaming,bench:projectionwith--require-engine), compile allplatform/cppsources per platform, and drop the standalonenpm run buildstep from the main test job. Workflows add least-privilegepermissions, checkoutfetch-depth: 0, and concurrency controls.Tag releases are split into preflight (tag vs
package.json, lockfile sync, changelog section, unreleased BREAKING guard), macOS gates (Swift + iOS Fabric compile), then a release job that mirrors CI gates, packs once, publishes that exact tarball with provenance, and attaches it to a GitHub release whose notes come fromCHANGELOG.md(not auto-generated).Adds
CHANGELOG.mdand a large README refresh (selection API, streaming/dispose, exports map, Jest allowlist, releasing steps). Android gains TalkBack virtual nodes for links/headings/list items/table cells, imperative selection (clearSelection/setSelection,onSelectionChange,exclusiveSelection), localized/custom selection menu titles, font resolution viaReactFontManager, and measure/layout alignment (edge box padding, text direction, resolved typefaces in the layout cache).install()on native now returnsinstalled|unavailable|refusedinstead of a boolean. Default Android SDK targets move to 36/24.Reviewed by Cursor Bugbot for commit 172da90. Bugbot is set up for automated code reviews on this repo. Configure here.