Skip to content

fix(ts): forward per-call hook options through predictBatch - #936

Merged
NandhaKishorM merged 1 commit into
NandhaKishorM:mainfrom
sathariels:fix/ts-predict-batch-hook-options
Oct 5, 2026
Merged

NandhaKishorM merged 1 commit into
NandhaKishorM:mainfrom
sathariels:fix/ts-predict-batch-hook-options

Conversation

@sathariels

Copy link
Copy Markdown
Contributor

What

TypeScript Router.predictBatch and predictMany accept onPredictStart, onPredictEnd and hooksRaise, and compose them the way Router.predict does. hooksRaise also applies to onRoute. This is the TypeScript counterpart of the Python predict_batch hook forwarding in #912.

Why

predictBatch only composed opts.hooks and always used the Router's hooksRaise, so a per-call start or end hook was dropped and a per-call hooksRaise: false was ignored. predictMany forwards to predictBatch, so it had the same gap.

How it was verified

cd laya-ts && npm ci && npm test -- --run && npx tsc --noEmit
python tests/test_router.py
ruff check laya/ --select=E9,F63,F7,F82,F401,F811 --line-length=120

Vitest: 34 files, 632 passed. tsc --noEmit: clean. python tests/test_router.py: 703 passed, 0 failed. Ruff: all checks passed.

Checklist

  • Focused on one change (split unrelated work into another PR)
  • Rebased on the latest main
  • Tests pass locally
  • Docs or examples updated when the public API changed

Fixes #935

@NandhaKishorM
NandhaKishorM merged commit 20f294f into NandhaKishorM:main Oct 5, 2026
30 checks passed
@NandhaKishorM

Copy link
Copy Markdown
Owner

Merged, and this is the one of the two fixes for #935 that went in.

What decided it: besides the composition at the dispatch site, this also forwards hooksRaise into the routeBatch call, so a per-call hooksRaise: false reaches onRoute and not only the predict lifecycle. #943 fixes the dispatch site identically but leaves onRoute on the router-level flag. The README update is the other half, since predictBatch's documented hook surface was the thing that disagreed with predict.

#943 does have two tests this does not, and they are worth having: a per-call hooksRaise: true turning raising back on where the router has it off, and an assertion that a per-call callback runs after a per-call hooks entry rather than merely that both ran. I have asked there for them as a test-only follow-up.

Merge note: this conflicted with #929, which rewrote the same docblock. I kept #929's sentence about setDefaultHooks defaults and extended it to name your two callables, so the comment now reads defaults, then installed, then this call's hooks, then onPredictStart / onPredictEnd. Worth confirming it still matches the code.

Verified on the merged tree: 654 TypeScript tests pass across 34 files, tsc --noEmit clean, all 81 Python suites pass.

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.

laya-ts: predictBatch ignores per-call onPredictStart, onPredictEnd and hooksRaise

3 participants