Skip to content

feat: AG-UI Decision Logic with Router and Rich UI Components#52

Merged
mathaix merged 3 commits intomainfrom
feature/ag-ui-decision-spec
Dec 23, 2025
Merged

feat: AG-UI Decision Logic with Router and Rich UI Components#52
mathaix merged 3 commits intomainfrom
feature/ag-ui-decision-spec

Conversation

@mathaix
Copy link
Copy Markdown
Owner

@mathaix mathaix commented Dec 23, 2025

Summary

  • Add intelligent router for switching between chat and rich UI components
  • Implement DataTableCapture and ProcessMapBuilder frontend components
  • Add structured output models with Instructor integration
  • Include design documents for future UX enhancements

Key Changes

Backend

  • router.py: Heuristic + LLM routing with confidence thresholds (0.75 tool, 0.45 clarify)
  • structured_output.py: Pydantic models for structured LLM responses
  • tools.py: request_data_table, request_process_map tools
  • Integration with design_assistant.py

Frontend

  • DataTableCapture.tsx: Bulk data entry with paste/inline modes
  • ProcessMapBuilder.tsx: Workflow step capture
  • Enhanced OptionCards for selection lists

Design Documents

  • ag-ui-decision-logic-spec.md: Router architecture spec
  • design-agent-avatars.md: Future avatar feature plan
  • design-ux-ideas.md: 15 UX enhancement ideas

Test plan

  • Verify router correctly triggers data table for bulk entry
  • Verify router triggers process map for workflow descriptions
  • Test selection list auto-detection from assistant output
  • Confirm anti-thrash rules prevent tool loops

🤖 Generated with Claude Code

Mathew and others added 3 commits December 23, 2025 01:26
…ents

Implement intelligent routing between chat and rich UI components:
- Add router.py with heuristic + LLM-based routing decisions
- Add DataTableCapture and ProcessMapBuilder frontend components
- Add request_data_table and request_process_map tools
- Integrate router state tracking in design assistant
- Add confidence thresholds for tool vs clarify vs chat decisions
- Support structured UI submissions with JSON markers
- Add infer_selection_from_assistant_output for numbered lists

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- design-agent-avatars.md: Plan for unique avatars + character names
- design-ux-ideas.md: Collection of 15 UX enhancement ideas

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Dec 23, 2025

PR Review: AG-UI Decision Logic with Router and Rich UI Components

This is a substantial PR that introduces intelligent routing and rich UI components to Clara. Overall, the implementation is well-structured and follows good practices, but there are several areas that need attention.


🎯 Summary

Additions: 6,870 lines
Deletions: 445 lines
Files Changed: 27

This PR adds:

  • Router system for switching between chat and UI components
  • Structured output models with Pydantic validation
  • DataTableCapture and ProcessMapBuilder frontend components
  • Design documents for future UX enhancements

✅ Strengths

1. Strong Type Safety & Validation

  • Excellent use of Pydantic models throughout (structured_output.py)
  • Comprehensive validation with clear error messages
  • Type hints are consistently applied

2. Well-Tested

  • test_structured_output.py has 282 lines of comprehensive unit tests
  • Tests cover edge cases (too few options, empty text, confidence bounds)
  • Good test organization with class-based grouping

3. Security-Conscious

  • Input sanitization using InputSanitizer throughout
  • Template injection prevention in hydrate_template()
  • Proper handling of user input in router and structured output

4. Clean Architecture

  • Clear separation of concerns (router, structured output, UI components)
  • Reusable components with well-defined interfaces
  • Good use of dataclasses and Pydantic models

5. Frontend Quality

  • React components are clean and functional
  • Good UX with paste/inline modes for data entry
  • Proper state management with React hooks

⚠️ Issues & Concerns

1. Missing Dependency Declaration

Severity: HIGH

The PR adds instructor to uv.lock but I don't see it being imported or used anywhere in the code. The title mentions "Instructor integration" but:

  • No imports of instructor library in any Python files
  • The code uses native Anthropic SDK with tool-forcing instead
  • pyproject.toml likely needs updating if instructor is actually needed

Action Required: Either remove the unused dependency or document where/how it will be used.


2. Router Decision Threshold Logic

Severity: MEDIUM

In router.py:

TOOL_CONFIDENCE_THRESHOLD = 0.75
CLARIFY_CONFIDENCE_THRESHOLD = 0.45

The thresholds are hardcoded, but the spec document (ag-ui-decision-logic-spec.md) describes these as critical decision points. Consider:

  1. Making these configurable via settings
  2. Adding telemetry/logging when threshold downgrades occur
  3. Documenting the rationale for these specific values

File: src/backend/clara/agents/router.py


3. API Client Initialization Pattern

Severity: MEDIUM

In both router.py and structured_output.py, there's a pattern of lazy client initialization:

if not self._client:
    if settings.anthropic_api_key:
        self._client = anthropic.AsyncAnthropic(api_key=settings.anthropic_api_key)
    else:
        self._client = anthropic.AsyncAnthropic()

Issues:

  • Client is created on every first call, not at init time
  • No connection pooling or reuse across instances
  • Error handling for missing API keys happens late

Recommendation: Move client initialization to __init__ with proper error handling.


4. Data Validation Gap

Severity: MEDIUM

In router.py:614-656 (_build_data_table_params), there's heuristic-based table scaffolding:

if "stakeholder" in normalized:
    title = "Stakeholder List"
    columns = [...]
elif "risk" in normalized:
    title = "Risk Register"

Issues:

  • Simple keyword matching may misfire
  • No validation that the scaffolded columns match user intent
  • Could lead to wrong table structures

