Skip to content

Address pre-publish review findings for corbits-embedding - #1

Merged
TheGreatAxios merged 3 commits into
mainfrom
cl-8192-address-pre-publish-review-findings-for-corbits-embedding
Sep 21, 2026
Merged

TheGreatAxios merged 3 commits into
mainfrom
cl-8192-address-pre-publish-review-findings-for-corbits-embedding

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Contributor

Summary

  • Reject batchSize below 1 at the embedTexts entry boundary with a RangeError plus a regression test.
  • README gains an install-from-npm section and documents the transport exports.
  • package.json gains homepage and bugs, a Node >= 20 engines floor, the exact author, and prettier-clean formatting.

Verification

  • bun run build: exit 0
  • bun run typecheck: exit 0
  • bun test ./src: 20 pass, 0 fail
  • bunx prettier --check .: clean
  • npm pack --dry-run: 12 files, 17.3 kB tarball

Fixes CL-8192

Reject batchSize below 1 at the embedTexts entry boundary, document installation and transport exports, and complete the publish metadata.
@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Review

Verdict: comment. MERGEABLE. CI check green, 20 tests. No prior reviews. batchSize < 1 now throws RangeError at embedTexts.

Should-fix: this PR documents npm install @corbits/embedding and engines.node >= 20, but packed dist/index.js uses extensionless from "./embed". Node import of the tarball throws ERR_MODULE_NOT_FOUND. Bun loads it. CI is bun-only.

Nit: EmbedConfigSchema batchSize is number > 0 (accepts 0.5); runtime is !(batchSize >= 1). Safe direction, schema/runtime disagree.

npm view @corbits/embedding 404s; git is the install path. Default export
is dist/; native Node does not load extensionless TypeScript source.
Node cannot import extensionless relative specifiers from packed dist.
Source relative imports now carry .js so tsc emits Node-loadable ESM.
EmbedConfigSchema and embedTexts both require batchSize integer >= 1,
rejecting 0.5 and NaN.
@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Re-review

Verdict: approve. SHA 6ca6878.

Packed dist emits .js specifiers. Node imports the tarball (embedTexts). batchSize schema/runtime are integer >= 1 (rejects 0, -1, 0.5, NaN, 1.5). Git-primary install unchanged. Local tests 22 pass, tsc/build/node pack import 0.

@TheGreatAxios
TheGreatAxios merged commit 394aa79 into main Sep 21, 2026
1 check passed
@TheGreatAxios
TheGreatAxios deleted the cl-8192-address-pre-publish-review-findings-for-corbits-embedding branch September 21, 2026 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant