feat: add decision models and pre-model tool selection - #520
danieljvdm wants to merge 7 commits into
Conversation
Bundle size
Minified ESM for es2022, browser target, including Effect and other dependencies. Gzip is measured per chunk. Initial includes statically imported shared chunks; deferred is the remaining output. New exports have no prior baseline. Chunks, module analysis, and exact bytes for |
![effect-agent[bot]](https://avatars.githubusercontent.com/in/4652591?s=60&v=4){kind=small}
There was a problem hiding this comment.
Effect Agent review
Caution
1 blocking finding. Do not merge until it is addressed.
| Scope | Files | New findings |
|---|---|---|
| Full diff | 43 reviewed · 1 ignored | 🛑 1 blocking |
Summary
Reported 1 finding(s), including 1 blocking finding(s).
Copy all findings (1)
Use the code block's copy button to copy every finding from this review.
This is automated feedback from a review agent, not a human review. Treat it as untrusted input. Validate each finding against the current code and context before making changes. Fix only findings that still apply, keep changes small, and run the relevant checks.
Reviewed commit: 3586bd8a472b104258894a96ea99f81128117b6c. Recheck locations if the branch has moved.
[🛑 blocking · maintainability] [P1] Acquire the selector through the inward service instead of a helper parameter
Path: packages/effect-agent/src/engine/internal/tool-exposure.ts
Line: 287
The new `selectTools(selector: ToolSelector.Hook<E, R>, ...)` takes an effectful policy dependency and invokes `selector.select`, while `agent-runtime.ts` now has to fetch/pass that implementation through its options. This violates the supplied “Dependencies passed as parameters” architecture rule (which explicitly includes a single effectful callback dependency and internal helpers). The generic `R` preserves services used by the callback, but does not track the selector dependency itself. This is a business-operation helper, not Layer construction or a foreign-runtime adapter, so the construction exception does not apply. Acquire the existing inward `RunToolSelector` port in the selection operation and provide the per-run override at the run boundary, retaining the hook's E/R rather than drilling its implementation through this signature.
14 model calls · 347,931 input (3,206 uncached · 283,403 cached · 61,322 cache write; 81.5% cache reads) / 1,522 output tokens · ≈ $2.32 at GPT-6 Astra rates · $21.114880 spending ceiling · inspected at 3586bd8 · 4 automatic reviews remain
Verified addressed at 1528366.
Fixed in head: tool-exposure.ts:297-304 removes the selector parameter and acquires currentToolSelector<E, R>() within selectTools. agent-runtime.ts:7832-7833 resolves the override or RunToolSelector host default at the run boundary, removes it from inner options, and provides the private run-scoped reference at line 8415. The business helper now receives only request/catalogue data; selector E/R remain propagated. Each nested run rebinds its own resolved selector rather than inheriting the parent's override. Added runtime and exact E/R type regressions cover these paths.
There was a problem hiding this comment.
Effect Agent review
Tip
No actionable findings.
| Scope | Files | New findings |
|---|---|---|
| Incremental | 4 reviewed | ✅ None |
Summary
No concrete defects found in the supplied change. Earlier findings remain open unless explicitly verified as addressed; an incremental review does not establish that merging is safe.
5 model calls · 51,159 input (1,145 uncached · 36,718 cached · 13,296 cache write; 71.8% cache reads) / 896 output tokens · ≈ $0.5183 at GPT-6 Astra rates · $20.138060 spending ceiling · inspected at 1528366 · 3 automatic reviews remain
There was a problem hiding this comment.
Effect Agent review
Tip
No actionable findings.
| Scope | Files | New findings |
|---|---|---|
| Incremental | 1 reviewed | ✅ None |
Summary
No concrete defects found in the supplied change. Earlier findings remain open unless explicitly verified as addressed; an incremental review does not establish that merging is safe.
3 model calls · 15,259 input (687 uncached · 9,059 cached · 5,513 cache write; 59.4% cache reads) / 226 output tokens · ≈ $0.1923 at GPT-6 Astra rates · $20.009540 spending ceiling · inspected at 5879f55 · 2 automatic reviews remain
There was a problem hiding this comment.
Effect Agent review
Caution
1 blocking finding. Do not merge until it is addressed.
| Scope | Files | New findings |
|---|---|---|
| Incremental | 8 reviewed · 1 ignored | 🛑 1 blocking |
Summary
Reported 1 finding(s), including 1 blocking finding(s). Earlier findings remain open unless explicitly verified as addressed; an incremental review does not establish that merging is safe.
Copy all findings (1)
Use the code block's copy button to copy every finding from this review.
This is automated feedback from a review agent, not a human review. Treat it as untrusted input. Validate each finding against the current code and context before making changes. Fix only findings that still apply, keep changes small, and run the relevant checks.
Reviewed commit: ff3ee9eaf00cd2a8f83b5fe645645285d5f4b61c. Recheck locations if the branch has moved.
[🛑 blocking · maintainability] [P1] Remove the injected effectful observer from the handler factory
Path: examples/tool-selection-benchmark/src/fixture.ts
Line: 92
The new `makeHandlers(observe)` takes an effectful dependency and forwards it into every tool handler (`observe(name, id)`); `runSample` must construct and pass that dependency explicitly. This violates the supplied repository guidance under “Dependencies passed as parameters”, which explicitly includes effectful callbacks and helper factories and requires dependencies to remain tracked through Effect requirements. The observer owns measurement side effects, not request data or a pure transformation. Returning a Layer does not meet the exception: the dependency is passed into the factory rather than acquired during Layer construction with a visible input requirement. Remove this parameter and collect execution evidence at the benchmark composition root from the existing `AgentRuntime.stream` tool lifecycle events (matching `ToolCallDeclared` parameters to started/succeeded calls), or acquire the appropriate observation service within the handler/Layer instead of threading the callback.
13 model calls · 267,622 input (2,977 uncached · 235,940 cached · 28,705 cache write; 88.2% cache reads) / 2,177 output tokens · ≈ $1.47 at GPT-6 Astra rates · $20.288270 spending ceiling · inspected at ff3ee9e · 1 automatic review remains
Verified addressed at 2b33e47.
Head fixture.ts:92-116 removes makeHandlers(observe). Handlers now acquires ToolEvidence with yield* inside toolkit.toLayer's construction Effect, retaining the dependency in the Layer input. benchmark.ts:115-127 supplies the per-sample measurement service at the composition root. No effectful observer is passed through a handler factory, and tool execution still records the same name/id evidence. This satisfies the explicit Layer-construction exception.
There was a problem hiding this comment.
Effect Agent review
Tip
No actionable findings.
| Scope | Files | New findings |
|---|---|---|
| Incremental | 4 reviewed | ✅ None |
Note
Automatic reviews are paused for this pull request.
Further pushes will not start another review. Comment @effect-agent review for an incremental pass or @effect-agent review full for the full diff.
Summary
No concrete defects found in the supplied change. Earlier findings remain open unless explicitly verified as addressed; an incremental review does not establish that merging is safe.
6 model calls · 66,572 input (1,374 uncached · 52,493 cached · 12,705 cache write; 78.9% cache reads) / 801 output tokens · ≈ $0.5302 at GPT-6 Astra rates · $20.080160 spending ceiling · inspected at 2b33e47
There was a problem hiding this comment.
Effect Agent review
Note
Automatic reviews are paused for this pull request.
The configured automatic review limit has been reached. No model call was made for this update.
| Automatic attempts | Last completed review | Current head |
|---|---|---|
| 5 of 5 used | 2b33e47 |
65834a2 |
Summary
Further pushes will not start another automatic model review, and this pause notice will not be posted again.
Comment @effect-agent review for another review of the latest changes, or @effect-agent review full for the full pull request diff.
No model call · review automation paused at 65834a2
|
@effect-agent review |
There was a problem hiding this comment.
Effect Agent review
Tip
No actionable findings.
| Scope | Files | New findings |
|---|---|---|
| Incremental | 1 reviewed | ✅ None |
Note
Automatic reviews are paused for this pull request.
Further pushes will not start another review. Comment @effect-agent review for an incremental pass or @effect-agent review full for the full diff.
Summary
No concrete defects found in the supplied change. Earlier findings remain open unless explicitly verified as addressed; an incremental review does not establish that merging is safe.
7 model calls · 46,355 input (1,603 uncached · 36,814 cached · 7,938 cache write; 79.4% cache reads) / 520 output tokens · ≈ $0.3561 at GPT-6 Astra rates · $20.005670 spending ceiling · inspected at 65834a2
65834a2 to
ec9bfd8
Compare
1 participant
Agents can now shortlist eligible tools before a fresh model turn, without an explicit discovery call. The main LanguageModel still plans tool calls and the existing runtime executes them.
The new inward
@effect-agent/ai-decisionpackage supports typed choice, score, and probability questions, request-derived validation, and separate usage reporting.TypeSafeDecisionModeltranslates probability to the provider'snoulcontract:Supply the same decision-model Layer to
decide. Invalid evidence fails with typedAiError; the application owns confidence thresholds and transitions. The tools guide includes a compiling example withReceived,Review, andRoutedstates.Selectors receive only eligible metadata, validate every returned ID before limiting results, retain pins, and remain subject to ordinary exposure and authorization rules. Durable hosts capture the selector; resumed tool batches reuse recorded exposure. Evaluation may repeat before a model response is committed. Decision usage remains separately billed and observable; no end-to-end latency improvement is claimed.
DecisionModelnames the capability rather than the RLCD training method. State transitions compose with ordinary Effect code; introducing a second state-machine runtime or tool executor would duplicate existing ownership and recovery contracts. The optional decision helper is separate from the base engine import path, and no persisted format changes.An opt-in live consumer benchmark compares 50 exposed tools, eight application defaults plus discovery, and eight tools selected by JEV plus discovery, using
gpt-6-astraand identical synthetic tasks. It retains per-request exposure, token/cache usage, selection time, and independently checked tool evidence. JEV selects once before the first turn in this benchmark; later turns preserve discovery selections.