Refine config docs and fix matcher required param detection#29
Merged
JohnRichard4096 merged 2 commits intomainfrom Mar 16, 2026
Merged
Refine config docs and fix matcher required param detection#29JohnRichard4096 merged 2 commits intomainfrom
JohnRichard4096 merged 2 commits intomainfrom
Conversation
Contributor
Reviewer's GuideUpdates configuration documentation to introduce BuiltinAgentConfig, relocate tool_calling_mode and agent_thought_mode settings from FunctionConfig to BuiltinAgentConfig examples in both English and Chinese docs, refines wording throughout the docs, and fixes a dependency resolution bug in the hook matcher by computing required parameters from already-filtered parameters. Class diagram for AmritaConfig and BuiltinAgentConfig configuration splitclassDiagram
class AmritaConfig {
+FunctionConfig function_config
+LLMConfig llm
+CookieConfig cookie
+BuiltinAgentConfig builtin
}
class FunctionConfig {
+bool use_minimal_context
+bool agent_mcp_client_enable
}
class BuiltinAgentConfig {
+string tool_calling_mode
+string agent_thought_mode
}
class LLMConfig {
+bool enable_memory_abstract
+float memory_abstract_proportion
+int max_tokens
+int llm_timeout
+bool auto_retry
+int max_retries
+int memory_length_limit
}
class CookieConfig {
+bool enable_cookie
+string cookie
}
AmritaConfig *-- FunctionConfig : composes
AmritaConfig *-- LLMConfig : composes
AmritaConfig *-- CookieConfig : composes
AmritaConfig *-- BuiltinAgentConfig : composes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
docs/guide/api-reference/classes/AmritaConfig.md, the import line usesBuiltinAgnentConfigwhile the example below usesBuiltinAgentConfig; align the name to the actual exported class to avoid confusion and import errors. - In the
BuiltinAgentConfigsections (both EN and ZH docs), the lists of string options fortool_calling_modeandagent_thought_modeuse unbalanced quotes (e.g.,"agentinstead of"agent"); fix these so the option names render correctly and can be copy-pasted safely. - The new
BuiltinAgentConfigexamples still use variable names likefunc_config_agent/func_config_rag, which previously referred toFunctionConfig; consider renaming them (e.g.,builtin_config_agent) to clearly distinguish built-in agent policy settings from function configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `docs/guide/api-reference/classes/AmritaConfig.md`, the import line uses `BuiltinAgnentConfig` while the example below uses `BuiltinAgentConfig`; align the name to the actual exported class to avoid confusion and import errors.
- In the `BuiltinAgentConfig` sections (both EN and ZH docs), the lists of string options for `tool_calling_mode` and `agent_thought_mode` use unbalanced quotes (e.g., ``"agent`` instead of ``"agent"``); fix these so the option names render correctly and can be copy-pasted safely.
- The new `BuiltinAgentConfig` examples still use variable names like `func_config_agent`/`func_config_rag`, which previously referred to `FunctionConfig`; consider renaming them (e.g., `builtin_config_agent`) to clearly distinguish built-in agent policy settings from function configuration.
## Individual Comments
### Comment 1
<location path="docs/guide/api-reference/classes/AmritaConfig.md" line_range="14" />
<code_context>
```python
-from amrita_core.config import AmritaConfig, FunctionConfig, LLMConfig, CookieConfig
+from amrita_core.config import AmritaConfig, FunctionConfig, LLMConfig, CookieConfig, BuiltinAgnentConfig
config = AmritaConfig(
</code_context>
<issue_to_address>
**issue (typo):** Fix the typo in the imported `BuiltinAgentConfig` class name.
The import currently uses `BuiltinAgnentConfig`, but the correct class name (as used in the docs and the `builtin=` argument) is `BuiltinAgentConfig`. Please update the import to use the correct spelling.
```suggestion
from amrita_core.config import AmritaConfig, FunctionConfig, LLMConfig, CookieConfig, BuiltinAgentConfig
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Member
Author
|
@sourcery-ai title |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Update configuration and advanced usage documentation to introduce BuiltinAgentConfig and clarify option responsibilities, and fix dependency resolution for required parameters in the hook matcher.
Bug Fixes:
Enhancements:
Documentation: