wenjin272 commented on code in PR #1259:
URL: https://github.com/apache/flink-agents/pull/1259#discussion_r4282686019
##########
docs/content/docs/development/react_agent.md:
##########
@@ -106,9 +110,9 @@ ReActAgent myReActAgent =
## Initialize Arguments
### Chat Model
-User should specify the chat model used in the ReAct Agent.
+Use a `ResourceDescriptor` to select the required chat model setup and pass
its arguments. See [Chat Models]({{< ref
"docs/development/calling-llms/chat-models" >}}) for the provider-specific
connection and model configuration.
Review Comment:
Could we move the tool-registration warning into the main text and present
the setup as two steps: first register the tool on
`AgentsExecutionEnvironment`, then reference its registered name in the chat
model descriptor’s `tools` argument? This is the normal usage flow and would be
clearer as guidance alongside the examples than as a warning.
##########
docs/content/docs/development/react_agent.md:
##########
@@ -253,11 +260,11 @@ new ChatMessage(MessageRole.USER, "{input}")
See [Prompt]({{< ref "docs/development/calling-llms/prompts" >}}) for more
details.
### Output Schema
-User can set output schema to configure the ReAct Agent output type. If output
schema is set, the ReAct Agent will deserialize the llm response to expected
type.
+The output schema is optional. Without one, the agent does not add a schema
instruction, parse or validate the final response against a schema, or perform
type conversion. It emits the text of the final assistant message as a `String`
(Java) or `str` (Python). This text is the concatenation of the message's
top-level text blocks; media, reasoning, and tool-call blocks are not included.
Review Comment:
Could we keep this focused on the default output type? The text-block
details seem unnecessary here.
```suggestion
The output schema is optional. If omitted, the agent returns the text of the
final assistant response as a `String` (Java) or `str` (Python).
```
##########
docs/content/docs/development/react_agent.md:
##########
@@ -46,6 +46,7 @@ from flink_agents.api.chat_message import ChatMessage,
MessageRole
from flink_agents.api.execution_environment import AgentsExecutionEnvironment
from flink_agents.api.prompts.prompt import Prompt
from flink_agents.api.resource import ResourceDescriptor, ResourceName,
ResourceType
+from flink_agents.api.skills import Skills
Review Comment:
Could we remove the shared imports section? These snippets focus on
individual concepts, and the page already links to complete runnable examples.
Any imports needed for clarity could stay with the relevant snippet.
##########
docs/content/docs/development/react_agent.md:
##########
@@ -333,7 +340,7 @@ RowTypeInfo myRowTypeInfo =
{{< /tabs >}}
### Skills
-User can pass [skills]({{< ref "docs/development/calling-llms/skills" >}}) to
the constructor instead of adding a `skills` argument to the chat model
descriptor. Two forms are accepted: a list of skill names, or a `Skills` object
declaring where the skills are loaded from.
+Skills are optional. You can pass [skills]({{< ref
"docs/development/calling-llms/skills" >}}) to the constructor instead of
adding a `skills` argument to the chat model descriptor. Two forms are
accepted: a list of skill names, or a `Skills` object declaring where the
skills are loaded from.
Review Comment:
Could we also simplify the [skills-merging paragraph and list
below](https://github.com/apache/flink-agents/blob/184dcddba3b3d836c2552d2d77b591d5bd6bf57d/docs/content/docs/development/react_agent.md#L368-L374)
to focus on configuration behavior? Details such as copying the descriptor
seem unnecessary for this guide. Suggested replacement:
> Skills passed to the constructor are combined with those configured on the
chat model descriptor, with duplicate names included only once. Referenced
skills must be declared on the agent or its execution environment.
>
> Configure `allowed_commands` on the chat model descriptor to control which
commands the built-in `bash` tool may execute.
##########
docs/content/docs/development/react_agent.md:
##########
@@ -78,14 +80,16 @@ import java.util.List;
## ReAct Agent Example
Review Comment:
Could we make this section show the minimal setup first? It currently passes
three variables that are only explained later, so it reads more like a
constructor reference than an example. Renaming it to “Create a ReAct Agent”
and starting with only the required chat model would make the entry point
clearer; the following sections can then introduce the optional configuration.
##########
docs/content/docs/development/react_agent.md:
##########
@@ -333,7 +340,7 @@ RowTypeInfo myRowTypeInfo =
{{< /tabs >}}
### Skills
-User can pass [skills]({{< ref "docs/development/calling-llms/skills" >}}) to
the constructor instead of adding a `skills` argument to the chat model
descriptor. Two forms are accepted: a list of skill names, or a `Skills` object
declaring where the skills are loaded from.
+Skills are optional. You can pass [skills]({{< ref
"docs/development/calling-llms/skills" >}}) to the constructor instead of
adding a `skills` argument to the chat model descriptor. Two forms are
accepted: a list of skill names, or a `Skills` object declaring where the
skills are loaded from.
**Skill names.** Pass the names of skills that are already declared to the
agent:
Review Comment:
Could we show how to register skills on `AgentsExecutionEnvironment` before
referencing them by name, similar to the tools section? This would also make
the second form easier to follow: passing a `Skills` object directly to the
constructor removes the need for separate registration.
--
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]
