Repository navigation
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a9c4d6880
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def handle_dry_run(): | ||
| return _handle_dry_run_with_deadline() | ||
| def handle_paper_execution_command_consumer(): |
There was a problem hiding this comment.
All ten @app.route(...) decorators from the previous entrypoint were removed during extraction, and a repository-wide search finds no replacement registration. Consequently, the application exported as main:app has only Flask's static route: execution, reconciliation, monitoring and health requests all return 404 once import succeeds. This also violates the route contract enforced by scripts/validate_cloud_run_startup.py, which the Dockerfile runs during image construction. Restore the decorators or explicitly register the handlers on this app.
Useful? React with 👍 / 👎.
| from notifications_runtime import * # noqa: F403, F401 | ||
| from strategy_plugins_runtime import * # noqa: F403, F401 | ||
| from http_routes import * # noqa: F403, F401 |
There was a problem hiding this comment.
Python star imports exclude underscore-prefixed names unless __all__ explicitly exports them, so this no longer exposes the private helpers that application/http_routes.py still resolves dynamically through its main proxy. For example, local monitor prechecks call main._handle_dry_run_with_deadline, successful probes with account facts call main._account_facts_match_runtime_target, and probe error handling calls main._publish_runtime_failure_notification; each now raises AttributeError. Even after restoring route registration, these flows remain broken. Explicitly re-export the required helpers or update their consumers to resolve them from their new owning modules.
Useful? React with 👍 / 👎.
| from runtime_wiring import * # noqa: F403, F401 | ||
| from notifications_runtime import * # noqa: F403, F401 | ||
| from strategy_plugins_runtime import * # noqa: F403, F401 | ||
| from http_routes import * # noqa: F403, F401 |
There was a problem hiding this comment.
When this project is installed as a wheel rather than run from its checkout, importing main now fails with ModuleNotFoundError: No module named 'runtime_wiring'. pyproject.toml explicitly enumerates the top-level py-modules, but none of the four newly imported modules is listed; its separate package-discovery configuration does not include standalone .py modules. A repository-wide packaging-config and tracked-artifact search found no alternative inclusion mechanism. Add all four extracted modules to the distribution configuration so the installed entrypoint retains its dependencies.
Useful? React with 👍 / 👎.
Keep main as a thin export surface for tests and process entry. Co-authored-by: Cursor <cursoragent@cursor.com>
0a9c4d6 to
c993641
Compare
Pigbibi
added
the
agent-approved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9936416e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _export(module): | ||
| globals().update({key: value for key, value in vars(module).items() if not key.startswith("__")}) |
There was a problem hiding this comment.
Copying functions into main does not change their global namespace, so existing overrides on main no longer affect the extracted implementations. This breaks the notification guards installed with setattr(main, name, forbidden) in scripts/pilot_app_isolation_entry.py, as well as regression tests that replace main.connect_ib, main.build_composer, or main.RUNTIME_SETTINGS. For example, replacing main.RUNTIME_SETTINGS leaves main.get_ib_port() reading the original settings in runtime_wiring. Preserve dynamic dependency lookup through the entrypoint or update all override consumers to patch the owning modules.
Useful? React with 👍 / 👎.
| import http_routes as _http_routes | ||
| import notifications_runtime as _notifications_runtime | ||
| import runtime_wiring as _runtime_wiring | ||
| import strategy_plugins_runtime as _strategy_plugins_runtime |
There was a problem hiding this comment.
The existing strategy_module_factory in tests/conftest.py changes environment variables and removes/reloads only main to construct each runtime. These imports now leave runtime_wiring and the other extracted modules cached, so every subsequent factory call retains the first runtime's settings and strategy configuration. Tests such as test_ib_connect_timeout_can_be_overridden consequently observe whichever timeout an earlier test loaded rather than their requested value, and paper/live configuration cases no longer exercise their intended runtime. Update the factory's module lifecycle alongside this extraction so configuration and dependent modules are recreated for each runtime.
Useful? React with 👍 / 👎.
| def handle_request(): | ||
| if request.method != "POST": | ||
| return "Method Not Allowed", 405 | ||
| return _route_with_runtime_error_fallback(_handle_request) |
There was a problem hiding this comment.
Even after route registration is restored, any POST reaching handle_request or handle_probe raises NameError because _route_with_runtime_error_fallback lives in notifications_runtime but is never imported into this module. The same incomplete wiring affects ordinary execution calls such as the bare load_strategy_plugin_signals() reference, whose implementation now lives in strategy_plugins_runtime. Importing those modules as nr and spr does not expose their functions as bare names, and exporting everything into main does not populate the extracted functions' defining namespaces. Explicitly import or qualify the cross-module dependencies so the default handlers work without overrides.
Useful? React with 👍 / 👎.
|
Closing without merge: the mechanical main.py split introduced broken private-symbol exports and adapter call sites (confirmed by CI + Codex review). QuantAIService #503 and QuantPlatformKit #782 from this untangle batch already merged. Platform entry splits will be redone in a follow-up PR with test-backed adapter wiring. |
AI assistance
Cursor Auto (Composer) with local agent session. Owner authorized commit, push, and merge for this untangle refactor batch.
What changed
How verified