Recommendation: Add confidence scoring for table type detection or let the LLM decide the structure.


5. Magic String Constants

Severity: LOW

Throughout the codebase, there are magic strings that should be constants:

# router.py line 21-24
DATA_TABLE_MARKER_START = "[DATA_TABLE_SUBMIT]"
DATA_TABLE_MARKER_END = "[/DATA_TABLE_SUBMIT]"

These markers are used for parsing but aren't documented. Consider:

  • Adding a docstring explaining the marker format
  • Creating a shared constants file if used across modules

6. Frontend: No Error Recovery

Severity: MEDIUM

In DataTableCapture.tsx and ProcessMapBuilder.tsx:

const handleSubmit = () => {
  // ... validation ...
  onSubmit({ title, columns, rows: cleanedRows });
};

Issues:

  • No handling of submission errors
  • No loading states during submission
  • User could submit multiple times

Recommendation: Add loading/success/error states with proper feedback.


7. Code Duplication

Severity: LOW

The _extract_tool_input function appears in both router.py:343-354 and structured_output.py:385-396 with identical implementation.

Recommendation: Extract to a shared utility module.


8. Design Documents in Code Repository

Severity: LOW - Process Issue

Three design documents were added to docs/:

  • ag-ui-decision-logic-spec.md
  • design-agent-avatars.md
  • design-ux-ideas.md

Per CLAUDE.md, the canonical design documents should be in /Users/mantiz/Clara-Analysis/. These docs should either:

  1. Move to the correct location, OR
  2. Be marked as implementation-specific specs (not product design)

9. Missing Test Coverage

Severity: MEDIUM

While structured_output.py has good tests, I don't see tests for:

  • router.py - the core routing logic
  • Integration tests for the full flow
  • Frontend component tests

Recommendation: Add unit tests for router decisions, especially edge cases.


10. Performance Consideration: Nested LLM Calls

Severity: MEDIUM

In design_assistant.py, the flow now involves:

  1. Main agent generates response
  2. StructuredOutputParser.parse() calls another LLM
  3. UIRouter.decide() may call yet another LLM

This creates a double/triple LLM call on every turn, which could:

  • Increase latency significantly
  • Increase API costs
  • Create failure points

Recommendation:

  • Add caching for router decisions on similar inputs
  • Consider using the main agent's structured output directly
  • Add circuit breakers for failed LLM calls

🔒 Security Review

Good:

  • Input sanitization via InputSanitizer
  • Pydantic validation prevents injection
  • Max limits on rows/steps/options

⚠️ Concerns:

  1. Regex Injection Risk (Low): _extract_selection_items uses user input in regex patterns, but appears safe due to escaping
  2. DOS Risk (Low): Large paste operations could cause memory issues - add size limits to paste input

🎨 Code Quality

Positives:

  • ✅ Clear function/variable naming
  • ✅ Good docstrings on most functions
  • ✅ Consistent code style
  • ✅ Type hints throughout

Areas for Improvement:

  • ⚠️ Some functions exceed 50 lines (e.g., _heuristic_decide is 77 lines)
  • ⚠️ Complex conditional logic could benefit from decomposition
  • ⚠️ Missing module-level docstrings in some files

📋 Testing Checklist

Before merging, verify:

  • Run uv run pytest tests/unit/test_structured_output.py -v - all tests pass
  • Run uv run mypy clara/ - no type errors
  • Run uv run ruff check clara/ - no linting issues
  • Test router with various user inputs (lists, processes, selections)
  • Test data table paste functionality with CSV/TSV
  • Test process map with min/max steps
  • Verify selection lists auto-add "Other" option
  • Test error states in frontend components
  • Verify confidence thresholds work as expected
  • Check API costs with nested LLM calls

🎯 Recommendations

Must Fix Before Merge:

  1. ✅ Clarify instructor dependency - remove or document usage
  2. ✅ Add error handling to frontend submission flows
  3. ✅ Add tests for router.py
  4. ✅ Move client initialization to __init__

Should Fix Soon:

  1. Add configuration for confidence thresholds
  2. Extract duplicate _extract_tool_input function
  3. Add telemetry for router decisions
  4. Optimize nested LLM call pattern

Nice to Have:

  1. Add integration tests for full flow
  2. Document marker format and magic strings
  3. Improve table scaffolding heuristics

📊 Overall Assessment

Code Quality: ⭐⭐⭐⭐ (4/5)
Test Coverage: ⭐⭐⭐ (3/5)
Security: ⭐⭐⭐⭐ (4/5)
Architecture: ⭐⭐⭐⭐ (4/5)

Overall: ⭐⭐⭐⭐ (4/5)

This is solid work that adds significant value to Clara. The structured output and router implementation is well-designed. Address the critical issues (especially the instructor dependency and frontend error handling) and this will be ready to merge.

Great job on the comprehensive test coverage for structured_output! 🎉


📝 Files Reviewed

Backend:

  • src/backend/clara/agents/router.py (1,085 lines)
  • src/backend/clara/agents/structured_output.py (586 lines)
  • src/backend/clara/agents/design_assistant.py (changes)
  • src/backend/tests/unit/test_structured_output.py (281 lines)

Frontend:

  • src/frontend/src/components/design-assistant/DataTableCapture.tsx
  • src/frontend/src/components/design-assistant/ProcessMapBuilder.tsx

Docs:

  • docs/ag-ui-decision-logic-spec.md
  • 📖 docs/design-agent-avatars.md (future feature)
  • 📖 docs/design-ux-ideas.md (future feature)

Let me know if you'd like me to dive deeper into any specific area!

@mathaix mathaix merged commit 258d878 into main Dec 23, 2025
1 check passed
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.

1 participant