-
Notifications
You must be signed in to change notification settings - Fork 2
Fix broken file structure in scripts/orchestrate_content.py #181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: 73junito <86015877+73junito@users.noreply.github.com>
73junito
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
viewed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a critical file structure issue in scripts/orchestrate_content.py where core functions were incorrectly nested inside an exception handler, making them unreachable. The file structure has been corrected by moving all functions to module level and properly implementing exception handling with retry logic.
Changes:
- Restored correct module structure with all functions at module level instead of nested inside exception handlers
- Enhanced HTTP error handling in
call_ollamawith status code validation before processing responses - Improved timeout specification using
aiohttp.ClientTimeoutexplicit constructor
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def main(): | ||
| if len(sys.argv) < 2: | ||
| print("Usage: python scripts/orchestrate_content.py path/to/modules.json [--stream] [--dry-run] [--concurrency N]") |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The usage message should document that min_tokens and min_chars can be configured via environment variables ORCH_MIN_TOKENS and ORCH_MIN_CHARS, since the command-line parsing for these parameters was removed. This would help users understand how to customize these quality thresholds without needing to read the source code.
| print("Usage: python scripts/orchestrate_content.py path/to/modules.json [--stream] [--dry-run] [--concurrency N]") | |
| print( | |
| "Usage: python scripts/orchestrate_content.py path/to/modules.json [--stream] [--dry-run] [--concurrency N]\n" | |
| "Environment: set ORCH_MIN_TOKENS and ORCH_MIN_CHARS to adjust quality thresholds (min_tokens, min_chars)." | |
| ) |
| if resp.status != 200: | ||
| error_text = await resp.text() | ||
| raise aiohttp.ClientResponseError( | ||
| request_info=resp.request_info, | ||
| history=resp.history, | ||
| status=resp.status, | ||
| message=f"Ollama server returned status {resp.status}: {error_text}", | ||
| headers=resp.headers, |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling correctly raises ClientResponseError for non-200 responses and properly implements retry logic with exponential backoff. However, the error message construction uses string interpolation which could fail if error_text is very large or contains special characters. Consider truncating the error text or using a safer formatting approach to prevent potential issues when error responses are large.
| message=f"Ollama server returned status {resp.status}: {error_text}", | ||
| headers=resp.headers, | ||
| ) | ||
|
|
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line contains trailing whitespace. Consider removing it for consistency with Python style guidelines.
Summary
scripts/orchestrate_content.pyChanges
The file structure was corrupted during merge conflict resolution (commit f97fe9b). Lines 76-183 incorrectly defined
main(),process_module(), andorchestrate()inside thecall_ollamaexception handler's retry loop:Restored correct structure from commit 199617e with all functions properly defined at module level.
Migration steps
git checkout 199617e -- scripts/orchestrate_content.pyTesting
Rollback plan
Revert commit a4b1325 if issues arise; file structure prior to f97fe9b was functional
Checklist
Reviewers
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.