Repository navigation
fix(sync): reconcile upstream CRLF store export coverage - #402
Conversation
colbymchenry#2364) The kernel's is_exported_later mirrors TreeSitterExtractor.isExportedLater's multiline regex, but JS's `m` flag treats `\r` as a line end while the regex crate's `(?m)` sees only `\n`. On a CRLF checkout (Windows, autocrlf) a line like `export default useStore;\r\n` matched in wasm but not in the kernel, so `const useStore = create(...)` + `export default useStore;` lost its actions (e.g. `function inc`) on the default kernel route for TS/TSX/JS/JSX. Compile it in CRLF mode, `(?mR)`: `^`/`$` then treat `\r` as a line end too, and the pattern body stays byte-identical to the TS regex. It agrees with the JS regex on CRLF, trailing whitespace and lone `\r` line ends, where an optional `\r?$` would still miss a lone `\r`. This was the only `(?m)` regex in the kernel. The other tsjs regexes are whole-string anchors, and the minified-bundle check splits lines on `\n` exactly as the TS does. Adds a kernel/wasm parity case (ts/tsx/js/jsx, LF and CRLF) that fails on the unfixed kernel for all four CRLF variants and passes with the fix. Co-authored-by: Claude Opus 5.5 <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe pull request adds extraction test coverage and documentation for JavaScript and TypeScript store actions exported on a later line across LF, CRLF, and CR line endings. It also updates the README's upstream comparison baseline. ChangesStore Action Export Coverage
Upstream Baseline Update
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This change adds regression coverage and documentation without changing extraction behavior. No actionable merge-blocking risk was identified in the reviewed diff. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 4 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Comment |
|
The description and diff agree: this is an ancestry-preserving reconciliation through upstream aeb8f95, with regression coverage and documentation. The fork already handles later exports through AST binding facts, so native production source is unchanged. The new tests cover four grammars and three line endings. This does not claim a newly introduced fork behavior fix. |
2 participants
Reconciles upstream through
aeb8f95581f23946d04b5ae9b3dbda918f5a7700while preserving upstream ancestry. The fork already reads later exports from AST binding facts, which handles the CRLF store-export case fixed upstream. This keeps that implementation and ports the native extraction coverage for TS, TSX, JS and JSX with LF, CRLF and CR line endings. The deleted WASM comparison test stays deleted.README's upstream merge point and comparison reference now name this tip; its feature tables and dated benchmark rows were checked, with no measured values changed. The docs site describes the same line-ending contract. Native source bytes are unchanged; the matching qualified native artifact was reused. No live build promotion is included.
Validation: product build and 46 focused export/golden/README checks passed. The full suite passed 606 test files with 7,764 tests passed and 39 skipped. The native source stamp, test floor and staged whitespace checks passed. Remote CI and review gates remain pending. Initial isolated checks stopped before tests on read-only copied build-cache paths; private-copy cache permissions were corrected and the build/focused checks passed.