refactor: introduce type safety and centralized configuration #61
+23,578
−68,175
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…olkit functionalities
- Deleted the OPTIMIZATION_GUIDE.md as it was no longer relevant. - Removed README.md from notes directory to streamline documentation. - Cleared out TESTING_REPORT.md to eliminate obsolete test results. - Erased review and run skill-fleet implementation plan to update project direction. - Removed DSPy 3.1 reasoning adapters upgrade plan to reflect current architecture.
…ws, and getting started guide - Introduced `modules.md` detailing core modules for skill creation, including their structure, features, and usage examples. - Created `signatures.md` to define the contract between modules and language models, outlining inputs, outputs, and type constraints. - Added `workflows.md` to describe the sequential workflows for skill creation, including understanding, generation, and validation phases. - Developed `getting-started.md` tutorial to guide users through installation, configuration, and creating their first skill.
…or skills and documentation
…and refining existing ones
- Updated type checks for readiness_score and confidence to use the new syntax (int | float). - Expanded the GatherRequirements signature to include validation-oriented outputs such as suggested_skill_name, trigger_phrases, negative_triggers, skill_category, requires_mcp, and suggested_mcp_server. - Introduced ValidateSkillStructure signature to validate skill structure against Anthropic's requirements. - Added CollectBaselineMetrics and CollectSkillMetrics signatures for metrics collection and comparison. - Implemented validation checks in UnderstandingWorkflow and ValidationWorkflow to catch structure issues early and generate test cases. - Enhanced database session management with transactional_session context manager to prevent idle-in-transaction timeouts. - Improved MLflow integration by checking for method availability before calling. - Updated logging to provide more informative messages during error handling.
…nd Claude Prompt Refiner
chore: enhance .gitignore to exclude additional files and directories docs: update AGENTS.md with new DSPy best practices and project structure chore: add entries to CHANGELOG.md for new features and fixes refactor: remove legacy migration scripts from archive chore: add internal scripts for development and technical debt cleanup fix: ensure proper type checking in benchmark optimizers fix: update run optimization scripts for type checking
…tests - Introduced integration tests for HITL workflows covering multi-question clarification, skill preview approval, and validation failure recovery. - Enhanced job persistence tests by mocking database interactions and ensuring correct job state updates. - Updated understanding workflow tests to utilize a context manager for mock language model. - Adjusted API tests to reflect changes in job status handling. - Improved unit tests for HITL signature consistency and dependency version checks.
…fety - Updated signature templates to use `list[str]` and `str | None` for type hints. - Enhanced `compile_module` function formatting for better readability. - Cleaned up whitespace and comments in various files for consistency. - Improved docstrings across metrics and signatures for better documentation. - Added missing newlines in JSON and YAML files for proper formatting. - Adjusted training data files to ensure they end with a newline.
- Deleted patch_endpoint_example.py, test_patch_endpoint.py, visual_guide.py, and README.md from the 02-partial-updates example directory. - Removed integration.md, metadata.json, quick-reference.md, requirements.json, troubleshooting.md, and test JSON files from the fastapi-production resources and tests. - This cleanup removes outdated examples and tests, streamlining the project structure for better maintainability.
…SON and markdown files
…tion.sh and test_repository.py
…g guide, skill template update, TUI readiness, and update summary documentation
…ementation strategy, and taxonomy system to streamline project structure and focus on current methodologies.
…creation and validation sections
…ironment management
- Deleted useHitlConfig hook and its associated types and logic. - Removed useHitl hook and its related functionality for managing Human-in-the-Loop interactions. - Eliminated TUI entry point and streaming client for handling Server-Sent Events. - Cleaned up utility functions related to HITL keywords and state persistence. - Removed test script and TypeScript configuration for the TUI.
…used TUI spawner and streaming chat functionality
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
…hrough an exception Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
…est practices DSPy 3.1.2+ Compliance: - Remove redundant forward() methods from 10 modules (150 lines) - BaseModule now handles automatic sync/async bridging - Modules: intent, requirements, plan, compliance, best_of_n_validator, content, refined_content, test_cases, parallel_analysis, conversational FastAPI Improvements: - Add responses parameter to 7 endpoints for better OpenAPI docs - Replace dict[str, Any] with Pydantic models (SkillListItem, UpdateSkillResponse) - Enhanced error documentation Typer CLI Enhancements: - Add callback validation for port (1-65535) and host parameters - Follow Context7 best practices for CLI validation Rich UI Improvements: - Use status context for long-running operations (database init, job creation) - Better UX with progress indicators in chat, terminal, and serve commands Test Updates: - Update 4 test files to work with BaseModule's async-first approach - Remove tests for removed forward() implementations - Add nest-asyncio dependency for test compatibility Code Quality: - All ruff checks pass - All type checks pass (ty check) - Security check pass (bandit) - 289 tests passing Net result: -70 lines of code, improved maintainability, full best practice compliance
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| items = [i for i in items if q in i.name.lower()] | ||
| if status: | ||
| # Status isn't tracked in metadata today; keep placeholder behavior. | ||
| items = items |
Comment on lines
+50
to
+52
| async def aforward( | ||
| self, task_description: str, requirements: dict | None = None | ||
| ) -> dspy.Prediction: |
Comment on lines
+59
to
+66
| async def aforward( # type: ignore[override] | ||
| self, | ||
| task_description: str, | ||
| requirements: dict[str, Any] | None = None, | ||
| taxonomy_structure: dict[str, Any] | None = None, | ||
| existing_skills: list[str] | None = None, | ||
| available_skills_catalog: dict[str, Any] | None = None, | ||
| ) -> dspy.Prediction: |
…hrough an exception Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
…ing method' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Comment on lines
+345
to
+347
| async def aforward( | ||
| self, current_content: str, weaknesses: list[str], target_score: float = 0.8 | ||
| ) -> dict[str, Any]: | ||
| ) -> dspy.Prediction: |
Comment on lines
+63
to
+70
| async def aforward( | ||
| self, | ||
| requirements: dict, | ||
| intent_analysis: dict, | ||
| taxonomy_analysis: dict, | ||
| dependency_analysis: dict, | ||
| user_confirmation: str = "", | ||
| ) -> dict[str, Any]: | ||
| ) -> dspy.Prediction: |
Comment on lines
+25
to
+26
| # OR directly with uvicorn | ||
| uvicorn skill_fleet.api.main:app --reload |
There was a problem hiding this comment.
The example command references skill_fleet.api.main:app, but according to .github/copilot-instructions.md (line 10), the correct path is skill_fleet.api.main:app. While this is consistent, the description in copilot-instructions mentions the command should be uv run skill-fleet serve which uses uvicorn skill_fleet.api.main:app internally. Consider clarifying that this is the direct uvicorn command equivalent to the CLI command.
Suggested change
| # OR directly with uvicorn | |
| uvicorn skill_fleet.api.main:app --reload | |
| # OR directly with uvicorn (equivalent to `uv run skill-fleet serve`) | |
| uvicorn skill_fleet.app.main:app --reload |
| ## Critical runtime configuration | ||
|
|
||
| - **LLM config** is loaded by `configure_dspy()` from `config/config.yaml` (`src/skill_fleet/llm/dspy_config.py`). | ||
| - **LLM config** is loaded by `configure_dspy()` from `src/skill_fleet/config/config.yaml` (`src/skill_fleet/dspy/config.py`). |
There was a problem hiding this comment.
The path src/skill_fleet/config/config.yaml appears incorrect. Based on the file structure, the config file should be at config/config.yaml (project root), not under src/skill_fleet/config/. Update to config/config.yaml for accuracy.
Suggested change
| - **LLM config** is loaded by `configure_dspy()` from `src/skill_fleet/config/config.yaml` (`src/skill_fleet/dspy/config.py`). | |
| - **LLM config** is loaded by `configure_dspy()` from `config/config.yaml` (`src/skill_fleet/llm/dspy_config.py`). |
Comment on lines
+222
to
+223
| gh pr comment "$PR_NUMBER" \ | ||
| --body "🤖 **Claude Auto-Fix Workflow** |
There was a problem hiding this comment.
The PR comment body uses markdown but doesn't properly escape backticks around the branch name on line 228. The variable $BRANCH should be escaped to prevent issues if the branch name contains special characters. Use \$BRANCH`instead of`$BRANCH``.
…ing method' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Comment on lines
+246
to
+253
| async def aforward( # type: ignore[override] | ||
| self, | ||
| plan: dict[str, Any], | ||
| understanding: dict[str, Any], | ||
| skill_style: str = "comprehensive", | ||
| target_quality: float = 0.80, | ||
| max_iterations: int = 3, | ||
| ) -> dspy.Prediction: |
…ing method' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: Zachary BENSALEM <zachary@qredence.ai>
Comment on lines
+136
to
+138
| async def aforward( | ||
| self, task_description: str, user_context: dict | None = None | ||
| ) -> dspy.Prediction: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This comprehensive refactoring enhances Skill Fleet's code quality and maintainability through type safety, centralized configuration, and improved utilities.
Type Safety & Contracts
Phase1UnderstandingOutputwith validation and helper methodsPhase2GenerationOutputwithis_ready_for_validation()checkPhase3ValidationOutputfor final phase outputQualityThresholdsmodel with validation for 8 configurable parametersCentralized Configuration Management
DEFAULT_QUALITY_THRESHOLDSsingleton with validated defaults:validation_pass_threshold: 0.75refinement_target_quality: 0.80taxonomy_confidence_threshold: 0.60trigger_coverage_target: 0.90optimal_word_count: 500-3000 min/max, 5000 max acceptableverbosity_warning_threshold: 0.70GenerationWorkflowandValidationWorkflowupdated to use centralized thresholdsEnhanced Utilities
@with_llm_fallbackdecorator - graceful DSPy module degradationSKILL_FLEET_ALLOW_LLM_FALLBACKenv var@timed_executiondecorator - performance tracking for sync/async functionssanitize_for_log()function - safe logging without injection risksCode Quality Improvements
# type: ignore[override]comments (7 total)Documentation Updates
Key Features
Files Changed
src/skill_fleet/core/workflows/models.py(778 lines, Pydantic types)src/skill_fleet/common/logging_utils.py(53 lines, safe logging)src/skill_fleet/common/llm_fallback.py(+52 lines, decorator)src/skill_fleet/common/utils.py(+73 lines, timed execution)generation.py,validation.py(use centralized thresholds)Quality Assurance
ruff check . --fix- All linting passedpy.typed check- All type checks passedmake security- No security issues (bandit scan clean)Statistics
Related Issues
Closes #XXXX (Update with actual issue number if applicable)
Testing
The changes have been validated with:
No migration needed - existing code works as-is. New typed models available for opt-in gradual migration.