Repository navigation
Conversation
yuluo-yx
left a comment
There was a problem hiding this comment.
The dispatch boundary and tool-history validation need tightening.
…ordering (issue agentic-ai-java#137 review)
|
Thanks for the review — both points are addressed (b5ad401, +96/−17):
On terminal enforcement: making the budget non-bypassable at the runtime dispatch belongs to the lifecycle execution chain (issue #299's "non-bypassable runtime checks") — happy to take that as a follow-up once the contract PR lands. |
|
[P2] The budget gate can dispatch a request whose requested output alone exceeds the context window.
Please validate the actual output limit against the reserved budget, account for it in the estimate, or clamp it before dispatch. Otherwise, even with the interceptor registered last, an oversized request still reaches the provider. Validation: targeted offline Java probe against this PR head; the full reactor test suite was not run. |
…dget estimate (issue agentic-ai-java#137 review)
|
Fixed in 140693b (+85/−1). The estimator now treats an explicit Three new cases in |
|
[P2] Dynamic tool schemas ignore explicit tool selection (line 134). Reproduced: |
|
Fixed in 7e1033b (+93/−12). One deliberate scope adjustment in the same area, stated explicitly: tool descriptions are no longer counted at all. They never enter the dispatched Three new cases in |
Describe what this PR does / why we need it
Fixes #137: the neutral agent API had no notion of a context window budget.
TokenCounter.countTokens(List<ChatMessage>)only measures the conversation, while the request which is finally dispatched carries more than the conversation -ReActAgent.invokeModelbuildsnew ChatRequest(composeMessages(request), effective.definitions(), request.getOptions(), request.getAttributes()), so the system message, the tool schemas and the reserved output belong to the request but stay outside the measured scope.SummarizationHookandContextEditingInterceptoreach trigger on their own message threshold and both continue when they cannot compress anything, so nothing keeps an oversized request from reaching the provider, andLargeResultEvictionInterceptorclaimed that a result was written to the filesystem even when the write failed.Does this pull request fix one issue?
Fixes #137
Describe how you did it
io.github.agentic.ai.agent.budgetinargi-agent-api:ContextWindowBudgetfor the context window and the reserved output,RequestTokenEstimatefor the decomposition into system, conversation, tool schema and reserved output with anEstimationAccuracythat tells exact and conservative counts apart,RequestTokenEstimatorwith the defaultApproximateRequestTokenEstimator,BudgetComponentandBudgetViolationfor a stable classification, andContextWindowBudgetExceededExceptionfor an explicit failure;ContextBudgetInterceptor, a neutralModelInterceptorwhich is opt-in through the existing interceptor chain: it estimates the request, asks the configuredRequestCompactorto shrink an oversized request, measures the compacted request again, validates the tool call pairing and only then dispatches; a request which still does not fit fails withContextWindowBudgetExceededExceptionand never reaches the provider;SummarizationRequestCompactordelegates to the existingSummarizationHookand rebuilds the request from the messages the hook returned; the hook keeps its own behaviour, so a failed summary returns the original history and the gate then fails explicitly instead of continuing with a broken context;argi-apiandargi-runtime-apionly.How was it tested / verified?
New
ContextBudgetInterceptorTestinargi-agent-api(offline, no model and no provider):New
SummarizationRequestCompactorTestand two additional cases inLargeResultEvictionInterceptorTestinargi-agent-framework: the compactor summarizes with the existing hook and returns the original request when the summary fails or is not needed, and a failed filesystem write produces a failure which contains no file pointer and no "saved in the filesystem" wording while keeping the metadata of the original response.Green after the change:
argi-agent-api41/41,argi-agent-framework1341/1341,argi-agent-core138/138,argi-workflow-graph-core551/551;spring-javaformat:validateandcheckstyle:checkreport no violation for the two touched modules,codespelland the newline check pass.Additional notes
argi-agent-api, one new adapter and one behavioural fix inargi-agent-framework, and no change to an existing public signature, event, configuration or persistence format;LargeResultEvictionInterceptoralways supplies a default filesystem backend when eviction is enabled, so the branch for a missing backend is defensive only and the test covers the failed write path.