feat: Add task list tracking system with LLM tools and Progress tab UI#39
Conversation
- Add Pydantic models for task tracking (Task, ConnectorTask, StreamTask) - Implement TaskList model with methods for managing tasks - Add LLM tools for task management (list, add, insert, update status, remove) - Integrate task list into ConnectorBuilderState with JSON serialization - Update Progress tab to render tasks with stream tasks in separate table - Add __str__ methods to tasks for text rendering - Create default connector task list with essential development tasks Co-Authored-By: AJ Steers <aj@airbyte.io>
Original prompt from AJ Steers |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRefactors state into three rx.State classes (BuilderState, ChatAgentState, UIState), adds a typed TaskList model and task-management tools, moves page composition to pages/index, restructures UI components and tabs, centralizes agent guidance in _guidance.py, updates dependencies, and adapts tests to the new state names. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Chat Sidebar UI
participant ChatState as ChatAgentState
participant Agent as Agent Runtime
participant SessionDeps
participant TaskList
participant Builder as BuilderState
User->>UI: submit message
UI->>ChatState: send_message()
ChatState->>ChatState: append user message
ChatState->>Agent: ensure started & run with history
Agent->>SessionDeps: call tools (add/update tasks, get/set fields)
SessionDeps->>TaskList: append/update tasks
TaskList-->>ChatState: task list updated
Agent-->>ChatState: stream assistant response
ChatState->>Builder: sync YAML/form fields from SessionDeps
ChatState-->>UI: update messages and task views
UI-->>User: render updated conversation and progress
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus review on:
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive task tracking system for the Agentic Connector Builder webapp, enabling users to track connector development progress through LLM-assisted task management. The system introduces task models, LLM tools for task manipulation, state synchronization, and a redesigned Progress tab UI.
Key changes:
- New Pydantic models for task tracking with support for generic connector tasks and stream-specific tasks
- Seven new LLM tools enabling conversational task management (list, add, insert, update status, remove)
- Complete Progress tab UI rewrite with visual status indicators and separate rendering for connector vs. stream tasks
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
task_list.py |
Defines task models, TaskList container, and default task list factory |
progress_tab.py |
Completely rewrites Progress tab to render tasks dynamically with status icons and separate views |
chat_agent.py |
Adds 7 LLM tools for task management and updates SessionDeps to include task_list_json |
agentic_connector_builder_webapp.py |
Adds task_list_json to state, implements lazy initialization, and syncs changes back from agent |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py
Outdated
Show resolved
Hide resolved
- Add flat computed vars to ConnectorBuilderState for task counts and display text - Replace nested dictionary access with direct computed var references - Use rx.cond and rx.foreach instead of Python if/for statements - Fix VarTypeError by avoiding boolean checks on Var objects in UI code - Ensure proper Reflex reactive pattern for state-driven UI rendering Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (1)
48-96: Initialize task_list_json so the Progress tab actually shows the defaultsRight now
task_list_jsonstarts as"", sohas_task_liststays false untilsend_message()callsget_task_list(). That means a user who opens the Progress tab before chatting just sees “No tasks yet,” even though we ship an eight-item default list. It’s a confusing regression: the new UI never renders the default checklist on first load.Please seed the state with the serialized default list (or otherwise call
get_task_list()during initial render) so the Progress tab shows the expected tasks without requiring a chat round-trip.- task_list_json: str = "" + task_list_json: str = create_default_connector_task_list().model_dump_json()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py(6 hunks)agentic_connector_builder_webapp/chat_agent.py(3 hunks)agentic_connector_builder_webapp/tabs/progress_tab.py(1 hunks)agentic_connector_builder_webapp/task_list.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
agentic_connector_builder_webapp/task_list.py (1)
agentic_connector_builder_webapp/chat_agent.py (2)
update_task_status(641-683)remove_task(686-716)
agentic_connector_builder_webapp/tabs/progress_tab.py (1)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (11)
ConnectorBuilderState(37-501)has_task_list(201-203)task_list_name(301-309)task_list_description(312-320)completed_of_total_text(367-369)in_progress_text(372-374)failed_text(377-379)has_connector_tasks(291-293)connector_tasks_view(206-233)has_stream_tasks(296-298)stream_tasks_view(236-265)
agentic_connector_builder_webapp/chat_agent.py (1)
agentic_connector_builder_webapp/task_list.py (9)
ConnectorTask(36-39)StreamTask(42-50)TaskList(53-112)TaskStatus(9-15)get_summary(98-112)add_task(69-72)insert_task(74-80)update_task_status(82-88)remove_task(90-96)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (2)
agentic_connector_builder_webapp/task_list.py (3)
TaskList(53-112)create_default_connector_task_list(115-140)get_summary(98-112)agentic_connector_builder_webapp/chat_agent.py (1)
set_connector_name(397-426)
- Add task_name field (short string, required) to Task model - Rename title field to description (optional longer text) - Remove details field - Update __str__ methods to use task_name - Update default task list to BuildNewConnector workflow with 5 steps: 1. Collect information from user 2. Research and analyze source API 3. Enumerate streams and create first stream's tasks 4. Run connector readiness report (pass 1) 5. Run connector readiness report (pass 2) - Update all LLM tools to use task_name and description parameters - Update computed vars in ConnectorBuilderState to map new fields to UI - Maintain backward compatibility with model_post_init hook Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (1)
438-438: Side-effect call for task list initialization.This call ensures the task list is initialized before creating
SessionDeps. A past review noted this side-effect pattern is unclear, but the behavior is correct for lazy initialization. See comment on lines 90-100 for suggested improvements.agentic_connector_builder_webapp/task_list.py (2)
63-80: Critical: Reorder union to prioritize concrete subclasses during deserialization.The union
Task | ConnectorTask | StreamTaskshould list concrete types first so Pydantic triesConnectorTaskandStreamTaskbefore the baseTaskduring deserialization. This preventsStreamTaskinstances from being incorrectly deserialized as plainTaskobjects.Apply this diff:
- tasks: list[Task | ConnectorTask | StreamTask] = Field( + tasks: list[ConnectorTask | StreamTask | Task] = Field( default_factory=list, description="List of tasks" ) - def get_task_by_id(self, task_id: str) -> Task | ConnectorTask | StreamTask | None: + def get_task_by_id(self, task_id: str) -> ConnectorTask | StreamTask | Task | None: """Get a task by its ID.""" for task in self.tasks: if task.id == task_id: return task return None - def add_task(self, task: Task | ConnectorTask | StreamTask) -> Task: + def add_task(self, task: ConnectorTask | StreamTask | Task) -> Task: """Add a new task to the list.""" self.tasks.append(task) return task def insert_task( - self, position: int, task: Task | ConnectorTask | StreamTask + self, position: int, task: ConnectorTask | StreamTask | Task ) -> Task:Optional: The union itself is redundant since subclasses are compatible with
Task. If Pydantic's discriminator field (task_type) reliably handles deserialization after forbidding extras, simplify tolist[Task].
18-38: Critical: Addextra="forbid"to prevent StreamTask deserialization bug.A past review identified that
TaskList.model_validate_json(...)losesStreamTask.stream_nameduring deserialization because Pydantic's defaultextra="allow"causes the baseTaskmodel to accept but discard thestream_namefield when the union triesTaskfirst. This breaks the stream tasks table in the Progress UI.Apply this diff to forbid extra fields on the base model:
+from pydantic import BaseModel, Field, ConfigDict class Task(BaseModel): """Base task model with common fields.""" + + model_config = ConfigDict(extra="forbid") task_type: str = Field(description="Type of task (connector or stream)")Note: The
model_post_initstub (lines 32-34) appears to be for backward compatibility but currently does nothing. Consider removing it if not needed.
🧹 Nitpick comments (1)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (1)
90-100: Consider renamingget_task_listto clarify initialization side effect.A past review noted that
get_task_list()has side effects (initializingtask_list_jsonif empty) but the return value is not always used. This makes the intent unclear. Consider either:
- Rename to
ensure_task_list_initialized()to signal the side effect- Always assign the result:
task_list = self.get_task_list()The lazy initialization pattern itself is sound, enabling automatic default task list creation on first use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py(6 hunks)agentic_connector_builder_webapp/chat_agent.py(3 hunks)agentic_connector_builder_webapp/task_list.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
agentic_connector_builder_webapp/task_list.py (1)
agentic_connector_builder_webapp/chat_agent.py (2)
update_task_status(655-697)remove_task(700-730)
agentic_connector_builder_webapp/chat_agent.py (1)
agentic_connector_builder_webapp/task_list.py (9)
ConnectorTask(41-44)StreamTask(47-55)TaskList(58-117)TaskStatus(9-15)get_summary(103-117)add_task(74-77)insert_task(79-85)update_task_status(87-93)remove_task(95-101)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (2)
agentic_connector_builder_webapp/task_list.py (3)
TaskList(58-117)create_default_connector_task_list(120-162)get_summary(103-117)agentic_connector_builder_webapp/chat_agent.py (1)
SessionDeps(80-91)
🔇 Additional comments (20)
agentic_connector_builder_webapp/task_list.py (3)
87-93: LGTM!The status update logic correctly mutates the task in place and returns success/failure.
103-117: LGTM!Summary calculation is efficient and provides all necessary metrics for the UI.
120-162: LGTM!The default task list provides a sensible workflow for connector development with appropriate checkpoints.
agentic_connector_builder_webapp/chat_agent.py (9)
12-12: LGTM!Import of task list types is appropriate for the new tools.
89-89: LGTM!Adding
task_list_jsonto session dependencies enables state synchronization between the agent and UI.
428-471: Verify StreamTask detection after fixing deserialization bug.Line 462 uses
isinstance(task, StreamTask)to detect stream tasks, but this will always returnFalseuntil the deserialization bug intask_list.pyis fixed. Once you apply theextra="forbid"and union reordering fixes, this check will work correctly.After applying the fixes to
task_list.py, test this tool to confirm stream tasks display with[Stream: {stream_name}]annotations.
473-511: LGTM!The connector task creation logic is correct with appropriate error handling.
513-557: LGTM!Stream task creation follows the same pattern as connector tasks with the additional
stream_namefield appropriately included.
559-601: LGTM!Position-based insertion is correctly implemented with automatic bounds clamping in the
TaskList.insert_taskmethod.
603-652: LGTM!Stream task insertion correctly mirrors the connector task insertion logic.
654-697: LGTM!Status update tool has excellent validation with clear error messages for invalid status values.
699-730: LGTM!Task removal logic is straightforward and includes appropriate error handling.
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (8)
24-24: LGTM!Import of task list utilities is appropriate for state management.
48-48: LGTM!Adding
task_list_jsonto state enables persistence and UI synchronization.
200-203: LGTM!The
has_task_listproperty provides a clean boolean check for conditional UI rendering.
205-235: Connector task view transformation is correct.The filtering logic correctly excludes stream tasks and maps to UI-friendly dictionaries. The bare
except Exception(line 234) is acceptable here since UI properties should be resilient and return safe defaults rather than crashing the view.Note: This view will only work correctly after the deserialization bug in
task_list.pyis fixed.
271-292: LGTM!Header property provides safe defaults and comprehensive error handling for UI consumption.
294-383: LGTM!The suite of computed properties follows a consistent pattern with appropriate error handling. The formatted text properties (lines 370-383) provide convenient UI bindings for displaying task statistics.
447-447: LGTM!Including
task_list_jsoninSessionDepsenables the agent tools to access and modify task state.
488-490: LGTM!Bidirectional sync correctly propagates agent-modified task state back to the UI, following the same pattern as
yaml_contentsynchronization.
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py
Outdated
Show resolved
Hide resolved
- Add ensure_task_list() event to initialize default BuildNewConnector task list - Call ensure_task_list() from set_current_tab() when Progress tab is selected - This ensures the 5-step BuildNewConnector workflow displays immediately - Follows Reflex best practices for event-based state initialization Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (1)
446-446: Useensure_task_list()instead ofget_task_list()when return value is unused.Line 446 calls
get_task_list()but doesn't use the return value. Since the intent is initialization, useensure_task_list()which makes the purpose clearer and has no return value.Apply this diff:
- self.get_task_list() + self.ensure_task_list()
🧹 Nitpick comments (2)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (2)
90-96: Consider splitting initialization side effect from query method.
get_task_list()both returns a TaskList and modifiestask_list_jsonas a side effect when empty. This violates the command-query separation principle. Sinceensure_task_list()already exists for initialization, consider removing the side effect fromget_task_list()so it only deserializes and returns.Apply this diff:
def get_task_list(self) -> TaskList: - """Get the task list object from JSON.""" + """Get the task list object from JSON. Assumes task_list_json is already initialized.""" if not self.task_list_json: - default_task_list = create_default_connector_task_list() - self.task_list_json = default_task_list.model_dump_json() - return default_task_list + raise ValueError("task_list_json is empty. Call ensure_task_list() first.") return TaskList.model_validate_json(self.task_list_json)This makes the API clearer: callers must explicitly initialize via
ensure_task_list()before querying withget_task_list().
213-392: Replace bareexceptclauses with specific exception types.Multiple computed properties use bare
except:clauses (lines 242, 276, 295, and others) that silently catch all exceptions and return default values. This hides errors likeAttributeError,JSONDecodeError, orValidationErrorthat could help diagnose issues during development.Replace bare except with specific exceptions:
try: task_list = TaskList.model_validate_json(self.task_list_json) # ... property logic - except Exception: + except (ValueError, AttributeError) as e: + print(f"Error in property_name: {e}") return [] # or appropriate defaultThis provides better visibility into deserialization failures while still gracefully degrading the UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (2)
agentic_connector_builder_webapp/task_list.py (3)
TaskList(58-117)create_default_connector_task_list(120-162)get_summary(103-117)agentic_connector_builder_webapp/chat_agent.py (1)
SessionDeps(80-91)
🔇 Additional comments (3)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (3)
140-141: LGTM!Lazy initialization of the task list when switching to the progress tab is a good pattern. This ensures the default task list is created before the UI attempts to render it.
448-458: LGTM!Adding
task_list_jsontoSessionDepscorrectly propagates the task list state to the agent session, consistent with the pattern used for other state fields likeyaml_contentandconnector_name.
496-498: LGTM!The bidirectional synchronization of
task_list_jsonfromsession_depsback to state correctly mirrors the pattern used foryaml_content. The yield ensures the UI refreshes after the agent modifies the task list.
- Resolved merge conflict in chat_agent.py - Kept task list imports from feature branch - Kept FormField enum and form field tools from main - Both features now coexist in the agent Co-Authored-By: AJ Steers <aj@airbyte.io>
…CKED status and agent checklist discipline - Remove FAILED status, keep only BLOCKED for simpler task state management - Add FinalizationTask class for post-stream work - Add task_status_detail field to capture context when updating task status - Add finalization task tools (add_finalization_task, insert_finalization_task) - Update update_task_status to accept task_status_detail parameter - Update list_tasks to group tasks into three sections (Connector, Stream, Finalization) - Update Progress tab UI to render three sections with blocked status badge - Add finalization_tasks_view computed var and has_finalization_tasks check - Update agent system prompt with 'Checklist discipline' section instructing agent to: - Call list_tasks at start of work - Mark tasks IN_PROGRESS when starting, COMPLETED/BLOCKED when done - Automatically proceed to next task unless user requests otherwise - Report back when blocked with task_status_detail - Update default BuildNewConnector workflow: steps 1-3 as ConnectorTask, steps 4-5 as FinalizationTask Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.gitignore (1)
1-2: Entry is functionally correct; consider grouping with other tool-specific ignores for consistency.The addition of
.claude/settings.local.jsonis appropriate for preventing local Claude IDE configuration from being committed. However, it is placed at the very top of the file, whereas similar tool-specific ignores (e.g.,.cursorignoreat line 209,.abstra/at line 190) are grouped in dedicated sections. For consistency with the existing organizational pattern, consider moving this entry to a dedicated Claude/IDE configuration section alongside other tool-specific ignores.agentic_connector_builder_webapp/state/progress_state.py (1)
44-132: Deduplicate status-to-icon mappingThe three view helpers copy-paste the same status→icon/color dictionary, and
list_tasksinchat_agent.pynow owns a parallel copy. Centralizing this keeps the UI and LLM responses from drifting when we tweak icons or add statuses. Please hoist the mapping into a shared constant and reuse it here (and in the agent) instead of redefining it per view.--- a/agentic_connector_builder_webapp/state/progress_state.py +++ b/agentic_connector_builder_webapp/state/progress_state.py @@ -from ..task_list import TaskList, create_default_connector_task_list +from ..task_list import TaskList, TaskStatus, create_default_connector_task_list + +STATUS_DISPLAY: dict[str, tuple[str, str]] = { + TaskStatus.NOT_STARTED.value: ("○", "gray.400"), + TaskStatus.IN_PROGRESS.value: ("◐", "blue.400"), + TaskStatus.COMPLETED.value: ("●", "green.400"), + TaskStatus.BLOCKED.value: ("⛔", "red.400"), +} @@ - icon, color = { - "not_started": ("○", "gray.400"), - "in_progress": ("◐", "blue.400"), - "completed": ("●", "green.400"), - "blocked": ("⛔", "red.400"), - }.get(status, ("?", "gray.400")) + icon, color = STATUS_DISPLAY.get(status, ("?", "gray.400")) @@ - icon, color = { - "not_started": ("○", "gray.400"), - "in_progress": ("◐", "blue.400"), - "completed": ("●", "green.400"), - "blocked": ("⛔", "red.400"), - }.get(status, ("?", "gray.400")) + icon, color = STATUS_DISPLAY.get(status, ("?", "gray.400")) @@ - icon, color = { - "not_started": ("○", "gray.400"), - "in_progress": ("◐", "blue.400"), - "completed": ("●", "green.400"), - "blocked": ("⛔", "red.400"), - }.get(status, ("?", "gray.400")) + icon, color = STATUS_DISPLAY.get(status, ("?", "gray.400"))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.gitignore(1 hunks)agentic_connector_builder_webapp/_guidance.py(1 hunks)agentic_connector_builder_webapp/agentic_connector_builder_webapp.py(1 hunks)agentic_connector_builder_webapp/chat_agent.py(5 hunks)agentic_connector_builder_webapp/components/chat_sidebar.py(3 hunks)agentic_connector_builder_webapp/pages/index.py(1 hunks)agentic_connector_builder_webapp/state/__init__.py(1 hunks)agentic_connector_builder_webapp/state/builder_state.py(1 hunks)agentic_connector_builder_webapp/state/chat_agent_state.py(1 hunks)agentic_connector_builder_webapp/state/progress_state.py(1 hunks)agentic_connector_builder_webapp/state/ui_state.py(1 hunks)agentic_connector_builder_webapp/tabs/progress_tab.py(1 hunks)agentic_connector_builder_webapp/tabs/tabs.py(1 hunks)pyproject.toml(1 hunks)tests/conftest.py(0 hunks)tests/test_app.py(5 hunks)
💤 Files with no reviewable changes (1)
- tests/conftest.py
🧰 Additional context used
🧬 Code graph analysis (10)
agentic_connector_builder_webapp/state/__init__.py (4)
agentic_connector_builder_webapp/state/builder_state.py (1)
BuilderState(6-72)agentic_connector_builder_webapp/state/chat_agent_state.py (1)
ChatAgentState(21-236)agentic_connector_builder_webapp/state/progress_state.py (1)
ProgressState(10-253)agentic_connector_builder_webapp/state/ui_state.py (1)
UIState(8-49)
agentic_connector_builder_webapp/pages/index.py (5)
agentic_connector_builder_webapp/components/chat_sidebar.py (1)
chat_sidebar(53-129)agentic_connector_builder_webapp/components/settings_modal.py (2)
settings_button(112-148)settings_modal(6-109)agentic_connector_builder_webapp/state/chat_agent_state.py (3)
ChatAgentState(21-236)set_chat_input(40-42)send_message(106-204)agentic_connector_builder_webapp/state/ui_state.py (7)
UIState(8-49)has_api_key(42-44)open_settings_modal(19-21)has_env_api_key(47-49)close_settings_modal(23-25)set_openai_api_key_input(27-29)save_settings(31-33)agentic_connector_builder_webapp/tabs/tabs.py (1)
connector_builder_tabs(10-54)
agentic_connector_builder_webapp/tabs/tabs.py (6)
agentic_connector_builder_webapp/state/builder_state.py (8)
BuilderState(6-72)set_source_api_name(21-23)set_connector_name(25-27)set_documentation_urls(29-31)set_functional_requirements(33-35)set_test_list(37-39)update_yaml_content(41-43)reset_yaml_content(45-68)agentic_connector_builder_webapp/state/ui_state.py (2)
UIState(8-49)set_current_tab(15-17)agentic_connector_builder_webapp/tabs/code_tab.py (1)
code_tab_content(8-12)agentic_connector_builder_webapp/tabs/progress_tab.py (1)
progress_tab_content(8-174)agentic_connector_builder_webapp/tabs/requirements_tab.py (1)
requirements_tab_content(8-134)agentic_connector_builder_webapp/tabs/save_publish_tab.py (1)
save_publish_tab_content(6-51)
agentic_connector_builder_webapp/agentic_connector_builder_webapp.py (1)
agentic_connector_builder_webapp/pages/index.py (1)
index(20-90)
agentic_connector_builder_webapp/state/builder_state.py (1)
agentic_connector_builder_webapp/chat_agent.py (1)
set_connector_name(397-426)
agentic_connector_builder_webapp/tabs/progress_tab.py (1)
agentic_connector_builder_webapp/state/progress_state.py (12)
has_task_list(34-36)task_list_name(175-183)task_list_description(186-194)completed_of_total_text(241-243)in_progress_text(246-248)blocked_text(251-253)has_connector_tasks(160-162)connector_tasks_view(39-68)has_stream_tasks(165-167)stream_tasks_view(71-102)has_finalization_tasks(170-172)finalization_tasks_view(105-134)
agentic_connector_builder_webapp/state/progress_state.py (1)
agentic_connector_builder_webapp/task_list.py (3)
TaskList(70-133)create_default_connector_task_list(136-187)get_summary(119-133)
agentic_connector_builder_webapp/chat_agent.py (1)
agentic_connector_builder_webapp/task_list.py (11)
ConnectorTask(47-50)FinalizationTask(64-67)StreamTask(53-61)TaskList(70-133)TaskStatus(9-15)get_summary(119-133)add_task(88-93)insert_task(95-101)update_task_status(103-109)get_task_by_id(79-86)remove_task(111-117)
agentic_connector_builder_webapp/state/chat_agent_state.py (4)
agentic_connector_builder_webapp/chat_agent.py (3)
SessionDeps(105-119)create_chat_agent(134-929)set_connector_name(397-426)agentic_connector_builder_webapp/state/builder_state.py (6)
BuilderState(6-72)set_source_api_name(21-23)set_connector_name(25-27)set_documentation_urls(29-31)set_functional_requirements(33-35)set_test_list(37-39)agentic_connector_builder_webapp/state/progress_state.py (2)
ProgressState(10-253)get_task_list(15-21)agentic_connector_builder_webapp/state/ui_state.py (1)
get_effective_api_key(35-39)
tests/test_app.py (2)
agentic_connector_builder_webapp/components/yaml_editor.py (1)
yaml_editor_component(7-49)agentic_connector_builder_webapp/state/builder_state.py (8)
BuilderState(6-72)update_yaml_content(41-43)reset_yaml_content(45-68)set_source_api_name(21-23)set_connector_name(25-27)set_documentation_urls(29-31)set_functional_requirements(33-35)set_test_list(37-39)
🪛 GitHub Actions: CI
agentic_connector_builder_webapp/components/chat_sidebar.py
[error] 4-4: F401: 'reflex.components.radix.themes.layout.flex.flex' imported but unused. Found 1 error. 1 fixable with the --fix option. Command 'uv run ruff check .' failed with exit code 1.
agentic_connector_builder_webapp/state/chat_agent_state.py
[error] 117-117: SyntaxError: 'await' outside async function
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
agentic_connector_builder_webapp/components/chat_sidebar.py (1)
90-90: Remove commented-out code.The commented line
# flex_grow="0"should be removed rather than left in the codebase.Apply this diff:
width="100%", height="80%", - # flex_grow="0", ),agentic_connector_builder_webapp/state/chat_agent_state.py (2)
125-125: Clarify the purpose of this side-effect call.The call to
progress_state.get_task_list()is made for its side effect (initializingtask_list_jsonif empty), but the return value is discarded. This creates an implicit dependency with Line 134 wheretask_list_jsonis used, which may confuse maintainers.Consider adding a comment to clarify the intent:
+ # Ensure task_list_json is initialized before passing to agent progress_state.get_task_list()
142-143: Consider simplifying message history construction.The current approach appends the user message to
chat_messages(Line 119) and then immediately removes it when building history (Line 142). While functionally correct, this is less clear than building the history before appending the new message.Consider this alternative:
+ # Build message history before appending current message + message_history = self._convert_to_pydantic_history(self.chat_messages) + user_message = self.chat_input.strip() self.chat_messages.append({"role": "user", "content": user_message}) self.chat_input = "" self.chat_loading = True self.current_streaming_message = "" yield progress_state.get_task_list() session_deps = SessionDeps( # ... session deps construction ... ) - recent_messages = self.chat_messages[:-1] # Remove the current user message - message_history = self._convert_to_pydantic_history(recent_messages) - effective_api_key = ui_state.get_effective_api_key()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
agentic_connector_builder_webapp/components/chat_sidebar.py(2 hunks)agentic_connector_builder_webapp/state/chat_agent_state.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
agentic_connector_builder_webapp/state/chat_agent_state.py (4)
agentic_connector_builder_webapp/chat_agent.py (3)
SessionDeps(105-119)create_chat_agent(134-929)set_connector_name(397-426)agentic_connector_builder_webapp/state/builder_state.py (6)
BuilderState(6-72)set_source_api_name(21-23)set_connector_name(25-27)set_documentation_urls(29-31)set_functional_requirements(33-35)set_test_list(37-39)agentic_connector_builder_webapp/state/progress_state.py (1)
get_task_list(15-21)agentic_connector_builder_webapp/state/ui_state.py (2)
UIState(8-49)get_effective_api_key(35-39)
🔇 Additional comments (4)
agentic_connector_builder_webapp/components/chat_sidebar.py (3)
76-91: Verify the layout behavior ofrx.spacer()afterrx.auto_scroll.The
rx.spacer()on line 87 is placed afterrx.auto_scroll, which may not have the intended effect. Typically,rx.auto_scrollshould expand to fill the available space within its parent container. The spacer might create unexpected layout behavior or gaps. Please verify that the message area renders correctly and that the auto-scroll functionality works as expected with this structure.
94-119: Verify thaton_sendhandles form submission events correctly.The form now uses
on_submit=on_send(line 116) instead of a buttonon_clickhandler. In Reflex,on_submithandlers receive form submission events, which may differ from click events. Please verify that theon_sendhandler is compatible with form submission events and handles the form data appropriately. If the handler was originally designed for button clicks, it may need adjustment.
104-114: Good use of Reflex form submission pattern.The form submission structure using
rx.form.submitwith the tooltip-wrapped button follows Reflex best practices. The tooltip providing keyboard shortcut information (Cmd+Enter) is a nice touch for user experience.agentic_connector_builder_webapp/state/chat_agent_state.py (1)
109-109: Past SyntaxError resolved.The previous review flagged that
send_messagewas synchronous but containedawaitexpressions. This has been correctly fixed by making the method async.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
agentic_connector_builder_webapp/state/chat_agent_state.py (1)
270-278: Avoid calling__aexit__when the agent never started.If the API key changes before
_ensure_agent_startedsuccessfully runs__aenter__, this branch invokes__aexit__on an agent that was never entered, which violates the async context manager protocol and can raise (same issue noted previously). Guard the cleanup with_agent_startedbefore calling__aexit__.[suggested fix]
- if self._cached_agent is not None and self._cached_api_key != effective_api_key: - try: - await self._cached_agent.__aexit__(None, None, None) - except Exception as e: - print( - f"[_ensure_agent_started] Error during agent cleanup when API key changed: {e}" - ) - self._cached_agent = None - self._agent_started = False + if ( + self._cached_agent is not None + and self._cached_api_key != effective_api_key + ): + if self._agent_started: + try: + await self._cached_agent.__aexit__(None, None, None) + except Exception as e: + print( + f"[_ensure_agent_started] Error during agent cleanup when API key changed: {e}" + ) + self._cached_agent = None + self._agent_started = False
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
agentic_connector_builder_webapp/_guidance.py(1 hunks)agentic_connector_builder_webapp/chat_agent.py(7 hunks)agentic_connector_builder_webapp/components/chat_sidebar.py(4 hunks)agentic_connector_builder_webapp/models/task_list.py(1 hunks)agentic_connector_builder_webapp/pages/index.py(1 hunks)agentic_connector_builder_webapp/state/__init__.py(1 hunks)agentic_connector_builder_webapp/state/chat_agent_state.py(1 hunks)agentic_connector_builder_webapp/tabs/progress_tab.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- agentic_connector_builder_webapp/state/init.py
- agentic_connector_builder_webapp/pages/index.py
- agentic_connector_builder_webapp/_guidance.py
🧰 Additional context used
🧬 Code graph analysis (5)
agentic_connector_builder_webapp/tabs/progress_tab.py (1)
agentic_connector_builder_webapp/state/chat_agent_state.py (8)
ChatAgentState(24-470)task_list_name(171-178)completed_of_total_text(221-223)in_progress_text(226-228)blocked_text(231-233)connector_tasks_view(51-75)stream_tasks_view(78-104)finalization_tasks_view(107-131)
agentic_connector_builder_webapp/state/chat_agent_state.py (4)
agentic_connector_builder_webapp/chat_agent.py (2)
SessionDeps(106-116)create_chat_agent(131-587)agentic_connector_builder_webapp/models/task_list.py (3)
TaskList(90-226)get_summary(172-190)new_connector_build_task_list(193-226)agentic_connector_builder_webapp/state/builder_state.py (1)
BuilderState(6-72)agentic_connector_builder_webapp/state/ui_state.py (2)
UIState(8-49)get_effective_api_key(35-39)
agentic_connector_builder_webapp/models/task_list.py (1)
agentic_connector_builder_webapp/chat_agent.py (1)
update_task_status(500-537)
agentic_connector_builder_webapp/chat_agent.py (1)
agentic_connector_builder_webapp/models/task_list.py (8)
ConnectorTask(67-70)FinalizationTask(84-87)StreamTask(73-81)TaskList(90-226)TaskStatusEnum(9-24)TaskTypeEnum(27-32)append_task(119-127)update_task_status(143-156)
agentic_connector_builder_webapp/components/chat_sidebar.py (1)
agentic_connector_builder_webapp/state/chat_agent_state.py (4)
ChatAgentState(24-470)has_started(46-48)start_demo(298-303)pause_agent(320-322)
🪛 GitHub Actions: CI
agentic_connector_builder_webapp/state/chat_agent_state.py
[error] 1-1: Ruff format check failed: 1 file would be reformatted by 'ruff format --check'. Run 'uv run ruff format' to fix formatting.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
agentic_connector_builder_webapp/models/task_list.py (1)
38-40: Use TaskTypeEnum for stronger type safety.The
task_typefield is currently defined asstr, but subclasses assignTaskTypeEnumvalues. Consider changing the type annotation toTaskTypeEnumfor better type safety and automatic validation.Apply this diff:
- task_type: str = Field( + task_type: TaskTypeEnum = Field( description="Type of task (connector, stream, or finalization)" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
agentic_connector_builder_webapp/models/task_list.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
agentic_connector_builder_webapp/models/task_list.py (1)
agentic_connector_builder_webapp/chat_agent.py (1)
update_task_status(500-537)
🔇 Additional comments (2)
agentic_connector_builder_webapp/models/task_list.py (2)
9-32: LGTM! Clean enum definitions.The status and type enums are well-structured with appropriate emoji representations for UI feedback.
119-127: LGTM! Well-structured CRUD operations and factory method.The
append_task,insert_task,get_summary, andnew_connector_build_task_listmethods are correctly implemented with appropriate defensive coding (bounds checking ininsert_task) and sensible defaults for the connector build workflow.Also applies to: 129-141, 172-226
- Format chat_agent_state.py with ruff to fix lint CI failure - Add yaml_editor_state fixture in conftest.py for backward compatibility - The fixture returns BuilderState instance to support existing tests - All 12 tests now pass locally Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
agentic_connector_builder_webapp/state/chat_agent_state.py (1)
276-284: Check agent context state before cleanup.When the API key changes,
__aexit__is called on the cached agent without verifying that_agent_startedis True. This violates the async context manager protocol and may cause runtime errors if__aenter__was never successfully called.Apply this diff to only clean up agents that were successfully started:
if self._cached_agent is not None and self._cached_api_key != effective_api_key: + if self._agent_started: - try: - await self._cached_agent.__aexit__(None, None, None) - except Exception as e: - print( - f"[_ensure_agent_started] Error during agent cleanup when API key changed: {e}" - ) + try: + await self._cached_agent.__aexit__(None, None, None) + except Exception as e: + print( + f"[_ensure_agent_started] Error during agent cleanup when API key changed: {e}" + ) self._cached_agent = None self._agent_started = False
🧹 Nitpick comments (3)
agentic_connector_builder_webapp/state/chat_agent_state.py (3)
51-137: Extract duplicated icon/color mapping to reduce maintenance burden.The status-to-icon/color mapping is duplicated across
connector_tasks_view,stream_tasks_view, andfinalization_tasks_view. This makes future updates error-prone.Consider extracting the mapping to a class-level constant or helper method:
# At class level _STATUS_ICONS = { "not_started": ("○", "gray.400"), "in_progress": ("◐", "blue.400"), "completed": ("●", "green.400"), "blocked": ("⛔", "red.400"), } @rx.var def connector_tasks_view(self) -> list[dict[str, Any]]: """Get connector tasks as JSON-serializable view data.""" if self.task_list is None: return [] try: result = [] for task in self.task_list.basic_connector_tasks: status = task.status.value icon, color = self._STATUS_ICONS.get(status, ("?", "gray.400")) # ... rest of implementationNote: Line 91 uses "✅" for stream tasks while Line 62 uses "●" for connector tasks—verify whether this difference is intentional.
241-268: Consider handling additional message roles or documenting the limitation.The method only handles "user" and "assistant" roles. If system messages or other role types are added to
chat_messagesin the future, they will be silently skipped due to the continue statement on Line 266.Consider either:
- Adding support for "system" messages if needed
- Logging a warning for unhandled roles to aid debugging
- Adding a docstring note that only user/assistant roles are supported
379-381: Redundant assignment in pause check.Line 380 sets
self.agent_paused = Truewhen the condition already verified it's True. This assignment has no effect.If the intent is just to break the loop, remove the redundant assignment:
if self.agent_paused: - self.agent_paused = True break
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
agentic_connector_builder_webapp/state/chat_agent_state.py(1 hunks)tests/conftest.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
agentic_connector_builder_webapp/state/chat_agent_state.py (4)
agentic_connector_builder_webapp/chat_agent.py (2)
SessionDeps(106-116)create_chat_agent(131-587)agentic_connector_builder_webapp/models/task_list.py (3)
TaskList(90-226)get_summary(172-190)new_connector_build_task_list(193-226)agentic_connector_builder_webapp/state/builder_state.py (1)
BuilderState(6-72)agentic_connector_builder_webapp/state/ui_state.py (2)
UIState(8-49)get_effective_api_key(35-39)
tests/conftest.py (1)
agentic_connector_builder_webapp/state/builder_state.py (1)
BuilderState(6-72)
🔇 Additional comments (3)
tests/conftest.py (1)
8-9: LGTM! Clean refactoring to align with state restructuring.The import and fixture have been correctly updated to use
BuilderStatefrom the new state package structure. The fixture name remains unchanged, preserving backward compatibility for existing tests.Also applies to: 62-67
agentic_connector_builder_webapp/state/chat_agent_state.py (2)
345-349: Verify that lazy task list initialization is the intended behavior.The task list is auto-initialized on first agent run rather than at state creation. This means
task_listremainsNoneuntil the user sends their first message, and the UI will show no tasks until then.If tasks should be visible immediately when the app loads, consider initializing the task list in the class definition:
- task_list: TaskList | None = None + task_list: TaskList | None = TaskList.new_connector_build_task_list()Or initialize it in an
on_loadevent handler if you want it created when the page loads but not hardcoded in the class.
450-480: LGTM! Proper serialization safeguards for non-picklable objects.The
__getstate__and__setstate__overrides correctly exclude backend variables (including the MCP server's SSLContext) from serialization and reinitialize them after deserialization. This prevents pickle errors while maintaining the agent cache lifecycle.
Aaron ("AJ") Steers (aaronsteers)
deleted the
devin/1761942373-task-list-tracking
branch
feat: Add task list tracking system with LLM tools and Progress tab UI
Summary
This PR implements a comprehensive task list tracking system for the Agentic Connector Builder webapp. The system includes:
Core Models (new file
task_list.py):ConnectorTask(generic) andStreamTask(with stream_name field)TaskListmodel with CRUD operations for managing tasks__str__methods for text rendering (StreamTask formats as "{stream_name}: {title}")LLM Integration (
chat_agent.py):list_tasks,add_connector_task,add_stream_task,insert_connector_task,insert_stream_task,update_task_status,remove_taskSessionDepsto includetask_list_jsonfor state synchronizationState Management (
agentic_connector_builder_webapp.py):task_list_jsonfield toConnectorBuilderStatewith JSON serializationUI Rendering (
progress_tab.py):Review & Testing Checklist for Human
Test Plan Recommendation
uv run reflex runNotes
Requested by: AJ Steers (Aaron ("AJ") Steers (@aaronsteers))
Devin Session: https://app.devin.ai/sessions/08df8cee9f0e4e7693113652f75dd673
Potential Issues to Watch For:
task_typefield as discriminatorrender_task_list()) which is a Reflex pattern but may have unexpected behavioryieldstatements in async generator, timing could be sensitiveArchitecture Notes:
get_task_list()method handles lazy initialization and JSON parsingctx.deps.task_list_jsondirectly, which syncs back to state after agent executionSummary by CodeRabbit
New Features
Refactor
Chores