Skip to content

fix: Added unique node IDs, validator for worflow_plan, and unified planner model config#345

Open
rohit-sahoo wants to merge 3 commits intodevelopfrom
fix/planner-unique-node-ids
Open

fix: Added unique node IDs, validator for worflow_plan, and unified planner model config#345
rohit-sahoo wants to merge 3 commits intodevelopfrom
fix/planner-unique-node-ids

Conversation

@rohit-sahoo
Copy link
Collaborator

@rohit-sahoo rohit-sahoo commented Feb 11, 2026

Summary

Added unique node IDs to handle duplicate agents, added validator to ensure workflow_plan exists when ready_to_generate is true, and moved planner model config to single source in CONFIG for the planner.

Changes

1. Unique Node IDs (Duplicate Agents)

  • Added id field to WorkflowNode (e.g., code_search_1, code_search_2)
  • Edges now reference node id instead of type
  • io_map JSONPath uses node id (e.g., $.code_search_1.outputs.results)
  • Updated validation methods for node IDs

2. Model Config - Planner

  • Added planner_model_name to ModelConfigSettings as single source of truth
  • PlannerConfig now uses CONFIG.planner_model_name
  • FieldMappingGenerator now uses CONFIG.planner_model_name
  • Removed hardcoded model names from planner module

3. Validators

  • Added validator: ready_to_generate=True requires workflow_plan to exist
  • Prevents crash when LLM says "ready" but forgets to provide plan

How to Test

Unit tests:

uv run pytest tests/planner/

Manual test:

python scripts/demo_planner.py automated "Find weather repos, find climate repos, and do gap analysis"

@github-actions
Copy link

✅ Tests passed

📊 Test Results

  • Passed: 580
  • Failed: 0
  • Skipped: 40
  • Warnings: 141
  • Coverage: 77%

Branch: fix/planner-unique-node-ids
PR: #345
Commit: f9ff097

📋 Full coverage report and logs are available in the workflow run.


type: str = Field(..., description="Node type corresponding to agent type")
id: str = Field(..., description="Unique node identifier (e.g., code_search_0, gap_analysis_0)")
type: str = Field(..., description="Node type corresponding to agent type for registry lookup")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since type is built-in identifier, maybe switch to something like type_?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

phase: ConversationPhase = Field(..., description="Current conversation phase")
question: Optional[PlannerQuestion] = Field(default=None, description="Follow-up question if needed")
question: PlannerQuestion | None = Field(default=None, description="Follow-up question if needed")
workflow_plan: Optional[WorkflowPlan] = Field(default=None, description="Generated workflow plan")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you apply the new type hint style across all the planner modules? wehrever there's older style

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@github-actions
Copy link

✅ Tests passed

📊 Test Results

  • Passed: 580
  • Failed: 0
  • Skipped: 40
  • Warnings: 141
  • Coverage: 78%

Branch: fix/planner-unique-node-ids
PR: #345
Commit: d112de3

📋 Full coverage report and logs are available in the workflow run.

@rohit-sahoo rohit-sahoo requested a review from NISH1001 February 13, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants