Skip to content

Setup Makefile, Pre-commit, and initial Tool and Schema definition#1

Merged
xingyaoww merged 4 commits intomainfrom
dev
Aug 23, 2025
Merged

Setup Makefile, Pre-commit, and initial Tool and Schema definition#1
xingyaoww merged 4 commits intomainfrom
dev

Conversation

@xingyaoww
Copy link
Collaborator

No description provided.

- Add Makefile with uv sync --dev for dependency management
- Add pre-commit hooks that run uv format on every commit
- Add pre-commit dependency to dev group
- Add uv version check requiring >= 0.8.13
- Replace multiple targets with single 'build' target
- Use native 'uv format' command instead of ruff directly
- Reduce from 8 targets to 5 for simplicity
@xingyaoww xingyaoww merged commit d9fe284 into main Aug 23, 2025
malhotra5 pushed a commit that referenced this pull request Sep 6, 2025
… requirements

- Test requirement #1: Multiple pause method calls successively only create one PauseEvent
- Test requirement #2: Calling conversation.pause() while conversation.run() is running in separate thread will pause the agent
- Test requirement #3: Calling conversation.run() on an already paused agent will resume it

The tests now properly validate the actual pause/resume behavior where run() resets the pause flag and resumes execution.

Co-authored-by: openhands <openhands@all-hands.dev>
xingyaoww pushed a commit that referenced this pull request Feb 9, 2026
This commit addresses the critical issues raised in the code review:

## Critical Issue #1: Data Structure Design Flaw
- Refactored AgentState to AgentStateRegistry using dict[str, Any] pattern
- Provides loosely-coupled storage for agent-specific state
- Agents store state using string keys instead of typed fields
- Maintains backward compatibility with AgentState alias

## Critical Issue #2: Hidden State Mutation
- Fixed _check_iterative_refinement to only increment iteration counter
  when refinement will actually continue
- State mutation now happens AFTER validation checks pass
- Function name now accurately reflects its behavior

## Critical Issue #3: Missing Tests
- Added comprehensive tests for AgentStateRegistry (14 tests)
- Added tests for IterativeRefinementConfig validation (4 tests)
- Added tests for _check_iterative_refinement logic (12 tests)
- Tests cover edge cases: max iterations, threshold boundaries, etc.

## Suggestion #5: Unnecessary Forward Reference
- Removed quotes from LLM type hint in example

## Suggestion #7: Unused Method
- Removed reset_iterative_refinement() as part of registry refactor

## Note on Issue #4 (FinishTool Special Case)
The suggestion to move iterative refinement to the conversation layer
is a valid architectural improvement but requires more significant
changes. This is deferred to a future PR to keep this change focused.

Co-authored-by: openhands <openhands@all-hands.dev>
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

Comments