feat: tywrap promotion, docs site, and maintenance automation#200
feat: tywrap promotion, docs site, and maintenance automation#200
Conversation
Reorganize flat docs/ files into guide/, reference/, examples/, and troubleshooting/ subdirectories; fix all cross-doc links to use absolute VitePress paths; rename README.md files to index.md; exclude plans/ from the VitePress build to prevent dead link failures.
Also adds ignoreDeadLinks to VitePress config to allow incremental doc authoring while reference pages (env-vars, cli) are pending.
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used🧠 Learnings (5)📚 Learning: 2026-01-19T21:48:27.823ZApplied to files:
📚 Learning: 2026-01-20T01:34:07.064ZApplied to files:
📚 Learning: 2026-01-20T18:37:05.670ZApplied to files:
📚 Learning: 2026-01-20T16:00:49.829ZApplied to files:
📚 Learning: 2026-01-19T21:14:37.032ZApplied to files:
🔇 Additional comments (23)
📝 WalkthroughWalkthroughAdds a VitePress docs site and many docs pages; introduces CI/CD workflows (docs deploy, labeler, release-please, stale, dependabot); updates packaging metadata; refines config types and exports; expands annotation parsing, mapping, and generator logic; refactors Node runtime warmup/worker-pool with background replacement/readiness probing; and adds/updates tests and tooling scripts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Consumer
participant Pool as WorkerPool
participant Worker as ActiveWorker
participant Replacement as ReplacementWorker
participant Bridge as NodeBridge
Client->>Pool: acquireWorker()
Pool->>Worker: allocate existing worker
alt worker healthy
Worker->>Bridge: executeWorkerCall(request)
Bridge-->>Worker: response
Worker-->>Client: return response
else worker failed / timed-out
Pool->>Pool: scheduleReplacementWorker()
Pool->>Replacement: create replacement (background)
Replacement->>Bridge: readiness probe (executeWorkerCall)
Bridge-->>Replacement: readiness ok
Replacement-->>Pool: mark available
Pool->>Worker: dispose old worker
Pool->>Client: allocate Replacement -> execute request
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/labeler.yml:
- Around line 24-31: The labeler config under the "area:docs" rule contains
redundant explicit file entries ('CONTRIBUTING.md', 'AGENTS.md', 'CLAUDE.md')
because the '*.md' glob already matches them; update the "any-glob-to-any-file"
list in the area:docs rule to remove those explicit filenames (leave '*.md' and
'docs/**' intact) so the rule is not redundant—look for the "area:docs" block
and the "any-glob-to-any-file" list to make this change.
In `@AGENTS.md`:
- Around line 40-61: The markdown fenced code block containing the repository
tree is missing a language identifier and triggers MD040; update the opening
fence to include a language such as "text" or "plaintext" (i.e., change the
triple-backtick that starts the repo tree to ```text or ```plaintext) so the
directory listing is treated as plain text and the linter warning is resolved.
In `@CLAUDE.md`:
- Around line 1-4: The top-level heading currently reads "# AGENTS.md" but the
file is CLAUDE.md; update the heading text to "# CLAUDE.md" so the document
title matches the filename and prevents confusion with other agent docs—change
the string in the heading line at the top of the file (the "# AGENTS.md" token)
to "# CLAUDE.md".
- Around line 40-61: The fenced repository tree in CLAUDE.md is missing a
language tag and triggers markdownlint MD040; update the opening fence from ```
to ```text so the block is treated as plain text (the tree showing src/,
tywrap_ir/, runtime/, test/, docs/, examples/, generated/, dist/), i.e., change
the code fence that wraps the repo tree to use the "text" language tag.
In `@docs/guide/runtimes/bun.md`:
- Around line 40-54: Update the text that currently says "tywrap ships with a
`bunfig.toml`" to clearly state that the shown `bunfig.toml` is an example
configuration users should add to their own projects when using Bun, because the
package does not include `bunfig.toml` (it is excluded via .npmignore / not
listed in package.json `files`). Replace the phrasing around the `bunfig.toml`
header and the surrounding paragraph to: present the TOML block as a sample
`bunfig.toml` to create in your project, not a file distributed by tywrap, and
keep the example content exactly as shown (`[build]` and `[dev]` entries).
In `@docs/guide/runtimes/comparison.md`:
- Around line 28-37: Update the fenced code block that starts with ``` and
contains the decision diagram beginning "Do you need subprocess-based Python
execution?" to include a language identifier (e.g., change the opening fence to
```text) so the block becomes ```text ... ```—this satisfies markdown linters
and improves accessibility; ensure any other plain triple-backtick blocks in the
same comparison diagram are similarly updated.
In `@docs/guide/runtimes/http.md`:
- Around line 27-31: Add a blank line before the fenced code block that starts
with ``` (the snippet showing headers: { 'Authorization': 'Bearer your-token',
}), to satisfy Markdown rules; edit docs/guide/runtimes/http.md so there is an
empty line separating the preceding heading/paragraph and the code fence.
- Around line 40-49: Add a blank line between the "Running the Python Server"
heading and the following fenced code block so the markdown is compliant with
MD022/MD031; locate the heading "Running the Python Server" and ensure there is
an empty line before the triple-backtick block that starts with "```bash" (the
pip install / python -m tywrap_ir.server snippet).
In `@docs/plans/2026-03-10-tywrap-promotion.md`:
- Around line 1407-1409: The plan uses two different major versions of the same
GitHub Action: update the workflow snippet that currently references
actions/stale@v9 so it matches the Tech Stack declaration (actions/stale@v10);
find the usage of the actions/stale action in the document (look for the token
"actions/stale" in the snippet) and change the pinned major version to `@v10` so
the plan is internally consistent.
In `@docs/reference/cli.md`:
- Around line 37-42: The CLI reference is missing several flags supported by the
"tywrap generate" command; update docs/reference/cli.md's flags table to add
entries for --modules, --runtime, --python, --output-dir, --format,
--declaration, --source-map, --use-cache / --cache, --debug, and --fail-on-warn
with short descriptions matching the CLI implementation, include any default
values or allowed values (e.g., runtime: node|pyodide|http|auto, format:
esm|cjs|both), and ensure aliases (--use-cache / --cache) and CI semantics
(e.g., fail-on-warn exit code) match the behavior implemented in the CLI code
referenced in src/cli.ts.
In `@docs/reference/env-vars.md`:
- Line 54: Update the documentation for NodeBridge's subprocess environment
inheritance to match the actual allowlist used by the implementation: state that
by default NodeBridge inherits PATH and any variables with the TYWRAP_ prefix,
plus the specific keys path, pythonpath, virtual_env, and pythonhome; explicitly
note that generic PYTHON* variables (e.g., PYTHONUNBUFFERED,
PYTHONDONTWRITEBYTECODE) are NOT inherited unless inheritProcessEnv: true is
set, and ensure the doc text references the NodeBridge option inheritProcessEnv
and the exact allowed prefixes/keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6d8691ea-b42c-44aa-acd9-75c5591d2db1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (32)
.github/labeler.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/labeler.yml.github/workflows/release.yml.github/workflows/stale.yml.gitignoreAGENTS.mdCLAUDE.mdREADME.mddocs/.vitepress/config.tsdocs/examples/index.mddocs/guide/configuration.mddocs/guide/getting-started.mddocs/guide/runtimes/browser.mddocs/guide/runtimes/bun.mddocs/guide/runtimes/comparison.mddocs/guide/runtimes/deno.mddocs/guide/runtimes/http.mddocs/guide/runtimes/node.mddocs/index.mddocs/plans/2026-03-10-tywrap-promotion.mddocs/public/llms-full.txtdocs/public/llms.txtdocs/reference/api/index.mddocs/reference/cli.mddocs/reference/env-vars.mddocs/reference/type-mapping.mddocs/troubleshooting/index.mdpackage.jsonrenovate.jsontywrap_ir/pyproject.toml
📜 Review details
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2026-01-19T21:14:40.872Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:40.872Z
Learning: In `src/runtime/bridge-core.ts` and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (e.g., runtime shape checks on error payloads) in tight loops unless the protocol boundary is untrusted or there's a concrete bug report. The Python bridge protocol is controlled and validated via tests, so defensive checks would add unnecessary branching overhead without meaningful benefit.
Applied to files:
AGENTS.mdCLAUDE.mddocs/guide/runtimes/http.mddocs/plans/2026-03-10-tywrap-promotion.mddocs/guide/runtimes/comparison.md
📚 Learning: 2026-01-19T21:14:37.032Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:375-385
Timestamp: 2026-01-19T21:14:37.032Z
Learning: In tywrap (src/runtime/bridge-core.ts and similar), environment variable parsing follows a tolerant/best-effort policy. For example, `TYWRAP_CODEC_MAX_BYTES=1024abc` should be accepted as 1024. Only reject clearly invalid values (non-numeric start or <=0). This avoids surprising failures from minor typos.
Applied to files:
AGENTS.mdREADME.mdCLAUDE.mddocs/reference/env-vars.md
📚 Learning: 2026-01-19T21:49:05.612Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: runtime/python_bridge.py:99-123
Timestamp: 2026-01-19T21:49:05.612Z
Learning: In the tywrap repository, TYWRAP_REQUEST_MAX_BYTES uses strict integer parsing that rejects values with trailing characters (e.g., "1024abc"). This differs from TYWRAP_CODEC_MAX_BYTES, which uses tolerant/best-effort parsing that accepts numeric prefixes. The strict policy for REQUEST_MAX_BYTES ensures explicit integer values and consistent parse behavior across Node/Python implementations.
Applied to files:
README.md
📚 Learning: 2026-01-20T16:01:39.136Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:826-830
Timestamp: 2026-01-20T16:01:39.136Z
Learning: In the tywrap BridgeProtocol architecture (ADR-002), `HttpIO` (and other Transport implementations like `ProcessIO`, `PyodideIO`) implements only the `Transport` interface (methods: `send`, `isReady`, `dispose`). The `RuntimeExecution` interface methods (`call`, `instantiate`, `callMethod`, `disposeInstance`) are implemented by `BridgeProtocol`, which delegates to `Transport` instances. Transport implementations should not include RuntimeExecution methods.
Applied to files:
docs/guide/runtimes/http.mddocs/plans/2026-03-10-tywrap-promotion.mddocs/guide/runtimes/comparison.md
📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In the tywrap repository, bridges currently use implicit ready detection: the first successful response from the Python subprocess proves the bridge is ready to handle requests.
Applied to files:
docs/guide/runtimes/http.mddocs/reference/env-vars.mddocs/guide/runtimes/comparison.md
📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In `src/runtime/node-bridge.ts` (NodeBridge), a test message is sent to the Python subprocess to confirm the bridge is responsive before marking it as ready.
Applied to files:
docs/guide/runtimes/http.mddocs/plans/2026-03-10-tywrap-promotion.mddocs/reference/env-vars.md
📚 Learning: 2026-01-20T01:33:12.841Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:238-249
Timestamp: 2026-01-20T01:33:12.841Z
Learning: In the tywrap repository, when reviewing init/startup patterns, prioritize cases that break core functionality over edge cases with diagnostic-only impact (e.g., missing bridgeInfo metadata). If workers are healthy and operational, defer low-impact defensive checks to follow-up PRs.
Applied to files:
docs/plans/2026-03-10-tywrap-promotion.md
🪛 GitHub Actions: CI
package.json
[error] 1-1: npm ci failed: package-lock.json is out of sync with package.json. Missing: @types/react@18.3.28 and @types/prop-types@15.7.15 in the lockfile. Run 'npm install' to update the lockfile before retrying 'npm ci'.
🪛 LanguageTool
docs/plans/2026-03-10-tywrap-promotion.md
[uncategorized] ~1250-~1250: The official name of this software platform is spelled with a capital “H”.
Context: ...ep 1: Add the Codecov upload step** In .github/workflows/ci.yml, find the test job....
(GITHUB)
🪛 markdownlint-cli2 (0.21.0)
AGENTS.md
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
CLAUDE.md
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/guide/runtimes/http.md
[warning] 29-29: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 30-30: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 42-42: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 43-43: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/plans/2026-03-10-tywrap-promotion.md
[warning] 53-53: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 55-55: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 57-57: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 62-62: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 64-64: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 66-66: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 71-71: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 76-76: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 86-86: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 88-88: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 92-92: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 107-107: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 112-112: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 119-119: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 124-124: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 126-126: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 130-130: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 143-143: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 149-149: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 154-154: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 156-156: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 158-158: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 177-177: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 182-182: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 237-237: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 293-293: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 294-294: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 294-294: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 302-302: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 317-317: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 322-322: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 329-329: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 331-331: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 335-335: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 443-443: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 507-507: Blank line inside blockquote
(MD028, no-blanks-blockquote)
[warning] 509-509: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 509-509: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 537-537: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 538-538: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 540-540: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 544-544: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 546-546: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 550-550: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 652-652: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 652-652: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 690-690: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 712-712: Multiple headings with the same content
(MD024, no-duplicate-heading)
[warning] 730-730: Multiple headings with the same content
(MD024, no-duplicate-heading)
[warning] 734-734: Multiple headings with the same content
(MD024, no-duplicate-heading)
[warning] 746-746: Multiple headings with the same content
(MD024, no-duplicate-heading)
[warning] 753-753: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 753-753: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 828-828: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 830-830: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 832-832: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 846-846: Multiple headings with the same content
(MD024, no-duplicate-heading)
[warning] 860-860: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 860-860: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 906-906: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 915-915: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 915-915: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 927-927: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 954-954: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 992-992: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1018-1018: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1018-1018: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 1059-1059: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1060-1060: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1064-1064: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1065-1065: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1069-1069: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1070-1070: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1089-1089: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1090-1090: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1090-1090: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 1157-1157: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1232-1232: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1236-1236: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1273-1273: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1283-1283: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1285-1285: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1287-1287: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1289-1289: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1295-1295: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1300-1300: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1377-1377: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1438-1438: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1486-1486: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 1560-1560: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
docs/reference/env-vars.md
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/guide/runtimes/comparison.md
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (28)
.gitignore (1)
50-54: LGTM!The VitePress ignore patterns are correctly added. Note that
.vitepress/on line 52 technically makes lines 53-54 redundant, but keeping the explicitdocs/.vitepress/dist/anddocs/.vitepress/cache/paths adds clarity about what build artifacts are produced.AGENTS.md (2)
1-39: LGTM!The project overview, build commands, and documentation for AI coding assistants are comprehensive and well-structured. This aligns well with the PR's goal of AI agent discoverability.
63-96: LGTM!Code conventions, commit conventions, PR guidelines, and environment variables documentation are clear and complete. The env vars table correctly documents key testing variables like
TYWRAP_PERF_BUDGETSandTYWRAP_CODEC_MAX_BYTES.docs/examples/index.md (1)
74-79: LGTM!The navigation links are correctly updated to use root-relative paths, consistent with the VitePress site structure established across the documentation.
docs/troubleshooting/index.md (1)
647-648: LGTM!The runtime guide links are correctly updated to root-relative paths, maintaining consistency with the VitePress site navigation pattern used throughout the documentation.
.github/labeler.yml (2)
1-7: LGTM!The runtime area labeling configuration correctly targets the runtime-related directories and test files.
33-37: LGTM!The CI area labeling for
.github/**andscripts/**is appropriate for tracking CI/automation changes.docs/guide/runtimes/deno.md (4)
1-11: LGTM!Excellent documentation of the Deno Deploy subprocess limitation. The warning is prominently placed and the alternatives (PyodideBridge, HttpBridge) are clearly presented with links to their respective guides.
32-48: LGTM!The required permissions documentation is thorough, with a clear table explaining why each flag is needed for NodeBridge subprocess execution.
60-77: LGTM!The bridge selection table provides clear guidance for different Deno deployment scenarios, and the troubleshooting section addresses the most common permission errors users will encounter.
22-30: No issues found with Deno npm specifier syntax.The imports
npm:tywrapandnpm:tywrap/runtimecorrectly follow Deno's npm compatibility syntax. The/runtimesubpath is properly exported by the tywrap package and the documentation example is accurate.docs/guide/configuration.md (1)
534-540: LGTM!The navigation links are correctly updated to root-relative paths, maintaining consistency with the VitePress documentation site structure.
.github/workflows/ci.yml (1)
71-79: LGTM!The Codecov integration is well-implemented:
- Running
test:coveragegenerates the lcov report as configured invitest.config.ts- Uploading only for Node 22 + Python 3.11 avoids duplicate coverage reports
fail_ci_if_error: falseis appropriate sinceCODECOV_TOKENis a post-merge setup requirementREADME.md (2)
6-10: LGTM!The new badges are well-structured and follow standard conventions. The Coverage badge links to Codecov, npm downloads provides download metrics, PRs Welcome encourages contributions, and the Docs badge links to the deployed documentation site.
23-32: LGTM!The comparison table effectively differentiates tywrap from alternatives (pythonia, node-calls-python, pymport) across key features. This provides clear value proposition for potential users evaluating the library.
.github/workflows/labeler.yml (1)
1-16: LGTM!The workflow is correctly configured:
- Uses
pull_request_targetappropriately for labeling PRs (including from forks)- Permissions are minimally scoped (
contents: read,pull-requests: write)- Uses the standard
actions/labeler@v6action withGITHUB_TOKENdocs/guide/runtimes/node.md (1)
586-589: LGTM!The navigation links are correctly updated to use root-relative paths, which is consistent with the broader documentation restructuring in this PR. Root-relative paths work well with VitePress and provide more predictable navigation.
docs/public/llms.txt (1)
1-28: LGTM!The
llms.txtfile is well-structured for AI/LLM discoverability. It provides a clear project description and comprehensive navigation to documentation resources. The URLs correctly point to the deployed docs site (bbopen.github.io/tywrap/) and GitHub resources.docs/guide/getting-started.md (1)
346-354: LGTM!The navigation links are correctly updated to use root-relative paths, maintaining consistency with the broader documentation restructuring across the PR.
docs/index.md (1)
1-51: LGTM!Well-structured VitePress homepage with clear hero messaging, feature highlights, and a practical Quick Start guide. The experimental warning and GitHub star CTA are appropriate for project visibility.
docs/guide/runtimes/http.md (1)
1-77: Documentation accurately reflects the API.The options table and code examples correctly match the
HttpBridgeOptionsinterface andregisterArrowDecoderutility from the source code..github/workflows/docs.yml (1)
1-52: LGTM!Well-structured GitHub Pages deployment workflow. Good choices include
fetch-depth: 0for git history access (useful for VitePress'slastUpdatedfeature),cancel-in-progress: falseto avoid disrupting active deployments, and proper artifact handling between build and deploy jobs.docs/.vitepress/config.ts (1)
1-84: LGTM!Comprehensive VitePress configuration with well-organized navigation and sidebar structure. The
base: '/tywrap/'correctly targets GitHub Pages deployment, andsrcExclude: ['**/plans/**']appropriately excludes internal planning documents from the built site.docs/guide/runtimes/bun.md (1)
1-39: LGTM!Documentation accurately reflects the API. Import paths match the package.json exports map, and the options table correctly documents
NodeBridgeOptionsproperties from the source code..github/workflows/release.yml (1)
1-57: LGTM!Well-structured release workflow with proper separation of concerns. The
publish-npmjob correctly:
- Checks out at the release tag
- Uses minimal permissions with
id-token: writefor npm provenance- Runs tests before publishing
- Uses
--provenance --access publicfor supply chain securityThe workflow-level permissions appropriately grant write access for release-please to create PRs and releases.
renovate.json (1)
1-26: LGTM!Well-balanced Renovate configuration with a conservative automerge strategy. Automerging is limited to low-risk updates (devDependencies patch/minor and type definitions) while production dependencies require manual review. The weekly schedule and PR limits prevent notification overload.
.github/workflows/stale.yml (1)
1-39: LGTM!Reasonable stale management configuration with appropriate timeframes and exemptions. The longer grace period for issues (90+30 days) vs PRs (60+14 days) reflects that issues may await upstream changes while PRs indicate active work. Exempting
security,pinned,good first issue, andhelp wantedlabels preserves important items.package.json (1)
141-142: Lockfile is already up-to-date with the VitePress dependency.The
package-lock.jsonalready includesvitepressand its transitive dependencies (@types/react).@types/prop-typesis not a declared dependency of VitePress, so its absence is not a sync issue. Bothpackage.jsonandpackage-lock.jsonare committed and in sync.> Likely an incorrect or invalid review comment.
| area:docs: | ||
| - changed-files: | ||
| - any-glob-to-any-file: | ||
| - 'docs/**' | ||
| - '*.md' | ||
| - 'CONTRIBUTING.md' | ||
| - 'AGENTS.md' | ||
| - 'CLAUDE.md' |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor redundancy in docs patterns.
Lines 29-31 (CONTRIBUTING.md, AGENTS.md, CLAUDE.md) are already covered by the *.md glob pattern on line 28. These explicit entries are redundant but harmless—they may serve as documentation of important files.
♻️ Optional: Remove redundant entries
area:docs:
- changed-files:
- any-glob-to-any-file:
- 'docs/**'
- '*.md'
- - 'CONTRIBUTING.md'
- - 'AGENTS.md'
- - 'CLAUDE.md'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| area:docs: | |
| - changed-files: | |
| - any-glob-to-any-file: | |
| - 'docs/**' | |
| - '*.md' | |
| - 'CONTRIBUTING.md' | |
| - 'AGENTS.md' | |
| - 'CLAUDE.md' | |
| area:docs: | |
| - changed-files: | |
| - any-glob-to-any-file: | |
| - 'docs/**' | |
| - '*.md' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/labeler.yml around lines 24 - 31, The labeler config under the
"area:docs" rule contains redundant explicit file entries ('CONTRIBUTING.md',
'AGENTS.md', 'CLAUDE.md') because the '*.md' glob already matches them; update
the "any-glob-to-any-file" list in the area:docs rule to remove those explicit
filenames (leave '*.md' and 'docs/**' intact) so the rule is not redundant—look
for the "area:docs" block and the "any-glob-to-any-file" list to make this
change.
| ``` | ||
| src/ | ||
| cli.ts # CLI entry point (tywrap init / generate) | ||
| runtime/ | ||
| node.ts # NodeBridge — subprocess-based (Node, Bun, Deno) | ||
| pyodide.ts # PyodideBridge — browser WebAssembly | ||
| http.ts # HttpBridge — remote Python server | ||
| utils/ | ||
| runtime.ts # detectRuntime(), isBun(), isDeno(), etc. | ||
| tywrap_ir/ # Python package: AST analysis → typed IR | ||
| tywrap_ir/ | ||
| __main__.py # CLI entry: tywrap-ir | ||
| runtime/ | ||
| python_bridge.py # Python subprocess server (JSONL protocol) | ||
| test/ # Vitest tests mirroring src/ structure | ||
| test-d/ # TSD type tests | ||
| docs/ # Documentation (VitePress site) | ||
| examples/ | ||
| living-app/ # End-to-end example with pandas + Arrow | ||
| generated/ # Generated wrapper output (gitignored in user projects) | ||
| dist/ # Compiled TypeScript output (do not edit) | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced code block.
The repository structure code block is missing a language identifier, which triggers a markdown lint warning (MD040). Use text or plaintext for directory tree structures.
🔧 Proposed fix
-```
+```text
src/
cli.ts # CLI entry point (tywrap init / generate)🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 40 - 61, The markdown fenced code block containing
the repository tree is missing a language identifier and triggers MD040; update
the opening fence to include a language such as "text" or "plaintext" (i.e.,
change the triple-backtick that starts the repo tree to ```text or ```plaintext)
so the directory listing is treated as plain text and the linter warning is
resolved.
| ``` | ||
| src/ | ||
| cli.ts # CLI entry point (tywrap init / generate) | ||
| runtime/ | ||
| node.ts # NodeBridge — subprocess-based (Node, Bun, Deno) | ||
| pyodide.ts # PyodideBridge — browser WebAssembly | ||
| http.ts # HttpBridge — remote Python server | ||
| utils/ | ||
| runtime.ts # detectRuntime(), isBun(), isDeno(), etc. | ||
| tywrap_ir/ # Python package: AST analysis → typed IR | ||
| tywrap_ir/ | ||
| __main__.py # CLI entry: tywrap-ir | ||
| runtime/ | ||
| python_bridge.py # Python subprocess server (JSONL protocol) | ||
| test/ # Vitest tests mirroring src/ structure | ||
| test-d/ # TSD type tests | ||
| docs/ # Documentation (VitePress site) | ||
| examples/ | ||
| living-app/ # End-to-end example with pandas + Arrow | ||
| generated/ # Generated wrapper output (gitignored in user projects) | ||
| dist/ # Compiled TypeScript output (do not edit) | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the repository tree fence.
This block is already tripping markdownlint (MD040). Mark it as text so the new docs file stays lint-clean.
Suggested fix
-```
+```text
src/
cli.ts # CLI entry point (tywrap init / generate)
runtime/
node.ts # NodeBridge — subprocess-based (Node, Bun, Deno)
@@
dist/ # Compiled TypeScript output (do not edit)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 40 - 61, The fenced repository tree in CLAUDE.md is
missing a language tag and triggers markdownlint MD040; update the opening fence
from ``` to ```text so the block is treated as plain text (the tree showing
src/, tywrap_ir/, runtime/, test/, docs/, examples/, generated/, dist/), i.e.,
change the code fence that wraps the repo tree to use the "text" language tag.
| ## bunfig.toml | ||
|
|
||
| tywrap ships with a `bunfig.toml` for build and run settings: | ||
|
|
||
| ```toml | ||
| [build] | ||
| target = "bun" | ||
| format = "esm" | ||
| splitting = true | ||
| sourcemap = "external" | ||
| external = ["pyodide"] | ||
|
|
||
| [dev] | ||
| hot = true | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if bunfig.toml exists in the repository
fd -t f 'bunfig.toml'
# Check if it would be included in the npm package (not in .npmignore or excluded)
if [ -f ".npmignore" ]; then
echo "=== .npmignore contents ==="
cat .npmignore
fi
if [ -f "package.json" ]; then
echo "=== package.json files field ==="
jq '.files // "not specified"' package.json
fiRepository: bbopen/tywrap
Length of output: 721
Update documentation to clarify bunfig.toml is not distributed with the package.
The statement "tywrap ships with a bunfig.toml" is inaccurate. The file is explicitly excluded in .npmignore and not included in the package.json files field. Users will not find this file in their installed tywrap package. Reword this as an example configuration they should create in their own projects when using Bun, not a file that comes with tywrap.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guide/runtimes/bun.md` around lines 40 - 54, Update the text that
currently says "tywrap ships with a `bunfig.toml`" to clearly state that the
shown `bunfig.toml` is an example configuration users should add to their own
projects when using Bun, because the package does not include `bunfig.toml` (it
is excluded via .npmignore / not listed in package.json `files`). Replace the
phrasing around the `bunfig.toml` header and the surrounding paragraph to:
present the TOML block as a sample `bunfig.toml` to create in your project, not
a file distributed by tywrap, and keep the example content exactly as shown
(`[build]` and `[dev]` entries).
| headers: { | ||
| 'Authorization': 'Bearer your-token', | ||
| }, | ||
| })); | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add blank line before code block for markdown compliance.
The static analysis tool flagged missing blank lines around headings and fenced code blocks.
📝 Proposed fix
## Configuration Options
+
`NodeBridge` accepts the same options under Bun as under Node.js. See the [Node.js guide](./node) for the full option reference.🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 29-29: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 30-30: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guide/runtimes/http.md` around lines 27 - 31, Add a blank line before
the fenced code block that starts with ``` (the snippet showing headers: {
'Authorization': 'Bearer your-token', }), to satisfy Markdown rules; edit
docs/guide/runtimes/http.md so there is an empty line separating the preceding
heading/paragraph and the code fence.
| | `codec` | No | Arrow | Codec options | | ||
|
|
||
| ## Running the Python Server | ||
|
|
||
| Start the Python bridge server: | ||
|
|
||
| ```bash | ||
| pip install tywrap-ir | ||
| python -m tywrap_ir.server --port 8080 | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add blank line before code block for markdown compliance.
Per the markdownlint warnings (MD022, MD031), headings and fenced code blocks should be surrounded by blank lines.
📝 Proposed fix
## Running the Python Server
+
Start the Python bridge server:🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 42-42: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 43-43: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guide/runtimes/http.md` around lines 40 - 49, Add a blank line between
the "Running the Python Server" heading and the following fenced code block so
the markdown is compliant with MD022/MD031; locate the heading "Running the
Python Server" and ensure there is an empty line before the triple-backtick
block that starts with "```bash" (the pip install / python -m tywrap_ir.server
snippet).
| - uses: actions/stale@v9 | ||
| with: | ||
| repo-token: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read line 9
sed -n '9p' docs/plans/2026-03-10-tywrap-promotion.md
# Then read lines 1407-1409
sed -n '1407,1409p' docs/plans/2026-03-10-tywrap-promotion.mdRepository: bbopen/tywrap
Length of output: 296
Use the same actions/stale major version throughout the plan.
Line 9 declares actions/stale@v10 in the Tech Stack, but this workflow snippet at lines 1407-1409 still pins actions/stale@v9. This drift makes the plan internally inconsistent and easy to implement incorrectly.
Suggested fix
- - uses: actions/stale@v9
+ - uses: actions/stale@v10🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-10-tywrap-promotion.md` around lines 1407 - 1409, The plan
uses two different major versions of the same GitHub Action: update the workflow
snippet that currently references actions/stale@v9 so it matches the Tech Stack
declaration (actions/stale@v10); find the usage of the actions/stale action in
the document (look for the token "actions/stale" in the snippet) and change the
pinned major version to `@v10` so the plan is internally consistent.
|
|
||
| ## Subprocess Environment Inheritance | ||
|
|
||
| By default, `NodeBridge` inherits only `PATH`, `PYTHON*`, and `TYWRAP_*` from `process.env` to keep the subprocess environment minimal. To pass the full environment: |
There was a problem hiding this comment.
Inaccurate description of subprocess environment inheritance.
The documentation states that PYTHON* variables are inherited, but the code (src/runtime/node.ts:638-662) shows a more specific allowlist:
- Prefixes allowed:
TYWRAP_ - Specific keys allowed:
path,pythonpath,virtual_env,pythonhome
Variables like PYTHONUNBUFFERED or PYTHONDONTWRITEBYTECODE would NOT be inherited unless inheritProcessEnv: true is set.
📝 Suggested fix
-By default, `NodeBridge` inherits only `PATH`, `PYTHON*`, and `TYWRAP_*` from `process.env` to keep the subprocess environment minimal. To pass the full environment:
+By default, `NodeBridge` inherits only `PATH`, `PYTHONPATH`, `VIRTUAL_ENV`, `PYTHONHOME`, and `TYWRAP_*` from `process.env` to keep the subprocess environment minimal. To pass the full environment:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| By default, `NodeBridge` inherits only `PATH`, `PYTHON*`, and `TYWRAP_*` from `process.env` to keep the subprocess environment minimal. To pass the full environment: | |
| By default, `NodeBridge` inherits only `PATH`, `PYTHONPATH`, `VIRTUAL_ENV`, `PYTHONHOME`, and `TYWRAP_*` from `process.env` to keep the subprocess environment minimal. To pass the full environment: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/reference/env-vars.md` at line 54, Update the documentation for
NodeBridge's subprocess environment inheritance to match the actual allowlist
used by the implementation: state that by default NodeBridge inherits PATH and
any variables with the TYWRAP_ prefix, plus the specific keys path, pythonpath,
virtual_env, and pythonhome; explicitly note that generic PYTHON* variables
(e.g., PYTHONUNBUFFERED, PYTHONDONTWRITEBYTECODE) are NOT inherited unless
inheritProcessEnv: true is set, and ensure the doc text references the
NodeBridge option inheritProcessEnv and the exact allowed prefixes/keys.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7c415bebc
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "peerDependencies": { | ||
| "@types/react": ">= 16.8.0 < 19.0.0", | ||
| "react": ">= 16.8.0 < 19.0.0", |
There was a problem hiding this comment.
This new @docsearch/react peer range (@types/react <19) conflicts with the lockfile state, which only resolves @types/react@19.1.9, and npm ci now aborts with Missing: @types/react@ from lock file. I reproduced this on the checked-out commit with Node 22.21.1 / npm 11.4.2 (while the parent commit installs cleanly), so any workflow that starts with npm ci can fail before tests or docs build run until the lockfile is regenerated with a compatible dependency graph.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/dependabot.yml:
- Around line 24-34: Add a "groups" block to the dependabot entry for
package-ecosystem: "pip" and directory: "/tywrap_ir" so pip updates are batched;
e.g., create a group name (like "tywrap_ir-deps") and set its "patterns" to
match your Python dependency files (e.g., "requirements.txt", "pyproject.toml",
"*.in" or a catch-all pattern) so multiple dependency updates in /tywrap_ir are
combined into a single PR instead of separate ones.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cea9021f-cd97-4eab-b9bf-5dbc830fa475
📒 Files selected for processing (1)
.github/dependabot.yml
📜 Review details
🔇 Additional comments (2)
.github/dependabot.yml (2)
1-22: Well-structured npm dependency grouping.The configuration correctly groups
@types/*packages and development dependencies (minor/patch only) to reduce PR noise while keeping production dependencies and major dev updates in separate PRs for careful review.Note:
@types/*packages typically are dev dependencies, but they'll match thetypescript-typesgroup first due to Dependabot's first-match behavior, which is the intended outcome here.
36-50: GitHub Actions grouping looks good.Grouping all GitHub Actions updates with
"*"pattern reduces PR noise effectively. Since Actions are typically infrastructure-level and updated together, this is a sensible approach.
| # Python dependencies | ||
| - package-ecosystem: "pip" | ||
| directory: "/tywrap_ir" | ||
| schedule: | ||
| interval: "weekly" | ||
| day: "monday" | ||
| time: "06:00" | ||
| timezone: "America/New_York" | ||
| open-pull-requests-limit: 3 | ||
| labels: | ||
| - "area:ci" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding grouping for pip dependencies.
The pip ecosystem configuration is functional but lacks grouping, unlike the other ecosystems. If the /tywrap_ir project has multiple dependencies, each update will create a separate PR.
♻️ Optional: Add grouping similar to other ecosystems
- package-ecosystem: "pip"
directory: "/tywrap_ir"
schedule:
interval: "weekly"
day: "monday"
time: "06:00"
timezone: "America/New_York"
+ groups:
+ python-dependencies:
+ patterns:
+ - "*"
+ update-types:
+ - "minor"
+ - "patch"
open-pull-requests-limit: 3
labels:
- "area:ci"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Python dependencies | |
| - package-ecosystem: "pip" | |
| directory: "/tywrap_ir" | |
| schedule: | |
| interval: "weekly" | |
| day: "monday" | |
| time: "06:00" | |
| timezone: "America/New_York" | |
| open-pull-requests-limit: 3 | |
| labels: | |
| - "area:ci" | |
| # Python dependencies | |
| - package-ecosystem: "pip" | |
| directory: "/tywrap_ir" | |
| schedule: | |
| interval: "weekly" | |
| day: "monday" | |
| time: "06:00" | |
| timezone: "America/New_York" | |
| groups: | |
| python-dependencies: | |
| patterns: | |
| - "*" | |
| update-types: | |
| - "minor" | |
| - "patch" | |
| open-pull-requests-limit: 3 | |
| labels: | |
| - "area:ci" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/dependabot.yml around lines 24 - 34, Add a "groups" block to the
dependabot entry for package-ecosystem: "pip" and directory: "/tywrap_ir" so pip
updates are batched; e.g., create a group name (like "tywrap_ir-deps") and set
its "patterns" to match your Python dependency files (e.g., "requirements.txt",
"pyproject.toml", "*.in" or a catch-all pattern) so multiple dependency updates
in /tywrap_ir are combined into a single PR instead of separate ones.
@docsearch/react (via VitePress) requires @types/react <19.0.0 while bun-types requires ^19. npm 10.x (Node 22 in CI) fails npm ci because the lock file generated by npm 11.x resolved to 19.x, which doesn't satisfy the <19.0.0 constraint. Add overrides to pin to ^18.3.0 so all CI environments resolve consistently to 18.3.28.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
Critical fixes: - Fix 4 broken README doc links (point to deployed docs site) - Fix CLAUDE.md heading (was copy-pasted as "# AGENTS.md") - Remove non-existent `tywrap_ir.server` reference from HTTP guide - Remove deprecated `enableJsonFallback`/`maxLineLength` from Node guide - Add API reference content to llms-full.txt and llms.txt index - Delete publish.yml to prevent double-publish with release.yml Important fixes: - Fix Bun guide import path (tywrap/node works on Bun, recommend it) - Expand AGENTS.md/CLAUDE.md repo structure (core/, config/, types/, etc.) - Add missing TYWRAP_REQUEST_MAX_BYTES env var to agent files - Remove overly broad `.vitepress/` gitignore rule - Only run test:coverage on the matrix cell that uploads to Codecov - Exclude orphaned release.md and codec-roadmap.md from VitePress build - Fix dead CHANGELOG.md link (point to GitHub releases page instead)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
AGENTS.md (1)
40-83:⚠️ Potential issue | 🟡 MinorAdd a language tag to the repository tree code block.
Line 40 uses a plain fenced block; this will keep triggering markdownlint MD040. Use a text language tag.
Suggested fix
-``` +```text src/ index.ts # Public API re-exports @@ dist/ # Compiled TypeScript output (do not edit)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@AGENTS.mdaround lines 40 - 83, The repository tree code block is missing a
language tag which triggers markdownlint MD040; update the fenced block opening
fromtotext so the tree is treated as plain text (the block starting
with the lines "src/" and ending with "dist/ # Compiled
TypeScript output (do not edit)") to silence the lint warning.</details> </blockquote></details> <details> <summary>CLAUDE.md (1)</summary><blockquote> `40-83`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language identifier to the repository tree fence.** Line 40 opens the tree block without a language tag; this is a markdownlint MD040 warning source. <details> <summary>Suggested fix</summary> ```diff -``` +```text src/ index.ts # Public API re-exports @@ dist/ # Compiled TypeScript output (do not edit) ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@CLAUDE.mdaround lines 40 - 83, Update the fenced repository tree block so
the opening fence includes a language identifier (e.g., change the openingtotext) for the block that lists src/, tywrap_ir/, runtime/, test/,
examples/, generated/, and dist/ (the tree starting with "src/" and ending with
"dist/"); modify the opening fence only so markdownlint MD040 is satisfied and
leave the block contents unchanged.</details> </blockquote></details> <details> <summary>docs/guide/runtimes/comparison.md (1)</summary><blockquote> `28-37`: _🧹 Nitpick_ | _🔵 Trivial_ **Add language identifier to fenced code block.** The decision guide diagram uses a plain code fence without a language specifier. Adding `text` improves accessibility and satisfies markdown linting rules. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text Do you need subprocess-based Python execution? ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/guide/runtimes/comparison.mdaround lines 28 - 37, Add the language
identifier "text" to the fenced code block that begins with "Do you need
subprocess-based Python execution?" in the comparison decision diagram so the
fence reads ```text, ensuring markdown linters and accessibility tools recognize
it as plain text; update the opening backticks only and leave the block content
unchanged.</details> </blockquote></details> <details> <summary>docs/guide/runtimes/bun.md (1)</summary><blockquote> `40-54`: _⚠️ Potential issue_ | _🟡 Minor_ **Update documentation to clarify `bunfig.toml` is not distributed with the package.** Line 42 states "tywrap ships with a `bunfig.toml`" but this file is excluded from the npm package (not in `package.json` `files` field and listed in `.npmignore`). Users will not find this file when they install tywrap. Reword this as an example configuration users should create in their own projects. <details> <summary>📝 Suggested fix</summary> ```diff ## bunfig.toml -tywrap ships with a `bunfig.toml` for build and run settings: +Add a `bunfig.toml` to your project root for optimal Bun build and run settings: ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/guide/runtimes/bun.md` around lines 40 - 54, The docs currently say "tywrap ships with a `bunfig.toml`" under the "## bunfig.toml" section which is incorrect because `bunfig.toml` is not included in the published package; change the wording to present the snippet as an example configuration users should add to their own projects (e.g., "Example bunfig.toml to add to your project") and optionally note that `bunfig.toml` is excluded from the npm package so users must create it themselves. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@docs/guide/runtimes/http.md:
- Around line 35-40: The table currently lists the default for codec as "Arrow"
which is incorrect; update the options table so that thecodecrow reflects
that HttpBridgeOptions.codec is an optional options object (no default transport
mode) — e.g., set the Default column to "—" or "optional" and change the
Description to something like "Optional codec options (not a transport-mode
default)" so readers knowHttpBridgeOptions.codecis an optional configuration
object rather than a default codec.In
@README.md:
- Around line 152-155: Replace the remaining local docs link in README.md that
points to "./docs/configuration.md" (the stale internal docs reference) with the
hosted configuration URL "https://bbopen.github.io/tywrap/guide/configuration"
so all documentation links are consistent with the migrated docs site.
Duplicate comments:
In@AGENTS.md:
- Around line 40-83: The repository tree code block is missing a language tag
which triggers markdownlint MD040; update the fenced block opening from ``` to"src/" and ending with "dist/ # Compiled TypeScript output (do not edit)") to silence the lint warning. In `@CLAUDE.md`: - Around line 40-83: Update the fenced repository tree block so the opening fence includes a language identifier (e.g., change the opening ``` to ```text) for the block that lists src/, tywrap_ir/, runtime/, test/, examples/, generated/, and dist/ (the tree starting with "src/" and ending with "dist/"); modify the opening fence only so markdownlint MD040 is satisfied and leave the block contents unchanged. In `@docs/guide/runtimes/bun.md`: - Around line 40-54: The docs currently say "tywrap ships with a `bunfig.toml`" under the "## bunfig.toml" section which is incorrect because `bunfig.toml` is not included in the published package; change the wording to present the snippet as an example configuration users should add to their own projects (e.g., "Example bunfig.toml to add to your project") and optionally note that `bunfig.toml` is excluded from the npm package so users must create it themselves. In `@docs/guide/runtimes/comparison.md`: - Around line 28-37: Add the language identifier "text" to the fenced code block that begins with "Do you need subprocess-based Python execution?" in the comparison decision diagram so the fence reads ```text, ensuring markdown linters and accessibility tools recognize it as plain text; update the opening backticks only and leave the block content unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID:
1aa4d295-3253-491d-ab5a-ad90f8df49e9⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json📒 Files selected for processing (14)
.github/workflows/ci.yml.github/workflows/publish.yml.gitignoreAGENTS.mdCLAUDE.mdREADME.mddocs/.vitepress/config.tsdocs/guide/runtimes/bun.mddocs/guide/runtimes/comparison.mddocs/guide/runtimes/http.mddocs/guide/runtimes/node.mddocs/public/llms-full.txtdocs/public/llms.txtpackage.json💤 Files with no reviewable changes (1)
- .github/workflows/publish.yml
📜 Review details
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2026-01-19T21:14:40.872Z
Learnt from: bbopen Repo: bbopen/tywrap PR: 127 File: src/runtime/bridge-core.ts:260-263 Timestamp: 2026-01-19T21:14:40.872Z Learning: In `src/runtime/bridge-core.ts` and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (e.g., runtime shape checks on error payloads) in tight loops unless the protocol boundary is untrusted or there's a concrete bug report. The Python bridge protocol is controlled and validated via tests, so defensive checks would add unnecessary branching overhead without meaningful benefit.Applied to files:
docs/guide/runtimes/comparison.mdAGENTS.mddocs/guide/runtimes/http.md📚 Learning: 2026-01-20T16:01:39.136Z
Learnt from: bbopen Repo: bbopen/tywrap PR: 152 File: docs/adr/002-bridge-protocol.md:826-830 Timestamp: 2026-01-20T16:01:39.136Z Learning: In the tywrap BridgeProtocol architecture (ADR-002), `HttpIO` (and other Transport implementations like `ProcessIO`, `PyodideIO`) implements only the `Transport` interface (methods: `send`, `isReady`, `dispose`). The `RuntimeExecution` interface methods (`call`, `instantiate`, `callMethod`, `disposeInstance`) are implemented by `BridgeProtocol`, which delegates to `Transport` instances. Transport implementations should not include RuntimeExecution methods.Applied to files:
docs/guide/runtimes/comparison.mddocs/guide/runtimes/http.md📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen Repo: bbopen/tywrap PR: 152 File: docs/adr/002-bridge-protocol.md:599-602 Timestamp: 2026-01-20T16:01:14.323Z Learning: In the tywrap repository, bridges currently use implicit ready detection: the first successful response from the Python subprocess proves the bridge is ready to handle requests.Applied to files:
docs/guide/runtimes/comparison.mddocs/guide/runtimes/http.md📚 Learning: 2026-01-19T21:14:37.032Z
Learnt from: bbopen Repo: bbopen/tywrap PR: 127 File: src/runtime/bridge-core.ts:375-385 Timestamp: 2026-01-19T21:14:37.032Z Learning: In tywrap (src/runtime/bridge-core.ts and similar), environment variable parsing follows a tolerant/best-effort policy. For example, `TYWRAP_CODEC_MAX_BYTES=1024abc` should be accepted as 1024. Only reject clearly invalid values (non-numeric start or <=0). This avoids surprising failures from minor typos.Applied to files:
docs/guide/runtimes/node.md📚 Learning: 2026-01-19T21:48:45.693Z
Learnt from: bbopen Repo: bbopen/tywrap PR: 127 File: src/runtime/bridge-core.ts:260-298 Timestamp: 2026-01-19T21:48:45.693Z Learning: In `src/runtime/bridge-core.ts`, keep `normalizeErrorPayload` to validate error payloads from the Python subprocess. The subprocess boundary is effectively untrusted, and normalizing error responses prevents `undefined: undefined` errors on malformed payloads. Error responses are not the hot path, so the small validation overhead is acceptable for the added resilience.Applied to files:
docs/guide/runtimes/node.md📚 Learning: 2026-01-20T16:00:49.738Z
Learnt from: bbopen Repo: bbopen/tywrap PR: 152 File: docs/adr/002-bridge-protocol.md:168-172 Timestamp: 2026-01-20T16:00:49.738Z Learning: In the tywrap project's BridgeProtocol SafeCodec implementation, Arrow format decoders can produce NaN/Infinity values from binary representations even when the raw JSON payload doesn't contain them. This is why validation for special floats must occur both before encoding (to reject invalid inputs) and after applying decoders (to catch values introduced during Arrow deserialization), protecting downstream consumers from unexpected NaN/Infinity values.Applied to files:
docs/guide/runtimes/node.md📚 Learning: 2026-01-19T21:49:05.612Z
Learnt from: bbopen Repo: bbopen/tywrap PR: 127 File: runtime/python_bridge.py:99-123 Timestamp: 2026-01-19T21:49:05.612Z Learning: In the tywrap repository, TYWRAP_REQUEST_MAX_BYTES uses strict integer parsing that rejects values with trailing characters (e.g., "1024abc"). This differs from TYWRAP_CODEC_MAX_BYTES, which uses tolerant/best-effort parsing that accepts numeric prefixes. The strict policy for REQUEST_MAX_BYTES ensures explicit integer values and consistent parse behavior across Node/Python implementations.Applied to files:
docs/guide/runtimes/node.mdREADME.md📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen Repo: bbopen/tywrap PR: 152 File: docs/adr/002-bridge-protocol.md:599-602 Timestamp: 2026-01-20T16:01:14.323Z Learning: In `src/runtime/node-bridge.ts` (NodeBridge), a test message is sent to the Python subprocess to confirm the bridge is responsive before marking it as ready.Applied to files:
docs/guide/runtimes/http.md🪛 markdownlint-cli2 (0.21.0)
docs/guide/runtimes/http.md
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (15)
.github/workflows/ci.yml (1)
71-79: LGTM! Coverage upload is correctly restricted to a single matrix cell.The conditional logic correctly ensures that:
- Only the Node 22 + Python 3.11 matrix cell runs
test:coverage(avoiding redundant coverage collection across 6 cells)- The Codecov upload step only runs for that same cell
fail_ci_if_error: falseis appropriate sinceCODECOV_TOKENis a post-merge setup item per the PR objectivesThe
./coverage/lcov.infopath aligns with the vitest configuration (v8 provider withlcovreporter outputs to the default./coveragedirectory). Thecodecov/codecov-action@v5reference will pull the latest v5.x patches..gitignore (1)
51-53: Good ignore coverage for docs build artifacts.Ignoring VitePress
distandcacheoutputs is correct and prevents generated-file churn.docs/public/llms.txt (1)
10-29: LLMS index content and link taxonomy look solid.The guide/reference/optional grouping is clear and matches the new docs IA.
docs/guide/runtimes/node.md (1)
114-114: These doc updates are consistent with current runtime configuration direction.
codecguidance and root-relative docs links align well with the new docs structure.Also applies to: 210-210, 571-574
docs/.vitepress/config.ts (1)
3-84: VitePress site configuration looks well-structured.Navigation, sidebar taxonomy, and edit-link wiring are coherent for the new docs site rollout.
docs/guide/runtimes/bun.md (2)
1-39: LGTM!The installation, setup, configuration options, and import path guidance are accurate and consistent with the package.json exports and the
setRuntimeBridgeAPI fromsrc/runtime/index.ts.
56-87: LGTM!The test commands, virtual environment configuration, environment variable references, and troubleshooting guidance are clear and helpful.
docs/guide/runtimes/comparison.md (2)
1-24: LGTM!The feature matrix accurately captures runtime capabilities, and the import paths are consistent with the package.json exports (
tywrap/node,tywrap/pyodide,tywrap/http).
39-45: LGTM!The bridge reference table provides a clear mapping between bridges, exports, and their respective guides.
package.json (6)
4-4: LGTM!The updated description accurately reflects the multi-runtime support (Node.js, Deno, Bun, browsers via Pyodide).
74-77: LGTM!Standard VitePress scripts for documentation development, building, and previewing.
90-100: LGTM!The expanded keywords improve npm discoverability and accurately reflect the project's capabilities (codegen, ffi, bindings, subprocess, numpy/pandas/scipy support).
111-111: LGTM!Homepage correctly points to the new VitePress docs site that will be deployed via GitHub Pages.
156-159: LGTM!The
overridesfor@types/reactand@types/prop-typesresolve peer dependency conflicts between VitePress/@docsearch and bun-types, ensuring consistent type resolution across CI environments as noted in the commit message.
142-142: Vitepress version 1.6.4 is valid. The specified version is available on npm and requires no action.
| | Option | Required | Default | Description | | ||
| |--------|----------|---------|-------------| | ||
| | `baseURL` | Yes | — | Base URL of the Python bridge server | | ||
| | `headers` | No | `{}` | Additional HTTP headers (auth, CORS, etc.) | | ||
| | `timeoutMs` | No | `30000` | Request timeout in milliseconds | | ||
| | `codec` | No | Arrow | Codec options | |
There was a problem hiding this comment.
Correct the codec default value in the options table.
Line 40 currently says the default is Arrow, but HttpBridgeOptions.codec is an optional options object, not a transport-mode default. This can mislead configuration.
Suggested fix
-| `codec` | No | Arrow | Codec options |
+| `codec` | No | — | Codec options (validation/serialization settings) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guide/runtimes/http.md` around lines 35 - 40, The table currently lists
the default for codec as "Arrow" which is incorrect; update the options table so
that the `codec` row reflects that HttpBridgeOptions.codec is an optional
options object (no default transport mode) — e.g., set the Default column to "—"
or "optional" and change the Description to something like "Optional codec
options (not a transport-mode default)" so readers know
`HttpBridgeOptions.codec` is an optional configuration object rather than a
default codec.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/node.ts (1)
330-353:⚠️ Potential issue | 🟠 MajorRun the readiness probe for initial workers too.
Initial workers still go through
onWorkerReady: userWarmup, so thebuiltins.len([])probe only executes on background replacements. That leaves first-spawned workers unprobed whenwarmupCommandsis empty, and runs user warmups before the readiness check when it is not. Use the readiness wrapper for both startup and replacement workers.Based on learnings, in `src/runtime/node-bridge.ts` (NodeBridge), a test message is sent to the Python subprocess to confirm the bridge is responsive before marking it as ready.Suggested wiring
- const replacementWorkerReady = createWorkerReadyCallback( + const workerReady = createWorkerReadyCallback( resolvedOptions.timeoutMs, userWarmup );- onWorkerReady: userWarmup, - onReplacementWorkerReady: replacementWorkerReady, + onWorkerReady: workerReady, + onReplacementWorkerReady: workerReady,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/node.ts` around lines 330 - 353, The initial workers are currently using onWorkerReady: userWarmup so the readiness probe from createWorkerReadyCallback only runs for replacements; change wiring so both initial and replacement workers use the readiness wrapper: pass the userWarmup into createWorkerReadyCallback (already done) and set onWorkerReady: replacementWorkerReady (instead of userWarmup) and keep onReplacementWorkerReady: replacementWorkerReady on the PooledTransport; ensure createWorkerReadyCallback(resolvedOptions.timeoutMs, userWarmup) remains the source of truth for readiness logic so both startup and replacement workers run the bridge responsiveness probe before being marked ready.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reference/api/index.md`:
- Around line 247-262: The docs incorrectly show the input API as TywrapOptions
(fully resolved shape) — update the Key Types block to document the authoring
input type TywrapConfig instead of TywrapOptions, and ensure the text/alias
references match the aliases exported in src/config/index.ts; mention that
fields like runtime, performance, and development are optional when calling
defineConfig() or generate() so the displayed type reflects those optional
properties (TywrapConfig) rather than the fully resolved TywrapOptions.
In `@scripts/generate-llms-full.mjs`:
- Around line 78-86: Wrap the file I/O in a try/catch around the loop and final
write so readFile and writeFile errors are handled: iterate over orderedDocs
inside a try block, catch and log a descriptive error (including the filename
and the caught error) when a readFile fails while building sections, and
similarly catch and log/write a thrown error when writing the final output
(output/header/sections). Ensure the catch rethrows or exits appropriately for
your script (so failures are visible), and reference the variables/functions
sections, orderedDocs, readFile, writeFile, header, and output when adding the
error handling.
In `@src/core/generator.ts`:
- Around line 983-986: The current fallback that returns the literal 'unknown'
when a custom type name fails the TS identifier regex should emit a diagnostic
instead of failing silently: in the function that performs this normalization
(the block that tests /^[A-Za-z_$][A-Za-z0-9_$]*$/ and returns c.name or
'unknown'), call the diagnostics hook (e.g., onUnknownTypeName) with the
original candidate name and the fallback value, then return the fallback;
include context like the raw input and the normalized/leaf name so users can see
what failed. Locate this logic in generator.ts (the identifier-validation
branch) and wire in the same diagnostics hook used by annotation-parser
(onUnknownTypeName) or the existing diagnostics mechanism so warnings are
produced when the fallback is used.
In `@src/runtime/node.ts`:
- Around line 634-659: In executeWorkerCall, detect and reject any Map or Set
inside the warmup arguments before building the JSON envelope: inspect
request.args (and any nested values) and throw a BridgeExecutionError using the
provided messages.label and a clear message like "Bridge protocol does not
support Map/Set values" (preserving the error cause if available) rather than
letting JSON.stringify collapse them; update the code path around
generateWarmupId()/JSON.stringify to run this validation and fail fast so the
bridge protocol never receives Map/Set values.
In `@test/integration.test.ts`:
- Around line 100-122: The test currently asserts expect(compile.code).toBe(0)
but when TypeScript fails (code 2) the test gives no diagnostics; update the
test in test/integration.test.ts so that after calling processUtils.exec (the
compile variable returned for generatedPath) any non-zero compile.code surfaces
the compiler output—e.g. change the assertions to include the diagnostics from
compile.stdout/compile.stderr in the failure message or add an expect that
checks compile.stderr/compile.stdout is empty while including them in the
assertion message; additionally, if the failure is due to generated advanced
types, fix the type emission in the generator producing advanced_types (the code
that writes generatedPath) so emitted types are valid for ES2022/DOM targets, or
at minimum include compile.stdout/compile.stderr in the test failure to reveal
which generated type is invalid.
---
Outside diff comments:
In `@src/runtime/node.ts`:
- Around line 330-353: The initial workers are currently using onWorkerReady:
userWarmup so the readiness probe from createWorkerReadyCallback only runs for
replacements; change wiring so both initial and replacement workers use the
readiness wrapper: pass the userWarmup into createWorkerReadyCallback (already
done) and set onWorkerReady: replacementWorkerReady (instead of userWarmup) and
keep onReplacementWorkerReady: replacementWorkerReady on the PooledTransport;
ensure createWorkerReadyCallback(resolvedOptions.timeoutMs, userWarmup) remains
the source of truth for readiness logic so both startup and replacement workers
run the bridge responsiveness probe before being marked ready.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 148edac9-7f6f-4e18-9359-f7e1dd13c043
📒 Files selected for processing (28)
.github/labeler.ymlREADME.mddocs/guide/configuration.mddocs/guide/getting-started.mddocs/guide/runtimes/node.mddocs/public/llms-full.txtdocs/public/llms.txtdocs/reference/api/index.mddocs/reference/cli.mddocs/reference/env-vars.mddocs/reference/type-mapping.mdpackage.jsonscripts/generate-llms-full.mjssrc/config/index.tssrc/core/annotation-parser.tssrc/core/generator.tssrc/core/mapper.tssrc/index.tssrc/runtime/node.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.tstest/annotation-parser.test.tstest/generator.test.tstest/integration.test.tstest/mapper.test.tstest/type_mapping_edge_cases.test.tstest/type_mapping_enhanced.test.tstest/worker-pool.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:238-249
Timestamp: 2026-01-20T01:33:12.841Z
Learning: In the tywrap repository, when reviewing init/startup patterns, prioritize cases that break core functionality over edge cases with diagnostic-only impact (e.g., missing bridgeInfo metadata). If workers are healthy and operational, defer low-impact defensive checks to follow-up PRs.
📚 Learning: 2026-01-20T01:34:07.064Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:444-458
Timestamp: 2026-01-20T01:34:07.064Z
Learning: When reviewing promise-based polling patterns (e.g., recursive setTimeout in waitForAvailableWorker functions), ensure that any recursive timer is tracked and cleared on timeout or promise resolution to prevent timer leaks. Do not rely on no-op checks after settlement; verify clearTimeout is invoked in all paths and consider using an explicit cancellation (e.g., AbortController) or a guard to cancel pending timers when the promise settles.
Applied to files:
src/index.tssrc/core/annotation-parser.tstest/annotation-parser.test.tstest/worker-pool.test.tssrc/config/index.tssrc/core/mapper.tstest/integration.test.tstest/type_mapping_enhanced.test.tssrc/runtime/node.tstest/mapper.test.tstest/generator.test.tstest/type_mapping_edge_cases.test.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.tssrc/core/generator.ts
📚 Learning: 2026-01-20T18:37:05.670Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: src/runtime/process-io.ts:37-40
Timestamp: 2026-01-20T18:37:05.670Z
Learning: In the tywrap repo, ESLint is used for linting (not Biome). Do not flag regex literals that contain control characters (e.g., \u001b, \u0000-\u001F) as lint errors, since the current ESLint configuration allows them. When reviewing TypeScript files (e.g., src/**/*.ts), rely on ESLint results and avoid raising issues about these specific control-character regex literals unless there is a competing config change or a policy requiring explicit escaping.
Applied to files:
src/index.tssrc/core/annotation-parser.tstest/annotation-parser.test.tstest/worker-pool.test.tssrc/config/index.tssrc/core/mapper.tstest/integration.test.tstest/type_mapping_enhanced.test.tssrc/runtime/node.tstest/mapper.test.tstest/generator.test.tstest/type_mapping_edge_cases.test.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.tssrc/core/generator.ts
📚 Learning: 2026-01-20T16:00:49.829Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:203-211
Timestamp: 2026-01-20T16:00:49.829Z
Learning: In the BridgeProtocol implementation (tywrap), reject Map and Set explicitly before serialization (e.g., in safeStringify or the serialization entrypoint) with a clear error like "Bridge protocol does not support Map/Set values". Do not rely on post-hoc checks of non-string keys at the point of JSON.stringify, since Maps/Sets cannot round-trip. This should be enforced at the boundary where data is prepared for serialization to ensure deterministic errors and prevent invalid data from propagating.
Applied to files:
src/index.tssrc/core/annotation-parser.tstest/annotation-parser.test.tstest/worker-pool.test.tssrc/config/index.tssrc/core/mapper.tstest/integration.test.tstest/type_mapping_enhanced.test.tssrc/runtime/node.tstest/mapper.test.tstest/generator.test.tstest/type_mapping_edge_cases.test.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.tssrc/core/generator.ts
📚 Learning: 2026-01-19T21:48:27.823Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/runtime_bridge_fixtures.test.ts:59-66
Timestamp: 2026-01-19T21:48:27.823Z
Learning: In fixture-based tests (e.g., test/runtime_bridge_fixtures.test.ts) and similar tests in the tywrap repository, prefer early returns when Python or fixture files are unavailable. Do not rely on Vitest dynamic skip APIs; a silent pass is intentional for environments lacking Python/fixtures. Treat missing fixtures as optional soft-guards and ensure the test remains non-disruptive in non-availability scenarios.
Applied to files:
test/annotation-parser.test.tstest/worker-pool.test.tstest/integration.test.tstest/type_mapping_enhanced.test.tstest/mapper.test.tstest/generator.test.tstest/type_mapping_edge_cases.test.ts
📚 Learning: 2026-01-19T21:14:37.032Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:375-385
Timestamp: 2026-01-19T21:14:37.032Z
Learning: In tywrap (src/runtime/bridge-core.ts and similar), environment variable parsing follows a tolerant/best-effort policy. For example, `TYWRAP_CODEC_MAX_BYTES=1024abc` should be accepted as 1024. Only reject clearly invalid values (non-numeric start or <=0). This avoids surprising failures from minor typos.
Applied to files:
README.mdsrc/config/index.tsdocs/guide/configuration.mddocs/reference/env-vars.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In `src/runtime/node-bridge.ts` (NodeBridge), a test message is sent to the Python subprocess to confirm the bridge is responsive before marking it as ready.
Applied to files:
README.mdtest/integration.test.tssrc/runtime/node.tsdocs/guide/getting-started.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In the tywrap repository, bridges currently use implicit ready detection: the first successful response from the Python subprocess proves the bridge is ready to handle requests.
Applied to files:
README.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-19T21:14:40.872Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:40.872Z
Learning: In `src/runtime/bridge-core.ts` and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (e.g., runtime shape checks on error payloads) in tight loops unless the protocol boundary is untrusted or there's a concrete bug report. The Python bridge protocol is controlled and validated via tests, so defensive checks would add unnecessary branching overhead without meaningful benefit.
Applied to files:
README.mdtest/integration.test.tsdocs/guide/getting-started.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-19T21:48:45.693Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-298
Timestamp: 2026-01-19T21:48:45.693Z
Learning: In `src/runtime/bridge-core.ts`, keep `normalizeErrorPayload` to validate error payloads from the Python subprocess. The subprocess boundary is effectively untrusted, and normalizing error responses prevents `undefined: undefined` errors on malformed payloads. Error responses are not the hot path, so the small validation overhead is acceptable for the added resilience.
Applied to files:
README.mdsrc/runtime/node.tsdocs/reference/env-vars.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-19T21:00:30.825Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/adversarial_playground.test.ts:339-360
Timestamp: 2026-01-19T21:00:30.825Z
Learning: In adversarial tests for the bridge (test/adversarial_playground.test.ts), recovery assertions should be kept in the finally block adjacent to cleanup to verify resilience under stress, even if that means post-timeout checks might mask the original error. The goal is to surface recovery failures as legitimate test failures.
Applied to files:
test/worker-pool.test.tstest/integration.test.ts
📚 Learning: 2026-01-19T21:14:35.390Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:35.390Z
Learning: In src/runtime/bridge-core.ts and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (such as runtime, shape, or error-payload checks) inside tight loops unless the protocol boundary is untrusted or there is a concrete bug report. The Python bridge protocol is controlled via tests, so these checks add unnecessary branching overhead without meaningful benefit. Apply this guidance to other hot-path runtime loops in src/runtime/**/*.ts, and re-enable additional validations only when a documented risk or failure scenario is identified. Ensure tests cover protocol validation where applicable.
Applied to files:
src/runtime/node.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.ts
📚 Learning: 2026-01-19T21:14:29.869Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:375-385
Timestamp: 2026-01-19T21:14:29.869Z
Learning: In the runtime env-var parsing (e.g., in src/runtime/bridge-core.ts and similar modules), adopt a tolerant, best-effort policy: numeric env values should parse by taking the leading numeric portion (e.g., TYWRAP_CODEC_MAX_BYTES=1024abc -> 1024). Only reject clearly invalid values (non-numeric start or <= 0). This reduces surprising failures from minor typos. Add tests to cover partial numeric prefixes, and ensure downstream logic documents the trusted range; consider a small helper to extract a positive integer from a string with a safe fallback.
Applied to files:
src/runtime/node.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.ts
📚 Learning: 2026-01-20T16:01:39.136Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:826-830
Timestamp: 2026-01-20T16:01:39.136Z
Learning: In the tywrap BridgeProtocol architecture (ADR-002), `HttpIO` (and other Transport implementations like `ProcessIO`, `PyodideIO`) implements only the `Transport` interface (methods: `send`, `isReady`, `dispose`). The `RuntimeExecution` interface methods (`call`, `instantiate`, `callMethod`, `disposeInstance`) are implemented by `BridgeProtocol`, which delegates to `Transport` instances. Transport implementations should not include RuntimeExecution methods.
Applied to files:
docs/guide/runtimes/node.md
📚 Learning: 2026-01-19T21:49:05.612Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: runtime/python_bridge.py:99-123
Timestamp: 2026-01-19T21:49:05.612Z
Learning: In the tywrap repository, TYWRAP_REQUEST_MAX_BYTES uses strict integer parsing that rejects values with trailing characters (e.g., "1024abc"). This differs from TYWRAP_CODEC_MAX_BYTES, which uses tolerant/best-effort parsing that accepts numeric prefixes. The strict policy for REQUEST_MAX_BYTES ensures explicit integer values and consistent parse behavior across Node/Python implementations.
Applied to files:
docs/guide/runtimes/node.md
📚 Learning: 2026-01-20T01:36:14.701Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:743-759
Timestamp: 2026-01-20T01:36:14.701Z
Learning: In the tywrap repository, when reviewing concurrent request handling patterns (especially with BridgeCore request multiplexing), flag uses of boolean `busy` flags that should be request counters instead. A boolean flag creates a race condition: when request A completes and sets `busy = false`, request B (still in flight via multiplexing) is incorrectly treated as idle. The fix is to use an `inFlightRequests: number` counter that increments on request start and decrements on completion, then check `worker.inFlightRequests === 0` before cleanup/termination.
Applied to files:
docs/guide/runtimes/node.md
🪛 GitHub Actions: CI
test/integration.test.ts
[error] 121-121: AssertionError: expected 0 to be 0? actually received 2. Test 'generate() emits valid TypeScript for advanced typing fixtures' failed. Expected compile.code to be 0 but got 2.
🪛 GitHub Check: test (20, 3.10)
test/integration.test.ts
[failure] 121-121: test/integration.test.ts > IR-only integration > generate() emits valid TypeScript for advanced typing fixtures
AssertionError: expected 2 to be +0 // Object.is equality
- Expected
- Received
- 0
- 2
❯ test/integration.test.ts:121:28
🪛 GitHub Check: test (22, 3.10)
test/integration.test.ts
[failure] 121-121: test/integration.test.ts > IR-only integration > generate() emits valid TypeScript for advanced typing fixtures
AssertionError: expected 2 to be +0 // Object.is equality
- Expected
- Received
- 0
- 2
❯ test/integration.test.ts:121:28
🔇 Additional comments (44)
docs/reference/cli.md (1)
1-129: LGTM!The CLI reference is comprehensive, covering both commands with their flags, exit codes, and a helpful starter config example. The flags table for
tywrap generatenow includes all supported options..github/labeler.yml (1)
26-33: Minor redundancy in docs patterns (previously noted).Lines 31-33 (
CONTRIBUTING.md,AGENTS.md,CLAUDE.md) are already covered by the*.mdglob on line 30. These explicit entries are redundant but harmless—they may serve as documentation of important files.test/generator.test.ts (1)
221-227: LGTM!The updated assertion correctly validates that when a function has both
varArgsandkwArgsparameters without required keyword-only params, the generator skips intermediate overloads and only emits the full implementation signature. This aligns with the overload generation logic ingenerator.ts(line 278 condition).docs/public/llms.txt (1)
1-29: LGTM!The
llms.txtfile provides a well-structured index for AI coding agents with clear categorization of documentation resources. The docs and optional sections appropriately prioritize essential guides over supplementary materials.docs/guide/getting-started.md (1)
1-376: LGTM!The getting started guide provides comprehensive coverage of installation, configuration, generation, and usage patterns. The formatting improvements enhance readability without changing the technical content.
src/core/generator.ts (3)
120-120: Good: UsingglobalThis.Array.isArrayfor cross-realm safety.This change ensures reliable array detection even when values originate from different JavaScript realms (e.g., iframes, VMs).
278-278: LGTM!The tightened condition correctly skips overload generation when
varArgsParamorneedsKwargsParamare present. This prevents emitting invalid TypeScript signatures where rest parameters would conflict with trailing kwargs.
370-370: Consistent use ofglobalThis.Array.isArrayacross all call preludes.The change is applied uniformly to function wrappers (line 370), class methods (line 596), and constructors (line 783), ensuring consistent cross-realm array detection throughout generated code.
Also applies to: 596-596, 783-783
src/core/annotation-parser.ts (5)
43-71: Well-structured helper for parsing typing factory names.The
parseTypingFactoryNamefunction correctly handles:
- Multiple module prefixes (typing, typing_extensions, collections.abc)
- Quoted string arguments with proper quote matching
- Arguments with trailing commas (e.g.,
TypeVar('T', bound=int))This cleanly extracts the name from factory calls like
TypeVar('T').
73-90: Good refactor:matchBracketedAliasreduces duplication.This helper consolidates the pattern matching for bracketed type aliases (Optional[T], Sequence[T], etc.) across all module prefixes. The return of both alias name and inner content enables context-aware handling downstream.
231-248: Correct handling of inferred type variables and ParamSpec components.
~Tpatterns are correctly mapped totypevarkindP.args→list[Any]andP.kwargs→dict[str, Any]are reasonable runtime approximations for ParamSpec components
283-331: Comprehensive alias handling for collection and async types.The new parsing branches correctly map:
Sequence[T]→listcollectionMapping[K, V]→dictcollectionIterator/Iterable/AsyncIterator/AsyncIterable→ generic typesAwaitable[T]→Promise<T>Coroutine[T, Y, R]→Promise<R>(extracts return type)These mappings align well with TypeScript equivalents.
371-396: Improved Callable parsing handles edge cases.The enhanced parsing correctly handles:
Callable[..., R]with ellipsis for any argumentsCallable[[T1, T2], R]with explicit parameter lists- Malformed inputs default to ellipsis parameter (defensive)
src/core/mapper.ts (4)
105-114: LGTM! Proper handling of variable-length tuple syntax.The ellipsis check correctly identifies
tuple[T, ...]Python syntax and maps it toArray<T>in TypeScript, which is the idiomatic representation for variable-length homogeneous tuples.
279-284: LGTM! Sound fallback for unsupported typing constructs.Degrading
typingandtyping_extensionsplaceholders tounknownprevents undeclared TypeScript identifiers (likeT,P,Ts) from leaking into generated code. The comment clearly explains the rationale.
331-337: LGTM! Return type change aligns with the type system.The return type change from
TSCustomTypetoTSPrimitiveTypeis correct sinceunknownis a primitive type. This is consistent with theTypescriptTypeunion insrc/types/index.tswhereTSPrimitiveTypeis a valid member.
468-473: LGTM! Pydantic preset follows established pattern.The
pydanticpreset mapping forBaseModeltorecordObjectfollows the same structure as other presets (pandas, scipy, torch, sklearn) and correctly reuses therecordObjectdefined at the top ofmapPresetType.package.json (2)
74-78: LGTM! Documentation scripts are well-structured.The docs workflow scripts follow a logical pattern:
docs:sync-llmsgenerates AI-discoverable docs,docs:buildensures sync before build, and the standard VitePress dev/preview scripts are included.
157-160: Document the purpose of the@types/reactand@types/prop-typesoverrides.The
@types/react@^18.3.0override resolves a peer dependency conflict from@docsearch/react(an Algolia dependency of VitePress) which requires@types/react <19. Pinning to ^18.3.0 prevents resolution conflicts when projects upgrade to@types/react@19. Add a comment in package.json explaining this dependency constraint for future maintainers.> Likely an incorrect or invalid review comment.docs/guide/configuration.md (2)
3-16: LGTM! Improved table formatting and field descriptions.The top-level fields table is now properly aligned with consistent column widths and clearer descriptions.
568-574: LGTM! Links updated for VitePress site structure.The absolute URL paths (
/troubleshooting/,/guide/runtimes/node,/examples/,/reference/api/) are correct for a VitePress deployment and follow the site navigation structure.src/index.ts (1)
10-10: LGTM! Public API extended with resolved config type.Exporting
ResolvedTywrapConfigalongsideTywrapConfigallows consumers to type-safely work with fully resolved configurations, improving the developer experience for programmatic API usage.test/mapper.test.ts (2)
112-119: LGTM! Test validates pydantic preset behavior.The test correctly verifies that with the
pydanticpreset enabled,pydantic.BaseModelmaps to an object type (therecordObjectshape).
121-153: LGTM! Comprehensive coverage for TypeVar-to-unknown mapping.The test covers three important cases:
- Direct
typevarkind →unknownprimitive- Custom type from
typingmodule →unknownprimitive- Callable with TypeVar return type → function with
unknownreturnThis ensures undeclared TypeScript identifiers won't leak into generated code.
docs/reference/env-vars.md (2)
65-86: LGTM! Clear documentation of configuration approaches.The section correctly explains that Python executable and virtual environment are configured via code or config files rather than environment variables, with practical examples for both approaches.
10-15: 🧹 Nitpick | 🔵 TrivialConsider documenting tolerant parsing behavior for numeric variables.
Based on learnings,
TYWRAP_CODEC_MAX_BYTES=1024abcis accepted as1024(tolerant parsing). Users might benefit from knowing this behavior to avoid confusion when debugging.📝 Suggested addition to table notes
| Variable | Scope | Default | Purpose | | -------------------------- | ------------- | ------- | ------------------------------------------------------------------------------ | | `TYWRAP_CODEC_FALLBACK` | Python bridge | unset | Set to `json` to allow JSON fallback when Arrow encoding is unavailable | -| `TYWRAP_CODEC_MAX_BYTES` | Python bridge | unset | Reject response payloads larger than this byte count | -| `TYWRAP_REQUEST_MAX_BYTES` | Python bridge | unset | Reject request payloads larger than this byte count | +| `TYWRAP_CODEC_MAX_BYTES` | Python bridge | unset | Reject response payloads larger than this byte count (parsed tolerantly) | +| `TYWRAP_REQUEST_MAX_BYTES` | Python bridge | unset | Reject request payloads larger than this byte count (parsed tolerantly) |⛔ Skipped due to learnings
Learnt from: bbopen Repo: bbopen/tywrap PR: 127 File: src/runtime/bridge-core.ts:375-385 Timestamp: 2026-01-19T21:14:37.032Z Learning: In tywrap (src/runtime/bridge-core.ts and similar), environment variable parsing follows a tolerant/best-effort policy. For example, `TYWRAP_CODEC_MAX_BYTES=1024abc` should be accepted as 1024. Only reject clearly invalid values (non-numeric start or <=0). This avoids surprising failures from minor typos.Learnt from: bbopen Repo: bbopen/tywrap PR: 127 File: runtime/python_bridge.py:99-123 Timestamp: 2026-01-19T21:49:05.612Z Learning: In the tywrap repository, TYWRAP_REQUEST_MAX_BYTES uses strict integer parsing that rejects values with trailing characters (e.g., "1024abc"). This differs from TYWRAP_CODEC_MAX_BYTES, which uses tolerant/best-effort parsing that accepts numeric prefixes. The strict policy for REQUEST_MAX_BYTES ensures explicit integer values and consistent parse behavior across Node/Python implementations.Learnt from: bbopen Repo: bbopen/tywrap PR: 152 File: docs/adr/002-bridge-protocol.md:599-602 Timestamp: 2026-01-20T16:01:14.323Z Learning: In the tywrap repository, bridges currently use implicit ready detection: the first successful response from the Python subprocess proves the bridge is ready to handle requests.Learnt from: bbopen Repo: bbopen/tywrap PR: 152 File: docs/adr/002-bridge-protocol.md:168-172 Timestamp: 2026-01-20T16:00:49.738Z Learning: In the tywrap project's BridgeProtocol SafeCodec implementation, Arrow format decoders can produce NaN/Infinity values from binary representations even when the raw JSON payload doesn't contain them. This is why validation for special floats must occur both before encoding (to reject invalid inputs) and after applying decoders (to catch values introduced during Arrow deserialization), protecting downstream consumers from unexpected NaN/Infinity values.docs/reference/type-mapping.md (3)
1-68: LGTM! Comprehensive and accurate type mapping reference.The documentation accurately reflects the implementation in
src/core/mapper.ts:
- Primitives, collections, unions, and callables are correctly documented
- Special names section properly documents TypeVar/ParamSpec →
unknownbehavior- Tables are well-formatted and easy to scan
93-98: LGTM! Pydantic preset documentation with runtime context.Good inclusion of the runtime serialization note (
model_dump(by_alias=True, mode='json')), which helps users understand both the static type and runtime behavior.
143-153: LGTM! Clear documentation of mapping limitations.The "Limits and Fallbacks" section accurately explains the constraints:
- TypeVar lowering rationale
- Annotated metadata stripping
- Runtime vs static type differences
This sets appropriate expectations for users.
test/worker-pool.test.ts (1)
931-957: LGTM! Comprehensive test for background worker replacement.The test correctly verifies the new replacement behavior: when a worker times out, the pool disposes the old transport, creates a replacement, and maintains the correct worker count. The
setTimeout(resolve, 0)allows the async replacement to complete before assertions.test/annotation-parser.test.ts (1)
75-106: 🛠️ Refactor suggestion | 🟠 MajorAdd coverage for bare
Callable[P, T]from the advanced fixture.
test/fixtures/python/advanced_types.pyusesCallable[P, T], but this block only exercises the factory forms andtyping.Callable[~P, ~T]. That lets these unit tests stay green while the new end-to-end fixture still fails to compile. Add assertions for the exact fixture spelling here so regressions fail closer to source.⛔ Skipped due to learnings
Learnt from: bbopen Repo: bbopen/tywrap PR: 127 File: test/fixtures/out_of_order_bridge.py:29-48 Timestamp: 2026-01-19T21:00:52.689Z Learning: Policy for adversarial test fixtures: adversarial fixtures must preserve the failure mode they exercise, even if that behavior would be "bad" in real code. When reviewing test fixtures with unusual patterns (e.g., missing error handling, intentional protocol violations, unanswered requests), ask for clarifying comments about the intent rather than suggesting normalization or fixes.README.md (5)
6-10: LGTM! New badges improve project visibility.The added Coverage, npm downloads, PRs Welcome, and Docs badges provide useful project health indicators and encourage community engagement.
26-35: LGTM! Feature comparison table is well-structured.The "Why tywrap?" comparison table clearly highlights differentiators against alternatives (pythonia, node-calls-python, pymport) and helps users quickly assess fit.
71-72: LGTM! Star CTA is appropriately placed.The star call-to-action is non-intrusive and positioned well after the Quick Start section.
87-99: LGTM! Runtime bridge notes are clarified.The updated notes about NodeBridge vs OptimizedNodeBridge and environment inheritance (
inheritProcessEnv,TYWRAP_CODEC_MAX_BYTES,TYWRAP_REQUEST_MAX_BYTES) are consistent with the code behavior documented in the retrieved learnings.
141-142: LGTM! Documentation links migrated to external URLs.All documentation links now point to the hosted docs site at
bbopen.github.io/tywrap, consistent with the VitePress docs deployment added in this PR.Also applies to: 165-168
src/runtime/worker-pool.ts (3)
42-48: LGTM! Well-documented callback for replacement workers.The
onReplacementWorkerReadycallback allows callers to distinguish between initial worker readiness and background replacement worker readiness, enabling separate warmup/publish strategies without charging latency to normal requests.
405-430: LGTM! Background replacement worker scheduling is well-implemented.The implementation correctly:
- Guards against non-ready state and maxWorkers limit
- Tracks
pendingCreationsto prevent over-spawning during concurrent removals- Falls back to
onWorkerReadywhenonReplacementWorkerReadyis not provided- Calls
release(worker)to wake queued waiters after the replacement is ready- Silently ignores background creation failures (appropriate for proactive replacement)
- Uses
Math.max(0, ...)defensively infinallyto ensure counter stays non-negativeBased on learnings, prioritizing core functionality over edge cases with diagnostic-only impact is appropriate here.
464-469: LGTM! Guard against disposal during worker creation.This check prevents leaking partially-created workers when the pool is torn down mid-creation. The transport is properly disposed before throwing.
docs/guide/runtimes/node.md (5)
20-27: LGTM! Clear bridge selection guidance.Good clarification that OptimizedNodeBridge is experimental and not part of public API exports, with appropriate reference to ROADMAP.md for unification plans.
80-84: LGTM! Important distinction between config file and constructor options.Clearly separating
tywrap.config.*(wrapper generation) fromnew NodeBridge(...)(runtime subprocess control) helps users understand the layered configuration model.
120-139: LGTM! Configuration table matches actual interface.The documented options, types, and defaults align with the
NodeBridgeOptionsinterface insrc/runtime/node.ts. Deprecation notes for legacy fields (maxIdleTime,maxRequestsPerProcess,enableJsonFallback,maxLineLength) are appropriately flagged.
238-267: LGTM! Environment variable approach for JSON fallback and payload limits.Documenting
TYWRAP_CODEC_FALLBACK=json,TYWRAP_CODEC_MAX_BYTES, andTYWRAP_REQUEST_MAX_BYTESaligns with the tolerant env-var parsing policy in bridge-core and provides clear guidance on payload control. Based on learnings, the env-var approach avoids surprising failures from minor typos.
623-628: LGTM! Next steps use root-relative paths.Using root-relative paths (
/guide/configuration,/examples/,/troubleshooting/,/reference/api/) is appropriate for the VitePress docs site structure.
| const sections = []; | ||
| for (const file of orderedDocs) { | ||
| const text = await readFile(file, 'utf8'); | ||
| sections.push(`<!-- Source: ${file} -->\n${text.trimEnd()}\n`); | ||
| } | ||
|
|
||
| const output = `${header}${sections.join('\n---\n\n').trimEnd()}\n`; | ||
| await writeFile('docs/public/llms-full.txt', output, 'utf8'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding error handling for file I/O operations.
The readFile and writeFile calls in the aggregation loop will throw unhandled rejections if any file operation fails. Adding a try/catch with a descriptive error message would improve debugging when docs are missing or unreadable.
♻️ Optional: wrap I/O in try/catch
+ try {
const sections = [];
for (const file of orderedDocs) {
const text = await readFile(file, 'utf8');
sections.push(`<!-- Source: ${file} -->\n${text.trimEnd()}\n`);
}
const output = `${header}${sections.join('\n---\n\n').trimEnd()}\n`;
await writeFile('docs/public/llms-full.txt', output, 'utf8');
+ } catch (err) {
+ throw new Error(`Failed to generate llms-full.txt: ${err.message}`);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/generate-llms-full.mjs` around lines 78 - 86, Wrap the file I/O in a
try/catch around the loop and final write so readFile and writeFile errors are
handled: iterate over orderedDocs inside a try block, catch and log a
descriptive error (including the filename and the caught error) when a readFile
fails while building sections, and similarly catch and log/write a thrown error
when writing the final output (output/header/sections). Ensure the catch
rethrows or exits appropriately for your script (so failures are visible), and
reference the variables/functions sections, orderedDocs, readFile, writeFile,
header, and output when adding the error handling.
| if (!/^[A-Za-z_$][A-Za-z0-9_$]*$/.test(c.name)) { | ||
| return 'unknown'; | ||
| } | ||
| return c.name; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent fallback to unknown may hide invalid type names.
When a custom type name doesn't match the valid TS identifier pattern, the code silently returns 'unknown'. Per the context snippet from mapper.ts, qualified names like "pandas.DataFrame" are normalized to extract the leaf name, but if normalization fails or edge cases arise, the fallback happens silently.
Consider emitting a warning via a diagnostics hook (similar to onUnknownTypeName in annotation-parser) so users can identify when their Python types aren't mapping correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/generator.ts` around lines 983 - 986, The current fallback that
returns the literal 'unknown' when a custom type name fails the TS identifier
regex should emit a diagnostic instead of failing silently: in the function that
performs this normalization (the block that tests /^[A-Za-z_$][A-Za-z0-9_$]*$/
and returns c.name or 'unknown'), call the diagnostics hook (e.g.,
onUnknownTypeName) with the original candidate name and the fallback value, then
return the fallback; include context like the raw input and the normalized/leaf
name so users can see what failed. Locate this logic in generator.ts (the
identifier-validation branch) and wire in the same diagnostics hook used by
annotation-parser (onUnknownTypeName) or the existing diagnostics mechanism so
warnings are produced when the fallback is used.
| async function executeWorkerCall( | ||
| worker: PooledWorker, | ||
| request: WorkerCallSpec, | ||
| timeoutMs: number, | ||
| messages: WorkerCallErrorMessages | ||
| ): Promise<unknown> { | ||
| const requestId = generateWarmupId(); | ||
| let message: string; | ||
| try { | ||
| message = JSON.stringify({ | ||
| id: requestId, | ||
| protocol: PROTOCOL_ID, | ||
| method: 'call', | ||
| params: { | ||
| module: request.module, | ||
| functionName: request.functionName, | ||
| args: request.args, | ||
| kwargs: {}, | ||
| }, | ||
| }); | ||
| } catch (error) { | ||
| throw new BridgeExecutionError( | ||
| `${messages.label} failed to encode request: ${error instanceof Error ? error.message : String(error)}`, | ||
| { cause: error instanceof Error ? error : undefined } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Reject Map/Set warmup arguments before building the JSON request.
executeWorkerCall() serializes user-supplied warmup args directly. Map and Set collapse during JSON.stringify(), so a bad warmup config can produce confusing Python-side behavior instead of a deterministic bridge error. Add the same boundary check the main bridge protocol uses before this envelope is encoded.
Based on learnings, in the BridgeProtocol implementation (tywrap), reject Map and Set explicitly before serialization with a clear error like "Bridge protocol does not support Map/Set values".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime/node.ts` around lines 634 - 659, In executeWorkerCall, detect and
reject any Map or Set inside the warmup arguments before building the JSON
envelope: inspect request.args (and any nested values) and throw a
BridgeExecutionError using the provided messages.label and a clear message like
"Bridge protocol does not support Map/Set values" (preserving the error cause if
available) rather than letting JSON.stringify collapse them; update the code
path around generateWarmupId()/JSON.stringify to run this validation and fail
fast so the bridge protocol never receives Map/Set values.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guide/configuration.md (1)
333-352: 🧹 Nitpick | 🔵 TrivialConsider documenting tolerant env-var parsing behavior.
Based on learnings, tywrap uses tolerant/best-effort env-var parsing (e.g.,
TYWRAP_CODEC_MAX_BYTES=1024abcis accepted as1024). This behavior might be worth documenting here so users understand partial numeric prefixes are accepted.📝 Optional documentation addition
export TYWRAP_CODEC_MAX_BYTES="10485760" # Max response payload size (bytes) export TYWRAP_REQUEST_MAX_BYTES="1048576" # Max request payload size (bytes) + +# Note: Numeric env values use tolerant parsing—leading numeric portions +# are extracted (e.g., "1024abc" → 1024). Only reject non-numeric starts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guide/configuration.md` around lines 333 - 352, Add a short note in the Runtime configuration section explaining that environment variables such as TYWRAP_CODEC_MAX_BYTES, TYWRAP_REQUEST_MAX_BYTES, TYWRAP_MEMORY_LIMIT, etc., are parsed tolerantly (partial numeric prefixes are accepted, e.g. "1024abc" → 1024) and that this best-effort behavior applies to numeric env-vars; mention any limits or expected units (bytes, MB, ms) and recommend validating values if exact parsing is required.
♻️ Duplicate comments (8)
AGENTS.md (1)
40-83:⚠️ Potential issue | 🟡 MinorAdd a language tag to the repository tree fenced block.
Line 40 uses an unlabeled code fence, which still triggers markdownlint MD040.
Suggested fix
-``` +```text src/ index.ts # Public API re-exports @@ dist/ # Compiled TypeScript output (do not edit)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@AGENTS.mdaround lines 40 - 83, Update the unlabeled fenced code block in
AGENTS.md (the repository tree block) to include a language tag (e.g., change
the opening "" to "text") so markdownlint MD040 is satisfied; locate the
repository tree block starting with "src/" and modify its opening fence only to
include the language tag while keeping the block contents unchanged.</details> </blockquote></details> <details> <summary>CLAUDE.md (1)</summary><blockquote> `40-83`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language tag to the repository tree fenced block.** Line 40 starts an unlabeled fence; markdownlint MD040 will keep flagging it. <details> <summary>Suggested fix</summary> ```diff -``` +```text src/ index.ts # Public API re-exports @@ dist/ # Compiled TypeScript output (do not edit) ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@CLAUDE.mdaround lines 40 - 83, The repo tree fenced code block in CLAUDE.md
is unlabeled; change the opening fence fromtotext so the block is
labeled (e.g., replace the initialbefore "src/" withtext) and leave the
closing ``` as-is; this will satisfy markdownlint MD040 for the repo tree block.</details> </blockquote></details> <details> <summary>.github/labeler.yml (1)</summary><blockquote> `29-33`: _🧹 Nitpick_ | _🔵 Trivial_ **Remove redundant explicit Markdown file patterns.** `*.md` already matches Lines 31-33, so those entries are unnecessary. <details> <summary>Optional cleanup</summary> ```diff area:docs: - changed-files: - any-glob-to-any-file: - 'docs/**' - '*.md' - - 'CONTRIBUTING.md' - - 'AGENTS.md' - - 'CLAUDE.md' ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In @.github/labeler.yml around lines 29 - 33, Remove the redundant explicit Markdown patterns in the labeler configuration: because the '*.md' glob already covers 'CONTRIBUTING.md', 'AGENTS.md', and 'CLAUDE.md', delete those specific entries from the list while keeping 'docs/**' and the '*.md' entry; update the block that currently contains 'docs/**', '*.md', 'CONTRIBUTING.md', 'AGENTS.md', 'CLAUDE.md' to only include the necessary patterns (e.g., 'docs/**' and '*.md'). ``` </details> </blockquote></details> <details> <summary>src/runtime/node.ts (1)</summary><blockquote> `634-653`: _⚠️ Potential issue_ | _🟡 Minor_ **Reject `Map`/`Set` warmup arguments before request encoding.** At Line 643, `request.args` is encoded directly; `Map`/`Set` values can degrade during `JSON.stringify`, causing non-deterministic bridge behavior instead of an explicit failure. <details> <summary>Proposed fix</summary> ```diff +function assertNoMapSet(value: unknown): void { + const stack: unknown[] = [value]; + const seen = new Set<unknown>(); + while (stack.length > 0) { + const current = stack.pop(); + if (!current || typeof current !== 'object') continue; + if (current instanceof Map || current instanceof Set) { + throw new BridgeExecutionError('Bridge protocol does not support Map/Set values'); + } + if (seen.has(current)) continue; + seen.add(current); + if (Array.isArray(current)) { + for (const item of current) stack.push(item); + } else { + for (const item of Object.values(current as Record<string, unknown>)) stack.push(item); + } + } +} + async function executeWorkerCall( worker: PooledWorker, request: WorkerCallSpec, timeoutMs: number, messages: WorkerCallErrorMessages ): Promise<unknown> { const requestId = generateWarmupId(); + assertNoMapSet(request.args); let message: string; ``` </details> ```shell #!/bin/bash rg -n -C3 "executeWorkerCall|JSON.stringify|Map/Set values|Bridge protocol does not support Map/Set values" src/runtime/node.ts src/runtime/safe-codec.ts ``` Based on learnings, in tywrap bridge serialization boundaries should reject `Map`/`Set` explicitly before serialization with a deterministic error. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/runtime/node.ts` around lines 634 - 653, The warmup payload encodes request.args directly with JSON.stringify in executeWorkerCall, which will silently degrade Map/Set values; add an explicit pre-encoding check that walks request.args and request.kwargs (from the WorkerCallSpec) and rejects any Map or Set (including nested occurrences) before the JSON.stringify call, throwing a clear error using the worker call error path (e.g., via WorkerCallErrorMessages or a thrown Error) so the bridge fails deterministically instead of producing non-deterministic serialized output. ``` </details> </blockquote></details> <details> <summary>docs/guide/runtimes/comparison.md (1)</summary><blockquote> `28-37`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language identifier to the decision-guide code fence.** Use an explicit fence language (for example `text`) to avoid markdown lint warnings and improve accessibility. <details> <summary>📝 Proposed fix</summary> ```diff -``` +```text Do you need subprocess-based Python execution? ├── Yes → Does your environment support subprocess? │ ├── Node.js or Bun → Use NodeBridge (import from 'tywrap/node' or 'tywrap') │ ├── Deno (local) → Use NodeBridge with --allow-run=python3 │ └── Deno Deploy / serverless → Continue ↓ └── No (browser, edge, serverless) → ├── Can you load ~50MB WebAssembly? → Use PyodideBridge └── Need heavy Python libs or can't load WASM → Use HttpBridge ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/guide/runtimes/comparison.mdaround lines 28 - 37, Add an explicit
language identifier to the decision-guide code fence (the fenced block starting
with the ASCII decision tree) so linting and accessibility tools recognize it;
change the opening triple backticks to include a language such as "text" (e.g.,guide.docs/guide/runtimes/bun.md (1)
40-43:⚠️ Potential issue | 🟠 MajorCorrect the
bunfig.tomldistribution claim.The current wording implies
bunfig.tomlis shipped by the package; this section should present it as a sample file users create in their own Bun projects.📝 Proposed fix
## bunfig.toml -tywrap ships with a `bunfig.toml` for build and run settings: +Example `bunfig.toml` you can add to your Bun project for build and run settings:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guide/runtimes/bun.md` around lines 40 - 43, The heading/section "## bunfig.toml" incorrectly states that tywrap "ships with a `bunfig.toml`"; change the copy to present it as a sample file users should add to their Bun project (e.g., "Example `bunfig.toml` for build and run settings" or "Sample `bunfig.toml` you can add to your Bun project") and remove any wording that implies the file is distributed by tywrap so readers understand they must create it themselves.docs/reference/api/index.md (1)
247-262:⚠️ Potential issue | 🟡 MinorDocument authoring input as
TywrapConfig, not resolvedTywrapOptions.This section currently makes the public input API look stricter than it is (for example,
runtime,performance, anddevelopmentappear mandatory on Line 254–Line 256).📘 Suggested doc fix
-## Key Types +## Key Types ```ts -interface TywrapOptions { +interface TywrapConfig { pythonModules: Record<string, PythonModuleConfig>; pythonImportPath?: string[]; - output: OutputConfig; - runtime: RuntimeConfig; - performance: PerformanceConfig; - development: DevelopmentConfig; + output?: OutputConfig; + runtime?: RuntimeConfig; + performance?: PerformanceConfig; + development?: DevelopmentConfig; types?: TypeMappingConfig; debug?: boolean; }</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/reference/api/index.mdaround lines 247 - 262, Update the docs to
present the public input shape as TywrapConfig (not the resolved TywrapOptions)
and make the environment-specific fields optional: change the interface name
from TywrapOptions to TywrapConfig and mark output, runtime, performance, and
development as optional (output?:, runtime?:, performance?:, development?:) so
the doc reflects the actual public API; locate the interface block that defines
TywrapOptions in docs/reference/api/index.md and apply these renames and
optional modifiers.</details> </blockquote></details> <details> <summary>test/integration.test.ts (1)</summary><blockquote> `100-122`: _⚠️ Potential issue_ | _🟠 Major_ **Surface `tsc` diagnostics when compile fails.** The current assertion on Line 121 fails without printing actionable compiler output, which blocks debugging the existing red CI run. <details> <summary>🛠️ Suggested fix</summary> ```diff const compile = await processUtils.exec( process.execPath, [ @@ { cwd: process.cwd(), timeoutMs: 30_000 } ); - expect(compile.code).toBe(0); - expect(compile.stderr).toBe(''); + const diagnostics = [compile.stdout, compile.stderr].filter(Boolean).join('\n'); + expect(compile.code, diagnostics || 'TypeScript compile failed with no diagnostics').toBe(0); + expect(compile.stderr, diagnostics).toBe('');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration.test.ts` around lines 100 - 122, The test currently asserts compile success but drops tsc output, making failures uninformative; update the block around processUtils.exec / compile to print or include compiler diagnostics when compile.code !== 0 by referencing the compile variable returned from processUtils.exec (and tscPath/generatedPath for context). Concretely, add a conditional that logs or fails with compile.stderr and compile.stdout (e.g., console.error or fail(`tsc failed: ${compile.stderr}\n${compile.stdout}`)) before the expect checks so the diagnostic text is visible when compilation fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/index.md`:
- Around line 49-51: The blockquote has an extra blank line between the two
quoted lines which triggers markdownlint MD028; remove the empty line so both
lines remain consecutive blockquote lines (each starting with ">"), i.e., make
the lines starting with "⚠️ **Experimental** — APIs may change before v1.0.0..."
and "If tywrap saves you time, a ⭐ on [GitHub]..." contiguous with no blank line
between them to satisfy MD028.
---
Outside diff comments:
In `@docs/guide/configuration.md`:
- Around line 333-352: Add a short note in the Runtime configuration section
explaining that environment variables such as TYWRAP_CODEC_MAX_BYTES,
TYWRAP_REQUEST_MAX_BYTES, TYWRAP_MEMORY_LIMIT, etc., are parsed tolerantly
(partial numeric prefixes are accepted, e.g. "1024abc" → 1024) and that this
best-effort behavior applies to numeric env-vars; mention any limits or expected
units (bytes, MB, ms) and recommend validating values if exact parsing is
required.
---
Duplicate comments:
In @.github/labeler.yml:
- Around line 29-33: Remove the redundant explicit Markdown patterns in the
labeler configuration: because the '*.md' glob already covers 'CONTRIBUTING.md',
'AGENTS.md', and 'CLAUDE.md', delete those specific entries from the list while
keeping 'docs/**' and the '*.md' entry; update the block that currently contains
'docs/**', '*.md', 'CONTRIBUTING.md', 'AGENTS.md', 'CLAUDE.md' to only include
the necessary patterns (e.g., 'docs/**' and '*.md').
In `@AGENTS.md`:
- Around line 40-83: Update the unlabeled fenced code block in AGENTS.md (the
repository tree block) to include a language tag (e.g., change the opening "```"
to "```text") so markdownlint MD040 is satisfied; locate the repository tree
block starting with "src/" and modify its opening fence only to include the
language tag while keeping the block contents unchanged.
In `@CLAUDE.md`:
- Around line 40-83: The repo tree fenced code block in CLAUDE.md is unlabeled;
change the opening fence from ``` to ```text so the block is labeled (e.g.,
replace the initial ``` before "src/" with ```text) and leave the closing ```
as-is; this will satisfy markdownlint MD040 for the repo tree block.
In `@docs/guide/runtimes/bun.md`:
- Around line 40-43: The heading/section "## bunfig.toml" incorrectly states
that tywrap "ships with a `bunfig.toml`"; change the copy to present it as a
sample file users should add to their Bun project (e.g., "Example `bunfig.toml`
for build and run settings" or "Sample `bunfig.toml` you can add to your Bun
project") and remove any wording that implies the file is distributed by tywrap
so readers understand they must create it themselves.
In `@docs/guide/runtimes/comparison.md`:
- Around line 28-37: Add an explicit language identifier to the decision-guide
code fence (the fenced block starting with the ASCII decision tree) so linting
and accessibility tools recognize it; change the opening triple backticks to
include a language such as "text" (e.g., ```text) to avoid markdown-lint
warnings and improve rendering for the decision guide.
In `@docs/reference/api/index.md`:
- Around line 247-262: Update the docs to present the public input shape as
TywrapConfig (not the resolved TywrapOptions) and make the environment-specific
fields optional: change the interface name from TywrapOptions to TywrapConfig
and mark output, runtime, performance, and development as optional (output?:,
runtime?:, performance?:, development?:) so the doc reflects the actual public
API; locate the interface block that defines TywrapOptions in
docs/reference/api/index.md and apply these renames and optional modifiers.
In `@src/runtime/node.ts`:
- Around line 634-653: The warmup payload encodes request.args directly with
JSON.stringify in executeWorkerCall, which will silently degrade Map/Set values;
add an explicit pre-encoding check that walks request.args and request.kwargs
(from the WorkerCallSpec) and rejects any Map or Set (including nested
occurrences) before the JSON.stringify call, throwing a clear error using the
worker call error path (e.g., via WorkerCallErrorMessages or a thrown Error) so
the bridge fails deterministically instead of producing non-deterministic
serialized output.
In `@test/integration.test.ts`:
- Around line 100-122: The test currently asserts compile success but drops tsc
output, making failures uninformative; update the block around processUtils.exec
/ compile to print or include compiler diagnostics when compile.code !== 0 by
referencing the compile variable returned from processUtils.exec (and
tscPath/generatedPath for context). Concretely, add a conditional that logs or
fails with compile.stderr and compile.stdout (e.g., console.error or fail(`tsc
failed: ${compile.stderr}\n${compile.stdout}`)) before the expect checks so the
diagnostic text is visible when compilation fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7a603497-67e3-427e-88fa-167b3733f55f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (51)
.github/dependabot.yml.github/labeler.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/labeler.yml.github/workflows/publish.yml.github/workflows/release.yml.github/workflows/stale.yml.gitignoreAGENTS.mdCLAUDE.mdREADME.mddocs/.vitepress/config.tsdocs/api/README.mddocs/examples/index.mddocs/guide/configuration.mddocs/guide/getting-started.mddocs/guide/runtimes/browser.mddocs/guide/runtimes/bun.mddocs/guide/runtimes/comparison.mddocs/guide/runtimes/deno.mddocs/guide/runtimes/http.mddocs/guide/runtimes/node.mddocs/index.mddocs/plans/2026-03-10-tywrap-promotion.mddocs/public/llms-full.txtdocs/public/llms.txtdocs/reference/api/index.mddocs/reference/cli.mddocs/reference/env-vars.mddocs/reference/type-mapping.mddocs/troubleshooting/index.mddocs/type-mapping-matrix.mdpackage.jsonscripts/generate-llms-full.mjssrc/config/index.tssrc/core/annotation-parser.tssrc/core/generator.tssrc/core/mapper.tssrc/index.tssrc/runtime/node.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.tstest/annotation-parser.test.tstest/generator.test.tstest/integration.test.tstest/mapper.test.tstest/type_mapping_edge_cases.test.tstest/type_mapping_enhanced.test.tstest/worker-pool.test.tstywrap_ir/pyproject.toml
💤 Files with no reviewable changes (3)
- docs/type-mapping-matrix.md
- .github/workflows/publish.yml
- docs/api/README.md
📜 Review details
🧰 Additional context used
🧠 Learnings (16)
📚 Learning: 2026-01-19T21:14:40.872Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:40.872Z
Learning: In `src/runtime/bridge-core.ts` and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (e.g., runtime shape checks on error payloads) in tight loops unless the protocol boundary is untrusted or there's a concrete bug report. The Python bridge protocol is controlled and validated via tests, so defensive checks would add unnecessary branching overhead without meaningful benefit.
Applied to files:
docs/guide/runtimes/comparison.mdAGENTS.mddocs/guide/runtimes/http.mdREADME.mddocs/guide/getting-started.mdtest/integration.test.tsdocs/plans/2026-03-10-tywrap-promotion.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-20T16:01:39.136Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:826-830
Timestamp: 2026-01-20T16:01:39.136Z
Learning: In the tywrap BridgeProtocol architecture (ADR-002), `HttpIO` (and other Transport implementations like `ProcessIO`, `PyodideIO`) implements only the `Transport` interface (methods: `send`, `isReady`, `dispose`). The `RuntimeExecution` interface methods (`call`, `instantiate`, `callMethod`, `disposeInstance`) are implemented by `BridgeProtocol`, which delegates to `Transport` instances. Transport implementations should not include RuntimeExecution methods.
Applied to files:
docs/guide/runtimes/comparison.mddocs/guide/runtimes/http.mddocs/plans/2026-03-10-tywrap-promotion.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In the tywrap repository, bridges currently use implicit ready detection: the first successful response from the Python subprocess proves the bridge is ready to handle requests.
Applied to files:
docs/guide/runtimes/comparison.mddocs/guide/runtimes/http.mdREADME.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-19T21:48:27.823Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/runtime_bridge_fixtures.test.ts:59-66
Timestamp: 2026-01-19T21:48:27.823Z
Learning: In fixture-based tests (e.g., test/runtime_bridge_fixtures.test.ts) and similar tests in the tywrap repository, prefer early returns when Python or fixture files are unavailable. Do not rely on Vitest dynamic skip APIs; a silent pass is intentional for environments lacking Python/fixtures. Treat missing fixtures as optional soft-guards and ensure the test remains non-disruptive in non-availability scenarios.
Applied to files:
test/type_mapping_edge_cases.test.tstest/integration.test.tstest/mapper.test.tstest/worker-pool.test.tstest/type_mapping_enhanced.test.tstest/annotation-parser.test.tstest/generator.test.ts
📚 Learning: 2026-01-20T01:34:07.064Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:444-458
Timestamp: 2026-01-20T01:34:07.064Z
Learning: When reviewing promise-based polling patterns (e.g., recursive setTimeout in waitForAvailableWorker functions), ensure that any recursive timer is tracked and cleared on timeout or promise resolution to prevent timer leaks. Do not rely on no-op checks after settlement; verify clearTimeout is invoked in all paths and consider using an explicit cancellation (e.g., AbortController) or a guard to cancel pending timers when the promise settles.
Applied to files:
test/type_mapping_edge_cases.test.tssrc/runtime/node.tssrc/config/index.tstest/integration.test.tssrc/runtime/pooled-transport.tstest/mapper.test.tssrc/core/generator.tssrc/index.tssrc/core/mapper.tssrc/core/annotation-parser.tstest/worker-pool.test.tstest/type_mapping_enhanced.test.tstest/annotation-parser.test.tstest/generator.test.tssrc/runtime/worker-pool.ts
📚 Learning: 2026-01-20T18:37:05.670Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: src/runtime/process-io.ts:37-40
Timestamp: 2026-01-20T18:37:05.670Z
Learning: In the tywrap repo, ESLint is used for linting (not Biome). Do not flag regex literals that contain control characters (e.g., \u001b, \u0000-\u001F) as lint errors, since the current ESLint configuration allows them. When reviewing TypeScript files (e.g., src/**/*.ts), rely on ESLint results and avoid raising issues about these specific control-character regex literals unless there is a competing config change or a policy requiring explicit escaping.
Applied to files:
test/type_mapping_edge_cases.test.tssrc/runtime/node.tssrc/config/index.tstest/integration.test.tssrc/runtime/pooled-transport.tstest/mapper.test.tssrc/core/generator.tssrc/index.tssrc/core/mapper.tssrc/core/annotation-parser.tstest/worker-pool.test.tstest/type_mapping_enhanced.test.tstest/annotation-parser.test.tstest/generator.test.tssrc/runtime/worker-pool.ts
📚 Learning: 2026-01-20T16:00:49.829Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:203-211
Timestamp: 2026-01-20T16:00:49.829Z
Learning: In the BridgeProtocol implementation (tywrap), reject Map and Set explicitly before serialization (e.g., in safeStringify or the serialization entrypoint) with a clear error like "Bridge protocol does not support Map/Set values". Do not rely on post-hoc checks of non-string keys at the point of JSON.stringify, since Maps/Sets cannot round-trip. This should be enforced at the boundary where data is prepared for serialization to ensure deterministic errors and prevent invalid data from propagating.
Applied to files:
test/type_mapping_edge_cases.test.tssrc/runtime/node.tssrc/config/index.tstest/integration.test.tssrc/runtime/pooled-transport.tstest/mapper.test.tssrc/core/generator.tssrc/index.tssrc/core/mapper.tssrc/core/annotation-parser.tstest/worker-pool.test.tstest/type_mapping_enhanced.test.tstest/annotation-parser.test.tstest/generator.test.tssrc/runtime/worker-pool.ts
📚 Learning: 2026-01-19T21:14:37.032Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:375-385
Timestamp: 2026-01-19T21:14:37.032Z
Learning: In tywrap (src/runtime/bridge-core.ts and similar), environment variable parsing follows a tolerant/best-effort policy. For example, `TYWRAP_CODEC_MAX_BYTES=1024abc` should be accepted as 1024. Only reject clearly invalid values (non-numeric start or <=0). This avoids surprising failures from minor typos.
Applied to files:
docs/guide/runtimes/http.mddocs/reference/env-vars.mdREADME.mdsrc/config/index.tsdocs/reference/api/index.mddocs/guide/configuration.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In `src/runtime/node-bridge.ts` (NodeBridge), a test message is sent to the Python subprocess to confirm the bridge is responsive before marking it as ready.
Applied to files:
docs/guide/runtimes/http.mdsrc/runtime/node.tsREADME.mddocs/guide/getting-started.mdtest/integration.test.tsdocs/plans/2026-03-10-tywrap-promotion.mddocs/guide/runtimes/node.md
📚 Learning: 2026-01-19T21:48:45.693Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-298
Timestamp: 2026-01-19T21:48:45.693Z
Learning: In `src/runtime/bridge-core.ts`, keep `normalizeErrorPayload` to validate error payloads from the Python subprocess. The subprocess boundary is effectively untrusted, and normalizing error responses prevents `undefined: undefined` errors on malformed payloads. Error responses are not the hot path, so the small validation overhead is acceptable for the added resilience.
Applied to files:
docs/reference/env-vars.mdsrc/runtime/node.tsREADME.mdsrc/core/generator.tsdocs/guide/runtimes/node.md
📚 Learning: 2026-01-19T21:14:35.390Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:260-263
Timestamp: 2026-01-19T21:14:35.390Z
Learning: In src/runtime/bridge-core.ts and similar hot request/response loop implementations in the tywrap repository, avoid adding extra defensive validation (such as runtime, shape, or error-payload checks) inside tight loops unless the protocol boundary is untrusted or there is a concrete bug report. The Python bridge protocol is controlled via tests, so these checks add unnecessary branching overhead without meaningful benefit. Apply this guidance to other hot-path runtime loops in src/runtime/**/*.ts, and re-enable additional validations only when a documented risk or failure scenario is identified. Ensure tests cover protocol validation where applicable.
Applied to files:
src/runtime/node.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.ts
📚 Learning: 2026-01-19T21:00:30.825Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/adversarial_playground.test.ts:339-360
Timestamp: 2026-01-19T21:00:30.825Z
Learning: In adversarial tests for the bridge (test/adversarial_playground.test.ts), recovery assertions should be kept in the finally block adjacent to cleanup to verify resilience under stress, even if that means post-timeout checks might mask the original error. The goal is to surface recovery failures as legitimate test failures.
Applied to files:
src/runtime/node.tstest/integration.test.tstest/worker-pool.test.ts
📚 Learning: 2026-01-20T01:36:14.701Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:743-759
Timestamp: 2026-01-20T01:36:14.701Z
Learning: In the tywrap repository, when reviewing concurrent request handling patterns (especially with BridgeCore request multiplexing), flag uses of boolean `busy` flags that should be request counters instead. A boolean flag creates a race condition: when request A completes and sets `busy = false`, request B (still in flight via multiplexing) is incorrectly treated as idle. The fix is to use an `inFlightRequests: number` counter that increments on request start and decrements on completion, then check `worker.inFlightRequests === 0` before cleanup/termination.
Applied to files:
src/runtime/node.tsdocs/guide/runtimes/node.md
📚 Learning: 2026-01-19T21:14:29.869Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: src/runtime/bridge-core.ts:375-385
Timestamp: 2026-01-19T21:14:29.869Z
Learning: In the runtime env-var parsing (e.g., in src/runtime/bridge-core.ts and similar modules), adopt a tolerant, best-effort policy: numeric env values should parse by taking the leading numeric portion (e.g., TYWRAP_CODEC_MAX_BYTES=1024abc -> 1024). Only reject clearly invalid values (non-numeric start or <= 0). This reduces surprising failures from minor typos. Add tests to cover partial numeric prefixes, and ensure downstream logic documents the trusted range; consider a small helper to extract a positive integer from a string with a safe fallback.
Applied to files:
src/runtime/node.tssrc/runtime/pooled-transport.tssrc/runtime/worker-pool.ts
📚 Learning: 2026-01-20T01:33:12.841Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:238-249
Timestamp: 2026-01-20T01:33:12.841Z
Learning: In the tywrap repository, when reviewing init/startup patterns, prioritize cases that break core functionality over edge cases with diagnostic-only impact (e.g., missing bridgeInfo metadata). If workers are healthy and operational, defer low-impact defensive checks to follow-up PRs.
Applied to files:
src/runtime/node.tsdocs/plans/2026-03-10-tywrap-promotion.md
📚 Learning: 2026-01-19T21:49:05.612Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: runtime/python_bridge.py:99-123
Timestamp: 2026-01-19T21:49:05.612Z
Learning: In the tywrap repository, TYWRAP_REQUEST_MAX_BYTES uses strict integer parsing that rejects values with trailing characters (e.g., "1024abc"). This differs from TYWRAP_CODEC_MAX_BYTES, which uses tolerant/best-effort parsing that accepts numeric prefixes. The strict policy for REQUEST_MAX_BYTES ensures explicit integer values and consistent parse behavior across Node/Python implementations.
Applied to files:
docs/guide/runtimes/node.md
🪛 GitHub Actions: CI
test/integration.test.ts
[error] 121-121: Test 'generate() emits valid TypeScript for advanced typing fixtures' failed. AssertionError: expected 2 to be +0.
🪛 GitHub Check: test (20, 3.10)
test/integration.test.ts
[failure] 121-121: test/integration.test.ts > IR-only integration > generate() emits valid TypeScript for advanced typing fixtures
AssertionError: expected 2 to be +0 // Object.is equality
- Expected
- Received
- 0
- 2
❯ test/integration.test.ts:121:28
🪛 GitHub Check: test (22, 3.10)
test/integration.test.ts
[failure] 121-121: test/integration.test.ts > IR-only integration > generate() emits valid TypeScript for advanced typing fixtures
AssertionError: expected 2 to be +0 // Object.is equality
- Expected
- Received
- 0
- 2
❯ test/integration.test.ts:121:28
🪛 LanguageTool
docs/plans/2026-03-10-tywrap-promotion.md
[uncategorized] ~1250-~1250: The official name of this software platform is spelled with a capital “H”.
Context: ...ep 1: Add the Codecov upload step** In .github/workflows/ci.yml, find the test job....
(GITHUB)
🪛 markdownlint-cli2 (0.21.0)
AGENTS.md
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/index.md
[warning] 50-50: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🔇 Additional comments (48)
.gitignore (1)
51-53: Good addition for docs artifact hygiene.Ignoring
docs/.vitepress/dist/anddocs/.vitepress/cache/prevents noisy commits from generated docs outputs.docs/troubleshooting/index.md (1)
647-648: Link migration looks correct.The updated runtime links match the new guide routing structure and reduce relative-path fragility.
.github/workflows/ci.yml (1)
71-79: Coverage gating strategy is well implemented.Running coverage on a single matrix cell and uploading only from that cell keeps CI time/cost controlled while preserving coverage reporting.
.github/workflows/labeler.yml (1)
10-16: Permissions and workflow scope look appropriate.The job keeps privileges minimal while enabling automated labeling on PR updates.
docs/public/llms.txt (1)
10-29: LLM discovery index is clear and well scoped.The curated links are relevant and aligned with the new docs structure.
src/runtime/pooled-transport.ts (1)
44-48: LGTM! Clean callback wiring for replacement worker readiness.The new
onReplacementWorkerReadycallback follows the established pattern ofonWorkerReady. The type definition, storage, and forwarding toWorkerPoolare correctly implemented.Also applies to: 86-88, 111-111, 133-133
.github/dependabot.yml (2)
1-22: LGTM! Well-structured Dependabot configuration.The npm and github-actions ecosystems are properly configured with grouping, weekly schedules, and appropriate PR limits. The consistent labeling with
area:ciis good for automation.Also applies to: 36-50
24-34: Consider adding grouping for pip dependencies.The pip ecosystem lacks grouping, unlike npm and github-actions. This was already flagged in a previous review.
.github/workflows/release.yml (1)
1-57: LGTM! Well-structured release automation workflow.The workflow correctly:
- Gates npm publishing on
release_createdoutput- Uses environment protection (
environment: npm) for the NPM token- Publishes with
--provenancefor supply chain security- Runs tests before publishing to catch regressions
.github/workflows/docs.yml (1)
1-52: LGTM! Standard VitePress + GitHub Pages deployment workflow.Good practices observed:
fetch-depth: 0enables VitePress "last updated" timestampscancel-in-progress: falseprevents partial deployments- Proper permissions scoped for Pages deployment
.github/workflows/stale.yml (1)
1-39: LGTM! Well-configured stale management workflow.The configuration appropriately:
- Uses shorter timelines for PRs (60+14 days) vs issues (90+30 days)
- Exempts security, pinned, and contribution-friendly labels
- Provides helpful messages guiding users on next steps
scripts/generate-llms-full.mjs (2)
54-67: LGTM! Solid order validation logic.The sync check between
orderedDocsand discovered files ensures the aggregation stays consistent. The detailed error message listing both "Missing from order" and "Not found on disk" will help maintainers quickly resolve mismatches.
78-86: Consider adding error handling for file I/O operations.This concern was already raised in a previous review. Adding a try/catch would provide clearer error messages when specific files fail to read.
tywrap_ir/pyproject.toml (1)
19-19: LGTM! Metadata improvements enhance discoverability.The expanded keywords and classifiers better describe the package's purpose. The documentation URL correctly points to the new VitePress site deployed by this PR.
Also applies to: 32-34, 40-40
docs/guide/configuration.md (1)
1-574: LGTM! Comprehensive formatting improvements.The documentation is well-organized with consistent table formatting, properly aligned columns, and clear code examples. The restructured sections and updated links improve navigation.
src/core/annotation-parser.ts (1)
43-411: Expanded typing construct support is well-structured.The new alias/factory helpers and branch ordering read cleanly and preserve the existing API while broadening parser coverage.
docs/plans/2026-03-10-tywrap-promotion.md (1)
9-9: Keep theactions/stalemajor version consistent across the plan.Line 9 specifies
actions/stale@v10, but Line 1407 still showsactions/stale@v9.Suggested fix
- - uses: actions/stale@v9 + - uses: actions/stale@v10#!/bin/bash rg -n "actions/stale@v" docs/plans/2026-03-10-tywrap-promotion.mdAlso applies to: 1407-1409
package.json (1)
75-79: Docs tooling and metadata updates look consistent.The scripts, homepage, and VitePress dependency changes align cleanly with the new docs workflow.
Also applies to: 112-113, 143-160
src/core/mapper.ts (1)
279-337: Theunknownfallback strategy here is a good safeguard.This avoids generating invalid TypeScript identifiers from unsupported Python typing placeholders.
Also applies to: 468-473
src/runtime/worker-pool.ts (1)
405-430: Background replacement + disposal handling is solid.The new replacement path and mid-creation disposal guard improve resilience without blocking foreground requests.
Also applies to: 438-469
test/worker-pool.test.ts (1)
931-957: Good regression coverage for timeout-triggered worker replacement.This test captures the key replacement/disposal invariants and verifies the newly acquired worker source.
docs/examples/index.md (1)
74-79: Updated docs links are aligned with the new route layout.The path migration in this section is consistent with the guide/reference restructuring.
docs/guide/runtimes/deno.md (1)
3-78: Deno runtime guidance is clear and actionable.The setup, permissions, and Deno Deploy caveat are documented clearly and match the expected bridge behavior.
test/generator.test.ts (1)
221-226: Good regression guard for overload pruning.This negative assertion cleanly protects against reintroducing the removed overload shape.
docs/reference/type-mapping.md (1)
1-171: Type-mapping reference is comprehensive and well-structured.Coverage of core mapping rules, presets, limits, and examples is clear and aligns with the updated typing behavior.
docs/reference/env-vars.md (1)
10-86: Environment variable documentation is accurate and practical.The variable tables and examples are clear, and the separation between env-configurable vs code-configurable options is helpful.
test/type_mapping_edge_cases.test.ts (1)
271-324: Edge-case expectations now correctly enforceunknownfallback behavior.These assertions line up with the mapper change for TypeVar/ParamSpec/TypeVarTuple handling.
docs/reference/cli.md (1)
31-100: Comprehensive CLI coverage looks solid.The flag tables, config resolution order, and CI-oriented
--check/exit-code section are clear and actionable.docs/.vitepress/config.ts (1)
3-84: VitePress configuration is well-structured.Navigation, sidebar grouping, local search, and GitHub edit-link wiring are consistent with the new docs IA.
test/annotation-parser.test.ts (1)
51-106: Great coverage expansion for advanced annotation forms.These cases close important gaps around
collections.abcaliases and TypeVar/ParamSpec-derived shapes.src/index.ts (1)
10-10: Good public type-surface update.Exporting
ResolvedTywrapConfigwithTywrapConfigimproves API discoverability without runtime risk.test/mapper.test.ts (1)
112-153: Nice regression coverage for advanced mapping cases.The new assertions around
pydantic.BaseModeland TypeVar-like placeholder fallback are precise and useful.docs/guide/getting-started.md (1)
291-297: Clear CI and navigation guidance improvements.The updated
--checkworkflow notes and root-relative “Next Steps/Support” links improve first-run and maintenance UX.Also applies to: 367-375
docs/guide/runtimes/http.md (2)
35-40: Correct thecodecdefault value in the options table.Line 40 lists the default for
codecas "Arrow", butHttpBridgeOptions.codecis an optional configuration object, not a transport-mode default. This could mislead users about configuration behavior.📝 Suggested fix
-| `codec` | No | Arrow | Codec options | +| `codec` | No | — | Codec options (validation/serialization settings) |
1-72: LGTM overall — documentation provides clear guidance for HttpBridge usage.The guide covers installation, setup, options, Arrow transport, environment variables, and security recommendations appropriately. The content aligns with the BridgeProtocol architecture where
HttpBridgeimplements theTransportinterface.src/core/generator.ts (3)
120-120: Good defensive change usingglobalThis.Array.isArray.Using
globalThis.Array.isArrayinstead ofArray.isArrayensures correct behavior when arrays cross realm boundaries (e.g., iframe, vm contexts). This is a sound defensive pattern for generated runtime code.
983-986: Silent fallback tounknownmay hide invalid type names.When a custom type name doesn't match a valid TypeScript identifier pattern, the code silently returns
'unknown'. This can mask issues where Python types aren't mapping correctly, making debugging difficult for users.Consider emitting a warning via a diagnostics hook so users can identify when their custom types fail validation.
278-278: This condition is intentional by design. The code explicitly excludes generating positional-drop overloads when varargs or kwargs are present, as documented in the comment at line 233: "generate trailing optional parameter drop variants (exclude *args/**kwargs)".This approach improves rather than reduces ergonomics: mixing positional-drop overloads with varargs/kwargs creates ambiguous and complex signatures. The test at line 144 validates this behavior—a function with optional parameters plus varargs/kwargs expects only the full signature overload, never a "dropped" variant. The current implementation is correct.
test/type_mapping_enhanced.test.ts (2)
15-63: LGTM — TypeVar tests correctly updated to expectunknownprimitive.The tests consistently validate that all TypeVar variants (basic, bounded, constrained, covariant) map to
{ kind: 'primitive', name: 'unknown' }. This aligns with the type mapping changes and provides good coverage for the simplified TypeVar handling.
259-264: LGTM — Test correctly validates custom type preservation.Using
(result as any).nameis acceptable in tests to access properties when the exact type shape varies. The test correctly verifies that unknown custom types preserve their name and module.src/config/index.ts (2)
32-40: LGTM — Clean type distinction between partial and resolved config.Separating
TywrapConfig(user-authored partial config) fromResolvedTywrapConfig(fully resolved config) improves API clarity. Users get better type inference when authoring configs withdefineConfig(), while internal code benefits from the fully-resolved type guarantee.
203-222: LGTM — Return types correctly updated.Both
createConfigandresolveConfignow returnResolvedTywrapConfig, accurately reflecting that defaults are merged and the config is complete after these calls.README.md (3)
26-35: Good addition of feature comparison table.The comparison table clearly differentiates tywrap from alternatives (pythonia, node-calls-python, pymport), highlighting unique features like auto-generated types, browser support, and multi-runtime compatibility.
141-142: LGTM — Configuration link updated to hosted docs.The configuration guide link now points to
https://bbopen.github.io/tywrap/guide/configuration, consistent with the docs site migration.
165-168: LGTM — Documentation links consistently point to hosted site.All documentation links (Getting Started, Configuration, API Reference, Troubleshooting) now use the
bbopen.github.io/tywrapURLs.docs/guide/runtimes/node.md (3)
18-27: Good bridge selection guidance.The documentation clearly distinguishes NodeBridge (default, recommended) from OptimizedNodeBridge (experimental, not public API). Referencing ROADMAP.md for the unification plan helps set expectations.
120-139: Comprehensive options table with clear deprecation guidance.The options table covers all constructor parameters with types, defaults, and descriptions. The deprecation notice for
maxIdleTime,maxRequestsPerProcess,enableJsonFallback, andmaxLineLengthhelps users avoid deprecated fields in new code.
246-267: LGTM — Payload limit documentation is accurate.The documentation correctly describes
TYWRAP_CODEC_MAX_BYTESfor response limits andTYWRAP_REQUEST_MAX_BYTESfor request limits, with explicit error behavior noted. This aligns with the repository's environment variable handling approach. Based on learnings,TYWRAP_REQUEST_MAX_BYTESuses strict integer parsing whileTYWRAP_CODEC_MAX_BYTESuses tolerant/best-effort parsing.
| > ⚠️ **Experimental** — APIs may change before v1.0.0. See [CHANGELOG](https://github.com/bbopen/tywrap/blob/main/CHANGELOG.md) for breaking changes. | ||
|
|
||
| > If tywrap saves you time, a ⭐ on [GitHub](https://github.com/bbopen/tywrap) helps others find it. |
There was a problem hiding this comment.
Fix blockquote spacing to satisfy markdownlint (MD028).
There is a blank line inside the blockquote sequence.
📝 Proposed fix
> ⚠️ **Experimental** — APIs may change before v1.0.0. See [CHANGELOG](https://github.com/bbopen/tywrap/blob/main/CHANGELOG.md) for breaking changes.
-
> If tywrap saves you time, a ⭐ on [GitHub](https://github.com/bbopen/tywrap) helps others find it.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > ⚠️ **Experimental** — APIs may change before v1.0.0. See [CHANGELOG](https://github.com/bbopen/tywrap/blob/main/CHANGELOG.md) for breaking changes. | |
| > If tywrap saves you time, a ⭐ on [GitHub](https://github.com/bbopen/tywrap) helps others find it. | |
| > ⚠️ **Experimental** — APIs may change before v1.0.0. See [CHANGELOG](https://github.com/bbopen/tywrap/blob/main/CHANGELOG.md) for breaking changes. | |
| > If tywrap saves you time, a ⭐ on [GitHub](https://github.com/bbopen/tywrap) helps others find it. |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 50-50: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/index.md` around lines 49 - 51, The blockquote has an extra blank line
between the two quoted lines which triggers markdownlint MD028; remove the empty
line so both lines remain consecutive blockquote lines (each starting with ">"),
i.e., make the lines starting with "⚠️ **Experimental** — APIs may change before
v1.0.0..." and "If tywrap saves you time, a ⭐ on [GitHub]..." contiguous with no
blank line between them to satisfy MD028.
2 participants
Summary
Test Plan