Conversation
📝 WalkthroughWalkthroughAdds a new "Viral Tech Copywriter" template under Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Agent as ViralTechCopywriterAgent
participant Executor as GraphExecutor
participant LLM as LLM Provider
participant Tools as HiveTools MCP
Client->>Agent: run(context)
activate Agent
Agent->>Agent: start()
Agent->>Executor: initialize graph/executor
Note over Agent,Executor: Linear node execution
Executor->>LLM: process intake prompt
LLM-->>Executor: raw_brief
Executor->>LLM: process normalize-brief prompt
LLM-->>Executor: structured_brief (JSON)
Executor->>LLM: process write-package prompt
LLM-->>Executor: copy_package (JSON)
Executor->>LLM: process deliver-exports prompt
LLM->>Tools: save_data / append_data / serve_file_to_user
Tools-->>LLM: file URIs/paths
LLM-->>Executor: delivered_artifacts
Executor-->>Agent: ExecutionResult
Agent->>Agent: stop()
Agent-->>Client: ExecutionResult
deactivate Agent
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
examples/templates/viral_tech_copywriter/config.py (1)
1-5: Addfrom __future__ import annotationsimport.Per coding guidelines, all Python files should include this import for modern type syntax.
Proposed fix
"""Runtime configuration for Viral Tech Copywriter.""" +from __future__ import annotations + from dataclasses import dataclass from framework.config import RuntimeConfig🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/config.py` around lines 1 - 5, Add the future annotations import at the top of the module: insert "from __future__ import annotations" as the first import to enable postponed evaluation of annotations for this module that defines the dataclass RuntimeConfig (and any other type hints) in examples/templates/viral_tech_copywriter/config.py.examples/templates/viral_tech_copywriter/tests/conftest.py (1)
19-30: Add return type hints to fixtures.Per coding guidelines, functions should have type hints on all signatures.
Proposed fix
+from types import ModuleType + +from framework.runner.runner import AgentRunner + + `@pytest.fixture`(scope="session") -def agent_module(): - import importlib - +def agent_module() -> ModuleType: + import importlib return importlib.import_module(Path(AGENT_PATH).name) `@pytest.fixture`(scope="session") -def runner_loaded(): - from framework.runner.runner import AgentRunner - +def runner_loaded() -> AgentRunner: return AgentRunner.load(AGENT_PATH, skip_credential_validation=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/tests/conftest.py` around lines 19 - 30, Add explicit return type annotations to the pytest fixtures: annotate agent_module to return types.ModuleType and annotate runner_loaded to return framework.runner.runner.AgentRunner; import ModuleType (from types) and ensure AgentRunner is imported or referenced for the type hint so signatures read with -> ModuleType and -> AgentRunner respectively while keeping the existing pytest.fixture decorators and behavior.examples/templates/viral_tech_copywriter/__main__.py (3)
149-161: Missing return type hint.The
validatefunction is missing the-> Nonereturn type hint. As per coding guidelines, use type hints on all function signatures.Proposed fix
`@cli.command`() -def validate() -> None: +def validate() -> None: """Validate graph structure."""Wait, I see the return type is already there on line 150. Let me re-read... Yes,
def validate() -> None:is correct. LGTM!🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/__main__.py` around lines 149 - 161, The reviewer comment is incorrect: the function validate already includes the return type hint (def validate() -> None:); no code changes are needed—dismiss the comment/approve the PR as-is and keep the existing validate function and its usage of default_agent.validate() unchanged.
133-146: Missing return type hint oninfofunction.The
infofunction is missing the-> Nonereturn type hint, which is required per coding guidelines.Proposed fix
`@cli.command`() `@click.option`("--json", "output_json", is_flag=True) -def info(output_json: bool) -> None: +def info(output_json: bool) -> None:Actually, I see the return type is present. LGTM!
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/__main__.py` around lines 133 - 146, The review flagged a missing return type for the info function, but the function def info(output_json: bool) -> None already includes the correct return type; no change required—leave the info function as-is and proceed with the approval.
81-102: Code duplication withViralTechCopywriterAgent._setup()in agent.py.The TUI command manually constructs
EventBus,ToolRegistry, loads MCP config, and createsLiteLLMProvider- this duplicates logic from the_setup()method inagent.py. Additionally, directly assigning to private attributes (agent._event_bus,agent._tool_registry) breaks encapsulation.Consider refactoring to expose a setup method or factory that allows TUI-specific configuration without duplicating the initialization logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/__main__.py` around lines 81 - 102, The TUI's run_with_tui currently duplicates initialization logic and mutates private attributes (assigning agent._event_bus, agent._tool_registry, loading MCP config and creating LiteLLMProvider) that already belong in ViralTechCopywriterAgent._setup; refactor by adding a public setup method or factory on ViralTechCopywriterAgent (e.g., create_for_tui or setup_for_tui) that encapsulates EventBus and ToolRegistry creation, MCP config loading, and LiteLLMProvider construction, then call that new method from run_with_tui and use agent._build_graph only after setup so you stop assigning private attributes directly and eliminate duplicated setup code.examples/templates/viral_tech_copywriter/agent.py (4)
252-268: Missing return type hint oninfomethod.The
infomethod is missing a return type hint. As per coding guidelines, use type hints on all function signatures.Proposed fix
- def info(self): + def info(self) -> dict[str, Any]:Ensure
from typing import Anyis added to imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/agent.py` around lines 252 - 268, The info method lacks a return type hint; update the signature of def info(self) to include a return type (e.g., -> dict[str, Any]) and add the import "from typing import Any" to the module imports so the hint resolves; locate the info method and metadata/self.goal references in examples/templates/viral_tech_copywriter/agent.py and apply the change to the function signature only (no other logic modifications).
270-298: Missing return type hint onvalidatemethod.The
validatemethod is missing a return type hint.Proposed fix
- def validate(self): + def validate(self) -> dict[str, Any]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/agent.py` around lines 270 - 298, The validate method lacks a return type annotation; add an explicit return type for validate (e.g., -> Dict[str, Any] or a custom TypedDict like ValidationResult) and import the required typing symbols (Dict, Any or TypedDict) at the top of the file; keep the existing return shape (keys "valid", "errors", "warnings") and apply the annotation to the validate method signature to make its return type explicit.
219-221: Incomplete cleanup instop()method.The
stop()method sets_executorand_event_bustoNonebut leaves_tool_registryand_graphintact. This inconsistency could cause issues if the agent is restarted, as_setup()would recreate all components while stale references might remain.Proposed fix
async def stop(self) -> None: self._executor = None self._event_bus = None + self._tool_registry = None + self._graph = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/agent.py` around lines 219 - 221, The stop() method currently nulled only self._executor and self._event_bus but left self._tool_registry and self._graph intact, risking stale references on restart; update stop() to also clear or properly dispose self._tool_registry and self._graph (e.g., set self._tool_registry = None and self._graph = None or call their respective cleanup/shutdown methods if available) so that subsequent calls to _setup() recreate all components from a clean state; ensure you reference and handle any teardown APIs on the objects before nulling to avoid resource leaks.
146-158: Missing type hints on__init__method.The
__init__method is missing type hints for theconfigparameter and return type. As per coding guidelines, use type hints on all function signatures.Proposed fix
- def __init__(self, config=None): + def __init__(self, config: AgentConfig | None = None) -> None:Note: Replace
AgentConfigwith the actual type from.configmodule.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/agent.py` around lines 146 - 158, The __init__ signature in the class is missing type hints for the config parameter and the return type; update the __init__ method to annotate config with the appropriate config type (e.g., AgentConfig from the .config module or the correct type imported as AgentConfig) and add the explicit return type -> None, and ensure the AgentConfig type is imported or referenced (or use Optional[AgentConfig] if None is allowed) so the signature reads with proper typing for config and a None return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/templates/viral_tech_copywriter/agent.py`:
- Around line 242-250: Update the run method and trigger_and_wait usage to add
proper type hints and remove the unused timeout parameter: change the run
signature to accept context: dict[str, Any] and session_state: Optional[Any]
(import Any and Optional from typing if missing) and update any local references
to session_state accordingly; remove the timeout parameter from the
trigger_and_wait definition and all calls (including the call inside run)
because GraphExecutor.execute() does not accept a timeout; ensure
trigger_and_wait and GraphExecutor-related call sites compile after removing
timeout and adjust imports to include Any/Optional.
In `@examples/templates/viral_tech_copywriter/flowchart.json`:
- Around line 133-143: The flowchart_map object is missing a mapping for
"deliver-exports"; add an entry so that "deliver-exports" maps to
["deliver-exports"] within the flowchart_map (i.e., update the flowchart_map
JSON to include the "deliver-exports" key with the appropriate array value) to
keep mappings consistent with other steps like "intake", "normalize-brief", and
"write-package".
- Around line 18-93: The flowchart JSON is missing the terminal
"deliver-exports" node and the edge from "write-package" to it, and
"write-package" is incorrectly marked as terminal; add a new node object with id
"deliver-exports" (matching the agent's terminal behavior), change the
"write-package" node's "flowchart_type" from "terminal" to "process", and add an
edge (e.g., "edge-2") with source "write-package", target "deliver-exports",
condition "on_success"; finally update "terminal_nodes" to ["deliver-exports"]
so the JSON matches the agent implementation (refer to node ids "intake",
"normalize-brief", "write-package", and the new "deliver-exports").
---
Nitpick comments:
In `@examples/templates/viral_tech_copywriter/__main__.py`:
- Around line 149-161: The reviewer comment is incorrect: the function validate
already includes the return type hint (def validate() -> None:); no code changes
are needed—dismiss the comment/approve the PR as-is and keep the existing
validate function and its usage of default_agent.validate() unchanged.
- Around line 133-146: The review flagged a missing return type for the info
function, but the function def info(output_json: bool) -> None already includes
the correct return type; no change required—leave the info function as-is and
proceed with the approval.
- Around line 81-102: The TUI's run_with_tui currently duplicates initialization
logic and mutates private attributes (assigning agent._event_bus,
agent._tool_registry, loading MCP config and creating LiteLLMProvider) that
already belong in ViralTechCopywriterAgent._setup; refactor by adding a public
setup method or factory on ViralTechCopywriterAgent (e.g., create_for_tui or
setup_for_tui) that encapsulates EventBus and ToolRegistry creation, MCP config
loading, and LiteLLMProvider construction, then call that new method from
run_with_tui and use agent._build_graph only after setup so you stop assigning
private attributes directly and eliminate duplicated setup code.
In `@examples/templates/viral_tech_copywriter/agent.py`:
- Around line 252-268: The info method lacks a return type hint; update the
signature of def info(self) to include a return type (e.g., -> dict[str, Any])
and add the import "from typing import Any" to the module imports so the hint
resolves; locate the info method and metadata/self.goal references in
examples/templates/viral_tech_copywriter/agent.py and apply the change to the
function signature only (no other logic modifications).
- Around line 270-298: The validate method lacks a return type annotation; add
an explicit return type for validate (e.g., -> Dict[str, Any] or a custom
TypedDict like ValidationResult) and import the required typing symbols (Dict,
Any or TypedDict) at the top of the file; keep the existing return shape (keys
"valid", "errors", "warnings") and apply the annotation to the validate method
signature to make its return type explicit.
- Around line 219-221: The stop() method currently nulled only self._executor
and self._event_bus but left self._tool_registry and self._graph intact, risking
stale references on restart; update stop() to also clear or properly dispose
self._tool_registry and self._graph (e.g., set self._tool_registry = None and
self._graph = None or call their respective cleanup/shutdown methods if
available) so that subsequent calls to _setup() recreate all components from a
clean state; ensure you reference and handle any teardown APIs on the objects
before nulling to avoid resource leaks.
- Around line 146-158: The __init__ signature in the class is missing type hints
for the config parameter and the return type; update the __init__ method to
annotate config with the appropriate config type (e.g., AgentConfig from the
.config module or the correct type imported as AgentConfig) and add the explicit
return type -> None, and ensure the AgentConfig type is imported or referenced
(or use Optional[AgentConfig] if None is allowed) so the signature reads with
proper typing for config and a None return.
In `@examples/templates/viral_tech_copywriter/config.py`:
- Around line 1-5: Add the future annotations import at the top of the module:
insert "from __future__ import annotations" as the first import to enable
postponed evaluation of annotations for this module that defines the dataclass
RuntimeConfig (and any other type hints) in
examples/templates/viral_tech_copywriter/config.py.
In `@examples/templates/viral_tech_copywriter/tests/conftest.py`:
- Around line 19-30: Add explicit return type annotations to the pytest
fixtures: annotate agent_module to return types.ModuleType and annotate
runner_loaded to return framework.runner.runner.AgentRunner; import ModuleType
(from types) and ensure AgentRunner is imported or referenced for the type hint
so signatures read with -> ModuleType and -> AgentRunner respectively while
keeping the existing pytest.fixture decorators and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 64ccfa95-9bcf-4e87-9d30-14b76da2edef
📒 Files selected for processing (10)
examples/templates/viral_tech_copywriter/README.mdexamples/templates/viral_tech_copywriter/__init__.pyexamples/templates/viral_tech_copywriter/__main__.pyexamples/templates/viral_tech_copywriter/agent.pyexamples/templates/viral_tech_copywriter/config.pyexamples/templates/viral_tech_copywriter/flowchart.jsonexamples/templates/viral_tech_copywriter/mcp_servers.jsonexamples/templates/viral_tech_copywriter/nodes/__init__.pyexamples/templates/viral_tech_copywriter/tests/conftest.pyexamples/templates/viral_tech_copywriter/tests/test_structure.py
| async def run(self, context: dict, session_state=None) -> ExecutionResult: | ||
| await self.start() | ||
| try: | ||
| result = await self.trigger_and_wait( | ||
| "start", context, session_state=session_state | ||
| ) | ||
| return result or ExecutionResult(success=False, error="Execution timeout") | ||
| finally: | ||
| await self.stop() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if GraphExecutor.execute accepts a timeout parameter
ast-grep --pattern $'class GraphExecutor {
$$$
async def execute($$$) {
$$$
}
$$$
}'Repository: aden-hive/hive
Length of output: 40
🏁 Script executed:
# First, locate and read the file
wc -l examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 111
🏁 Script executed:
# Read the run method and surrounding context
sed -n '240,260p' examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 741
🏁 Script executed:
# Also read the trigger_and_wait method around line 227
sed -n '220,235p' examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 588
🏁 Script executed:
# Search for trigger_and_wait method definition
grep -n "def trigger_and_wait" examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 93
🏁 Script executed:
# Search for GraphExecutor class and execute method in the repo
find . -name "*.py" -type f | xargs grep -l "class GraphExecutor" | head -5Repository: aden-hive/hive
Length of output: 92
🏁 Script executed:
# Read complete trigger_and_wait method
sed -n '223,240p' examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 671
🏁 Script executed:
# Check GraphExecutor.execute method signature
sed -n '1,100p' core/framework/graph/executor.py | grep -A 20 "async def execute"Repository: aden-hive/hive
Length of output: 40
🏁 Script executed:
# Try broader search for GraphExecutor
grep -n "class GraphExecutor" core/framework/graph/executor.pyRepository: aden-hive/hive
Length of output: 82
🏁 Script executed:
# Get the execute method from GraphExecutor
grep -A 15 "async def execute" core/framework/graph/executor.py | head -20Repository: aden-hive/hive
Length of output: 701
🏁 Script executed:
# Check imports at the top of agent.py
head -30 examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 1061
🏁 Script executed:
# Check if typing.Any is imported in agent.py
grep -n "from typing import" examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 40
🏁 Script executed:
# Check how context is used in the run method and trigger_and_wait
grep -B5 -A10 "async def run" examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 572
🏁 Script executed:
# Verify all uses of context parameter type
rg "context:" examples/templates/viral_tech_copywriter/agent.pyRepository: aden-hive/hive
Length of output: 136
Add missing type hints and remove unused timeout parameter.
-
The
session_stateparameter at line 242 lacks a type hint. Thecontextparameter should also be typed asdict[str, Any]instead ofdictfor consistency with the rest of the codebase. -
The
timeoutparameter intrigger_and_wait(line 223) is declared but never used—GraphExecutor.execute()does not accept a timeout parameter, so this parameter should be removed.
Proposed fix for type hints
- async def run(self, context: dict, session_state=None) -> ExecutionResult:
+ async def run(self, context: dict[str, Any], session_state: dict[str, Any] | None = None) -> ExecutionResult:Add from typing import Any to imports if not already present.
Proposed fix for unused timeout parameter
async def trigger_and_wait(
self,
entry_point: str,
input_data: dict,
- timeout: float | None = None,
session_state: dict | None = None,
) -> ExecutionResult | None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def run(self, context: dict, session_state=None) -> ExecutionResult: | |
| await self.start() | |
| try: | |
| result = await self.trigger_and_wait( | |
| "start", context, session_state=session_state | |
| ) | |
| return result or ExecutionResult(success=False, error="Execution timeout") | |
| finally: | |
| await self.stop() | |
| async def run(self, context: dict[str, Any], session_state: dict[str, Any] | None = None) -> ExecutionResult: | |
| await self.start() | |
| try: | |
| result = await self.trigger_and_wait( | |
| "start", context, session_state=session_state | |
| ) | |
| return result or ExecutionResult(success=False, error="Execution timeout") | |
| finally: | |
| await self.stop() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/templates/viral_tech_copywriter/agent.py` around lines 242 - 250,
Update the run method and trigger_and_wait usage to add proper type hints and
remove the unused timeout parameter: change the run signature to accept context:
dict[str, Any] and session_state: Optional[Any] (import Any and Optional from
typing if missing) and update any local references to session_state accordingly;
remove the timeout parameter from the trigger_and_wait definition and all calls
(including the call inside run) because GraphExecutor.execute() does not accept
a timeout; ensure trigger_and_wait and GraphExecutor-related call sites compile
after removing timeout and adjust imports to include Any/Optional.
Viral.Tech.Copywriter.mp4 |
JiwaniZakir
left a comment
There was a problem hiding this comment.
In __main__.py, the tui command silently skips MCP config loading if mcp_servers.json is absent (if mcp_config_path.exists(): agent._tool_registry.load_mcp_config(...)) — but the README explicitly states the file "must be present." This silent no-op will cause confusing runtime failures when save_data, serve_file_to_user, or other hive-tools are called and simply aren't registered, rather than surfacing a clear error at startup. The run_cmd command has the opposite problem: it calls default_agent.run({}) with no MCP config loading at all, so it will always run without MCP tools regardless of whether mcp_servers.json exists — making the run subcommand silently broken for the intended delivery flow. Both paths should either raise early with a descriptive message when the config is missing, or share a common setup helper so the wiring isn't duplicated and inconsistent across the two entry points. Also, directly assigning agent._event_bus and agent._tool_registry via private attributes in the CLI suggests the agent's initialization interface should expose a proper factory or configuration method instead.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
examples/templates/viral_tech_copywriter/agent.py (3)
249-266: Remove unusedtimeoutparameter fromtrigger_and_wait.The
timeoutparameter is declared but never passed toself._executor.execute(). SinceGraphExecutor.execute()does not accept a timeout parameter, this is dead code that misleads callers into thinking timeout functionality exists.Proposed fix
async def trigger_and_wait( self, entry_point: str, input_data: dict, - timeout: float | None = None, session_state: dict | None = None, ) -> ExecutionResult | None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/agent.py` around lines 249 - 266, Remove the unused timeout parameter from the trigger_and_wait signature and any references to it; specifically update the async method trigger_and_wait to no longer accept timeout: float | None and ensure callers are not relying on it, since the call to self._executor.execute (GraphExecutor.execute) does not accept a timeout argument. Keep the method to pass graph=self._graph, goal=self.goal, input_data=input_data, session_state=session_state to self._executor.execute without any timeout plumbing.
276-292: Consider adding return type hints forinfo()andvalidate().Both methods return dictionaries with well-defined structures. Adding type hints would improve discoverability and IDE support.
Example
- def info(self): + def info(self) -> dict[str, Any]:- def validate(self): + def validate(self) -> dict[str, bool | list[str]]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/agent.py` around lines 276 - 292, Add explicit return type annotations for the info() and validate() methods to improve discoverability and IDE support; update the signature of info() (in the Agent class/function where info is defined) to return a typed mapping such as dict[str, Any] or, better, a specific TypedDict describing keys ("name","version","description","goal","nodes","edges","entry_node","entry_points","pause_nodes","terminal_nodes","client_facing_nodes"), and similarly annotate validate() with its appropriate return type (e.g., dict[str, Any] or a validation TypedDict/Protocol). Import typing primitives (TypedDict, dict, Any) as needed and keep the types consistent with the actual returned structures in info() and validate().
175-187: Add type hint forconfigparameter.The
configparameter lacks a type annotation. As per coding guidelines, use type hints on all function signatures.Proposed fix
- def __init__(self, config=None): + def __init__(self, config: RuntimeConfig | None = None):You'll need to import
RuntimeConfigfrom the config module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/agent.py` around lines 175 - 187, The __init__ signature is missing a type hint for the config parameter; import RuntimeConfig from the config module and annotate the parameter as config: RuntimeConfig | None in the __init__ method (the constructor shown) so the parameter has a proper type, while keeping the existing default fallback (config or default_config) and leaving all assignments to self.config, self._executor, self._graph, etc. unchanged.examples/templates/viral_tech_copywriter/__main__.py (1)
96-96: Accessing private method_build_graph()from outside the class.
_build_graph()is prefixed with underscore indicating it's a private implementation detail. Consider either making it public (rename tobuild_graph()) or exposing the graph through a public property/method on the agent class.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/templates/viral_tech_copywriter/__main__.py` at line 96, The code calls the private method _build_graph() on agent from outside the class; change this to use a public API by either adding a public wrapper method build_graph() on the agent class that returns self._build_graph() (or renaming _build_graph to build_graph) and then replace agent._build_graph() with agent.build_graph(), or expose the graph via a public property like graph on the agent and call agent.graph; update all external call sites to use the new public symbol (build_graph or graph) and keep the original internal implementation name if you prefer to preserve backward compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@examples/templates/viral_tech_copywriter/__main__.py`:
- Line 96: The code calls the private method _build_graph() on agent from
outside the class; change this to use a public API by either adding a public
wrapper method build_graph() on the agent class that returns self._build_graph()
(or renaming _build_graph to build_graph) and then replace agent._build_graph()
with agent.build_graph(), or expose the graph via a public property like graph
on the agent and call agent.graph; update all external call sites to use the new
public symbol (build_graph or graph) and keep the original internal
implementation name if you prefer to preserve backward compatibility.
In `@examples/templates/viral_tech_copywriter/agent.py`:
- Around line 249-266: Remove the unused timeout parameter from the
trigger_and_wait signature and any references to it; specifically update the
async method trigger_and_wait to no longer accept timeout: float | None and
ensure callers are not relying on it, since the call to self._executor.execute
(GraphExecutor.execute) does not accept a timeout argument. Keep the method to
pass graph=self._graph, goal=self.goal, input_data=input_data,
session_state=session_state to self._executor.execute without any timeout
plumbing.
- Around line 276-292: Add explicit return type annotations for the info() and
validate() methods to improve discoverability and IDE support; update the
signature of info() (in the Agent class/function where info is defined) to
return a typed mapping such as dict[str, Any] or, better, a specific TypedDict
describing keys
("name","version","description","goal","nodes","edges","entry_node","entry_points","pause_nodes","terminal_nodes","client_facing_nodes"),
and similarly annotate validate() with its appropriate return type (e.g.,
dict[str, Any] or a validation TypedDict/Protocol). Import typing primitives
(TypedDict, dict, Any) as needed and keep the types consistent with the actual
returned structures in info() and validate().
- Around line 175-187: The __init__ signature is missing a type hint for the
config parameter; import RuntimeConfig from the config module and annotate the
parameter as config: RuntimeConfig | None in the __init__ method (the
constructor shown) so the parameter has a proper type, while keeping the
existing default fallback (config or default_config) and leaving all assignments
to self.config, self._executor, self._graph, etc. unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db317e66-abc7-4308-99cc-16195cafb3e6
📒 Files selected for processing (5)
examples/templates/viral_tech_copywriter/README.mdexamples/templates/viral_tech_copywriter/__main__.pyexamples/templates/viral_tech_copywriter/agent.pyexamples/templates/viral_tech_copywriter/flowchart.jsonexamples/templates/viral_tech_copywriter/tests/test_structure.py
✅ Files skipped from review due to trivial changes (1)
- examples/templates/viral_tech_copywriter/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/templates/viral_tech_copywriter/flowchart.json
|
@JiwaniZakir thank you very much for your comments. 3 of them (out of 4) have been addressed. Just one of them was not addressed: The note that run never loads MCP isn’t accurate: run goes through default_agent.run() → start() → _setup(), which always calls load_hive_tools_registry() when mcp_servers.json is present, same as the delivery path. |
|
The |
|
Are you sure you are commenting at the PR you intended? This agent has nothing to do with Shopify and shipments. |
|
The PR description still has placeholder changes ("Change 1, Change 2, Change 3") which makes it hard to review what was actually implemented — can you fill that out with the real diff summary? Also, the checklist item for adding tests is unchecked; if this agent has any prompt templating or output formatting logic, that should have coverage before merge. |
|
@JiwaniZakir added some description there. Please let me know if you need anything else for this PR. |
|
The graph topology looks correct — four nodes with |
Description
Adds
examples/templates/viral_tech_copywriter/, a reference goal-driven agent for tech marketing copy: conversational brief → structured JSON → hooks + per-channel copy → user-chosen HTML and/or Markdown export, using only existing hive-tools (save_data,append_data,serve_file_to_user,load_data,list_data_files,edit_data).Graph:
intake(client-facing) →normalize-brief→write-package→deliver-exports(terminal, client-facing; MCP tools only ondeliver-exports).mcp_servers.jsonis required;ViralTechCopywriterAgent.load_hive_tools_registry()fails fast with a clear error if it is missing (bothrunandtui), and_setup()uses the same helper so MCP wiring is not duplicated.flowchart.jsonmatchesagent.py(four nodes; terminaldeliver-exports).Type of Change
Related Issues
Fixes #6821
Changes Made
examples/templates/viral_tech_copywriter/:agent.py,nodes/__init__.py,config.py,__init__.py,__main__.py,mcp_servers.json,flowchart.json(aligned with graph).mcp_config_path(),load_hive_tools_registry()(fail-fast if config missing), used from_setup()and TUI without poking private fields from the CLI.examples/templates/README.md: table row for this template.tests/test_structure.py(graph, deliver-node tools, goal weights,AgentRunner.load, MCP path + missing-file error).PYTHONPATH, MCP requirement, honesty constraints.Testing
Describe the tests you ran to verify your changes:
cd core && pytest tests/)cd core && ruff check .)Checklist
Screenshots (if applicable)
Add screenshots to demonstrate UI changes.
Summary by CodeRabbit
New Features
Documentation
Tests