-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add internal monologue and smaller steps #113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add internal monologue and smaller steps #113
Conversation
📝 WalkthroughWalkthroughReworked manager-to-developer handoff into a single delegation flow with a new DelegatedDeveloperTask and dual manager handbacks. Updated ManagerHandoffInput field order. Renamed prompt constants. Added INTERNAL_MONOLOGUE_GUIDANCE and updated prompt assembly. Adjusted manager run prompt phrasing. Fixed emoji parsing in update_progress_log. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Manager
participant Developer
participant ManagerCallbacks as Manager callbacks
User->>Manager: Start session
note over Manager: Prepare delegation input (DelegatedDeveloperTask)
Manager->>Developer: delegate_to_developer(DelegatedDeveloperTask)
note right of Developer: Executes task
alt Progress update
Developer-->>ManagerCallbacks: report_back_to_manager(ManagerHandoffInput)
end
alt Completion
Developer-->>ManagerCallbacks: report_task_completion_to_manager(ManagerHandoffInput)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
connector_builder_agents/src/agents.py (1)
95-101
: Consider removing redundantapi_name
field.The
api_name
field inDelegatedDeveloperTask
appears redundant since the developer agent already receivesapi_name
in its instructions (line 41). Including it in the delegation input creates an opportunity for inconsistency if the values differ.Consider simplifying to:
class DelegatedDeveloperTask(BaseModel): """Input data for handoff from manager to developer.""" - api_name: str assignment_title: str assignment_description: str
And update the callback accordingly:
async def on_developer_delegation(ctx, input_data: DelegatedDeveloperTask) -> None: update_progress_log( - f"🤝 Delegating task to developer agent." - f"\n Task Name: {input_data.assignment_title}" + f"🤝 Delegating task to developer agent: {input_data.assignment_title}" f"\n Task Description: {input_data.assignment_description}", session_state, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
connector_builder_agents/src/agents.py
(4 hunks)connector_builder_agents/src/constants.py
(1 hunks)connector_builder_agents/src/guidance.py
(3 hunks)connector_builder_agents/src/run.py
(1 hunks)connector_builder_agents/src/tools.py
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/agents.py (1)
connector_builder_agents/src/tools.py (2)
SessionState
(23-54)update_progress_log
(190-211)
🪛 GitHub Actions: Linters
connector_builder_agents/src/agents.py
[error] 1-1: ruff format check detected differences. 2 files would be reformatted by 'ruff format --diff'. Run 'uv run ruff format .' (or 'ruff format') to fix formatting.
connector_builder_agents/src/guidance.py
[error] 1-1: ruff format check detected differences. 2 files would be reformatted by 'ruff format --diff'. Run 'uv run ruff format .' (or 'ruff format') to fix formatting.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build PokemonTGG Connector
- GitHub Check: Build Hubspot Connector
- GitHub Check: Test Connector Build (JSONPlaceholder)
- GitHub Check: Build JSONPlaceholder Connector
- GitHub Check: Test Connector Build (PokemonTGG)
- GitHub Check: Test Connector Build (PokemonTGG)
- GitHub Check: Test Connector Build (JSONPlaceholder)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (7)
connector_builder_agents/src/tools.py (1)
203-203
: Correct fix for emoji unpacking.The tuple assignment properly extracts the emoji character and remainder of the message, fixing the previous bug.
connector_builder_agents/src/constants.py (1)
25-26
: LGTM: Clear renaming to reflect root prompt file.The constant names now better convey their purpose as the root prompt file reference.
connector_builder_agents/src/run.py (1)
248-250
: LGTM: Clearer and more concise prompt.The updated wording directly specifies the API and uses "objectives" terminology, aligning better with the delegated developer workflow.
connector_builder_agents/src/guidance.py (2)
11-23
: LGTM: Clear narration guidance for developer agent.The
INTERNAL_MONOLOGUE_GUIDANCE
provides structured guidance for task narration, helping the developer agent maintain concise, observable internal monologue during execution.
1-1
: Runruff format
to fix formatting issues.The pipeline detected formatting differences in this file and
agents.py
.Run the following command to fix:
uv run ruff format .
connector_builder_agents/src/agents.py (2)
4-4
: LGTM: Necessary import for callback typing.The
Callable
import supports the new callback factory pattern introduced in this file.
150-165
: Clarify the distinction between two manager handoffs.The developer agent now has two separate handoff tools for reporting back to the manager:
report_back_to_manager
andreport_task_completion_to_manager
. Both use the sameManagerHandoffInput
type and callback, which may confuse the developer agent about when to use each.Consider one of these approaches:
- Consolidate into a single handoff if there's no semantic difference in handling
- Use different input types to enforce different semantics (e.g.,
TaskCompletionInput
with fewer optional fields)- Add clarifying documentation in the tool descriptions explaining when to use each
Example for approach 2:
class TaskProgressInput(BaseModel): """Input for reporting interim progress.""" short_status: str detailed_progress_update: str is_blocked: bool class TaskCompletionInput(BaseModel): """Input for reporting task completion.""" short_status: str detailed_progress_update: str is_full_success: bool is_partial_success: boolCan you clarify the intended distinction between these two handoffs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
connector_builder_agents/src/tools.py (1)
201-203
: Syntax fix looks correct, but emoji detection range is limited.The unpacking on line 203 is now syntactically correct. However, the Unicode range check only covers the "Emoticons" block (U+1F600–U+1F64F), missing most emoji blocks like Miscellaneous Symbols and Pictographs (U+1F300–U+1F5FF), Transport (U+1F680–U+1F6FF), Flags (U+1F1E6–U+1F1FF), and others. Multi-codepoint emojis (e.g., with skin tone modifiers or ZWJ sequences) also won't be detected.
Since there's a fallback emoji (
"📍"
), this won't break functionality, but consider using a library likeemoji
for more robust detection:# At the top of the file import emoji # In the function if message and emoji.is_emoji(message[0]): emoji, message = message[0], message[1:].lstrip()Alternatively, expand the range check to cover more common blocks or document this limitation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
connector_builder_agents/src/agents.py
(4 hunks)connector_builder_agents/src/constants.py
(1 hunks)connector_builder_agents/src/guidance.py
(3 hunks)connector_builder_agents/src/run.py
(1 hunks)connector_builder_agents/src/tools.py
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/agents.py (1)
connector_builder_agents/src/tools.py (2)
SessionState
(23-54)update_progress_log
(190-211)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Pytest (Fast)
- GitHub Check: Test Connector Build (JSONPlaceholder)
- GitHub Check: Test Connector Build (PokemonTGG)
🔇 Additional comments (11)
connector_builder_agents/src/agents.py (5)
4-4
: LGTM!The
Callable
import is correctly added and used in thecreate_on_developer_delegation
function return type.
73-79
: LGTM!The handoff configuration correctly uses the new
DelegatedDeveloperTask
input type and properly binds the callback to the session state.
95-101
: LGTM!The
DelegatedDeveloperTask
model is well-structured with appropriate fields for delegation context.
103-111
: LGTM!The field reordering improves logical flow by grouping success indicators before the blocked flag.
144-167
: LGTM!The dual handback pattern effectively allows the developer to report progress or completion using appropriately named tools while sharing the same handler logic.
connector_builder_agents/src/constants.py (1)
25-26
: LGTM!The constant rename from
PROMPT_FILE_*
toROOT_PROMPT_FILE_*
improves clarity by explicitly indicating these relate to the root prompt.connector_builder_agents/src/run.py (1)
248-249
: LGTM!The simplified prompt construction and semantic change from "phases" to "objectives" aligns well with the broader prompt updates in this PR.
connector_builder_agents/src/guidance.py (4)
8-8
: LGTM!The import correctly uses the renamed
ROOT_PROMPT_FILE_STR
constant.
11-23
: LGTM!The internal monologue guidance provides clear structure for agent narration and aligns with the PR objective of breaking work into smaller steps.
60-76
: LGTM!The manager prompt correctly uses
ROOT_PROMPT_FILE_STR
following the constant rename.
79-96
: LGTM!The developer prompt correctly incorporates
INTERNAL_MONOLOGUE_GUIDANCE
to enable step-by-step narration. Note thatROOT_PROMPT_FILE_STR
is intentionally only included in the manager prompt, which is appropriate for the separation of concerns between orchestration and execution.
This resolves an issue where poor coordination between manager and developer agent would lead to the developer finishing all tasks in the first "phase" and then the manager re-delegating the work again in phase 2 and phase 3.
There are still failures in communication between manager and agent, but now the manager will properly note when the work is done and exit earlier.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor