Copilot commented on code in PR #1533:
URL: https://github.com/apache/dubbo-admin/pull/1533#discussion_r3817917917
##########
ai/component/server/engine/models.go:
##########
@@ -37,9 +36,8 @@ func NewErrorResponse(message string) *Response {
// ChatRequest defines streaming chat request
type ChatRequest struct {
- Message string `json:"message" binding:"required"` // User
message
- SessionID string `json:"sessionID" binding:"required"` //
Session ID
- Context json.RawMessage `json:"context,omitempty"` //
Current-turn page context
+ Message string `json:"message" binding:"required"` // User message
+ SessionID string `json:"sessionID" binding:"required"` // Session ID
Review Comment:
The backend now silently drops the page context that the current frontend
still sends. `ui-vue3/src/api/service/ai.ts:68-72` includes `context` in every
selected-context request, and its unit test explicitly verifies that contract;
with this field and the handler plumbing removed, those requests still succeed
but the agent loses the user's selected page state. Preserve the context
contract, or update/remove the frontend feature in the same change rather than
silently ignoring it.
##########
ai/component/models/models.yaml:
##########
@@ -1,18 +1,12 @@
type: models
spec:
- default_model: "dashscope/qwen3.7-max"
+ default_model: "dashscope/qwen-max"
Review Comment:
This also changes the configured production/default model family
(`models.yaml` switches from qwen3.7-max to qwen-max, while `agent.yaml`
switches to qwen3.5-plus) and removes the qwen3.7 registrations. That migration
is independent of the described ReAct/config-shape refactor and can materially
change answer quality, latency, and cost. Retain the existing models, or
explicitly document and verify the model migration as part of this PR.
##########
ai/component/agent/react/steps.go:
##########
@@ -34,248 +32,123 @@ import (
"github.com/firebase/genkit/go/ai"
)
-// buildSteps materializes the step closures for one interaction, binding the
-// per-interaction channels so progress/streaming reaches the right consumer.
-func (ra *ReActAgent) buildSteps(chans *agent.Channels) []step {
- steps := make([]step, 0, len(ra.stages))
- for _, st := range ra.stages {
- switch st.kind {
- case flowReasonAct:
- steps = append(steps, ra.reasonActStep(st.prompt,
chans, st.timeout))
- case flowObserve:
- steps = append(steps, ra.observeStep(st.prompt, chans,
st.timeout))
- }
- }
- return steps
-}
-
-// historyFromCtx pulls the session-scoped history out of ctx, replacing the
-// pointer/value assertion churn the old flows repeated at every stage.
-func historyFromCtx(ctx context.Context) (*memory.HistoryMemory, string,
error) {
- history, ok := ctx.Value(memory.ChatHistoryKey).(*memory.HistoryMemory)
- if !ok {
- return nil, "", fmt.Errorf("failed to get history from context")
- }
- sessionID, ok := ctx.Value(memory.SessionIDKey).(string)
- if !ok || sessionID == "" {
- return nil, "", fmt.Errorf("session id not found in context")
- }
- return history, sessionID, nil
-}
-
-// reasonActStep merges the old think + act stages: one model call reasons
about
-// the request and, via native function calling, either issues tool requests
-// (which it executes) or issues none (answering directly). The observe stage
-// then composes the reply, so this step never terminates the loop.
-func (ra *ReActAgent) reasonActStep(prompt ai.Prompt, chans *agent.Channels,
timeout time.Duration) step {
- return func(ctx context.Context, s *state) (bool, error) {
- emitStageProgress(chans, flowReasonAct, true)
- defer emitStageProgress(chans, flowReasonAct, false)
-
- history, sessionID, err := historyFromCtx(ctx)
- if err != nil {
- return false, err
- }
- if history.IsEmpty(sessionID) {
- return false, fmt.Errorf("history is empty")
- }
- messages, err := injectCurrentPageContext(ctx,
history.WindowMemory(sessionID))
- if err != nil {
- return false, err
+// run drives the reason-and-act loop for one interaction. Each iteration is a
+// single model call: with native function calling the model either requests
+// tools (whose results are fed back as context for the next iteration) or
+// answers directly — a tool-free response IS the final answer, so no separate
+// "observe" reasoning step is needed to decide when to stop. The last allowed
+// iteration uses the tool-less answer prompt so the loop always terminates
with
+// a real answer rather than an exhausted-budget silence.
+//
+// run streams the answer itself and returns the interaction's accumulated
token
+// usage; the caller emits the final usage marker and closes the channels.
+func (ra *ReActAgent) run(ctx context.Context, chans *agent.Channels)
(*ai.GenerationUsage, error) {
+ history, sessionID, err := historyFromCtx(ctx)
+ if err != nil {
+ return nil, err
+ }
+ if history.IsEmpty(sessionID) {
+ return nil, fmt.Errorf("history is empty")
+ }
+
+ usage := &ai.GenerationUsage{}
+ for i := 0; i < ra.maxIterations; i++ {
+ // The final iteration must answer: drop the tools so the model
can only
+ // synthesize from what it has already gathered.
+ forceAnswer := i == ra.maxIterations-1
+ prompt := ra.actPrompt
+ if forceAnswer {
+ prompt = ra.answerPrompt
}
- // Only the model call is bound by the stage timeout; tool
execution below
- // runs on the original ctx so a slow reasoning step can't
starve the tools
- // it just asked for (which would otherwise fail hard on the
shared deadline).
- lctx, cancel := withTimeout(ctx, timeout)
- resp, err := prompt.Execute(lctx, ai.WithMessages(messages...))
+ // Only the model call is bound by the per-call timeout; tool
execution
+ // below runs on the original ctx so a slow reasoning step
can't starve the
+ // tools it just asked for on a shared deadline.
+ lctx, cancel := withTimeout(ctx, ra.callTimeout)
+ resp, err := prompt.Execute(lctx,
ai.WithMessages(history.WindowMemory(sessionID)...))
cancel()
if err != nil {
- return false, fmt.Errorf("failed to execute reasonAct
prompt: %w", err)
+ return usage, fmt.Errorf("failed to execute react
prompt: %w", err)
}
- s.addUsage(resp.Usage)
-
- toolReqs := resp.ToolRequests()
- runtime.GetLogger().Info("tool requests:", "req", toolReqs)
-
- // No tools needed: the model answered directly. Record its
reasoning so
- // the observe stage can build on it, and leave tool outputs
empty.
- if len(toolReqs) == 0 {
- if text := resp.Text(); text != "" {
- history.AddHistory(sessionID,
ai.NewMessage(ai.RoleModel, nil, ai.NewTextPart(text)))
- }
- s.Tools = &schema.ToolOutputs{UsageInfo:
&ai.GenerationUsage{}}
- return false, nil
- }
-
- var parts []*ai.Part
- actOuts := &schema.ToolOutputs{UsageInfo: &ai.GenerationUsage{}}
- for _, req := range toolReqs {
- // Each tool runs under its own timeout (per-tool
override, else the
- // shared default), independent of the model call's
budget above.
- tctx, cancel := withTimeout(ctx,
ra.toolTimeouts.For(req.Name))
- output, err := toolEngine.Call(tctx, ra.registry,
req.Name, req.Input)
- cancel()
- if err != nil {
- // Degrade instead of aborting: record the
failure as a tool output
- // so the observe stage can still compose an
answer (or explain the
- // gap) from whatever other tools returned.
- runtime.GetLogger().Warn("tool call failed,
continuing with degraded context",
- "tool", req.Name, "error", err)
- output = toolEngine.ToolOutput{
- ToolName: req.Name,
- Summary: fmt.Sprintf("tool %q failed:
%v", req.Name, err),
+ schema.AccumulateUsage(usage, resp.Usage)
+
+ if !forceAnswer {
+ if reqs := resp.ToolRequests(); len(reqs) > 0 {
+ runtime.GetLogger().Debug("react: model
requested tools", "count", len(reqs))
+ agent.EmitProgress(chans, "🔍 分析问题并调用工具中...\n")
+ if err := ra.execTools(ctx, history, sessionID,
reqs); err != nil {
+ return usage, err
}
+ continue
}
- outputJson, err := json.Marshal(output)
- if err != nil {
- return false, fmt.Errorf("failed to marshal
output: %w", err)
- }
- parts = append(parts,
ai.NewJSONPart(string(outputJson)))
- actOuts.Add(&output)
}
- runtime.GetLogger().Info("act out:", "out", actOuts)
- // ai.RoleTool's messages will be ignored by ai.WithMessages
- history.AddHistory(sessionID, ai.NewMessage(ai.RoleModel, nil,
parts...))
- s.Tools = actOuts
- return false, nil
+
+ ra.finish(chans, history, sessionID, resp.Text())
+ return usage, nil
Review Comment:
A successful model response with neither tool requests nor text is treated
as a completed answer here. `finish` deliberately emits no content when `answer
== ""`, so both an early tool-free response and the forced final iteration can
end with only stream markers, contrary to the promised real reply. Retry empty
responses while budget remains and handle an empty forced answer with an
explicit fallback or error.
##########
ai/component/agent/react/steps.go:
##########
@@ -34,248 +32,123 @@ import (
"github.com/firebase/genkit/go/ai"
)
-// buildSteps materializes the step closures for one interaction, binding the
-// per-interaction channels so progress/streaming reaches the right consumer.
-func (ra *ReActAgent) buildSteps(chans *agent.Channels) []step {
- steps := make([]step, 0, len(ra.stages))
- for _, st := range ra.stages {
- switch st.kind {
- case flowReasonAct:
- steps = append(steps, ra.reasonActStep(st.prompt,
chans, st.timeout))
- case flowObserve:
- steps = append(steps, ra.observeStep(st.prompt, chans,
st.timeout))
- }
- }
- return steps
-}
-
-// historyFromCtx pulls the session-scoped history out of ctx, replacing the
-// pointer/value assertion churn the old flows repeated at every stage.
-func historyFromCtx(ctx context.Context) (*memory.HistoryMemory, string,
error) {
- history, ok := ctx.Value(memory.ChatHistoryKey).(*memory.HistoryMemory)
- if !ok {
- return nil, "", fmt.Errorf("failed to get history from context")
- }
- sessionID, ok := ctx.Value(memory.SessionIDKey).(string)
- if !ok || sessionID == "" {
- return nil, "", fmt.Errorf("session id not found in context")
- }
- return history, sessionID, nil
-}
-
-// reasonActStep merges the old think + act stages: one model call reasons
about
-// the request and, via native function calling, either issues tool requests
-// (which it executes) or issues none (answering directly). The observe stage
-// then composes the reply, so this step never terminates the loop.
-func (ra *ReActAgent) reasonActStep(prompt ai.Prompt, chans *agent.Channels,
timeout time.Duration) step {
- return func(ctx context.Context, s *state) (bool, error) {
- emitStageProgress(chans, flowReasonAct, true)
- defer emitStageProgress(chans, flowReasonAct, false)
-
- history, sessionID, err := historyFromCtx(ctx)
- if err != nil {
- return false, err
- }
- if history.IsEmpty(sessionID) {
- return false, fmt.Errorf("history is empty")
- }
- messages, err := injectCurrentPageContext(ctx,
history.WindowMemory(sessionID))
- if err != nil {
- return false, err
+// run drives the reason-and-act loop for one interaction. Each iteration is a
+// single model call: with native function calling the model either requests
+// tools (whose results are fed back as context for the next iteration) or
+// answers directly — a tool-free response IS the final answer, so no separate
+// "observe" reasoning step is needed to decide when to stop. The last allowed
+// iteration uses the tool-less answer prompt so the loop always terminates
with
+// a real answer rather than an exhausted-budget silence.
+//
+// run streams the answer itself and returns the interaction's accumulated
token
+// usage; the caller emits the final usage marker and closes the channels.
+func (ra *ReActAgent) run(ctx context.Context, chans *agent.Channels)
(*ai.GenerationUsage, error) {
+ history, sessionID, err := historyFromCtx(ctx)
+ if err != nil {
+ return nil, err
+ }
+ if history.IsEmpty(sessionID) {
+ return nil, fmt.Errorf("history is empty")
+ }
+
+ usage := &ai.GenerationUsage{}
+ for i := 0; i < ra.maxIterations; i++ {
+ // The final iteration must answer: drop the tools so the model
can only
+ // synthesize from what it has already gathered.
+ forceAnswer := i == ra.maxIterations-1
+ prompt := ra.actPrompt
+ if forceAnswer {
+ prompt = ra.answerPrompt
Review Comment:
The core guarantee that the last iteration uses the tool-less answer prompt
is not actually covered: `scriptPrompt` assigns the same instance to
`actPrompt` and `answerPrompt`, so the tests pass even if this branch selects
the wrong prompt. Use distinct prompt spies and assert that the final call
reaches only the answer prompt after the tool-round budget is consumed.
##########
ai/component/server/engine/docs/openapi.yaml:
##########
@@ -23,30 +23,10 @@ paths:
application/json:
schema:
type: object
- required:
- - message
- - sessionID
- properties:
- message:
- type: string
- sessionID:
- type: string
- context:
- $ref: "#/components/schemas/AIContextSnapshot"
- additionalProperties: false
+ properties: {}
Review Comment:
The request schema now documents an unconstrained empty object even though
`StreamChat` still requires `message` and `sessionID`, and the example
immediately below uses both. Generated clients and validators will therefore
omit required inputs. Keep those two required properties in the OpenAPI schema
even if `context` is intentionally removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
