m12-pr19: TD-1 first cut - extract _module_state.py from routes/generate/jobs.py#37
Open
BalaShankar9 wants to merge 1 commit into
Open
m12-pr19: TD-1 first cut - extract _module_state.py from routes/generate/jobs.py#37BalaShankar9 wants to merge 1 commit into
BalaShankar9 wants to merge 1 commit into
Conversation
…enerate/jobs.py
The routes/generate/jobs.py module had grown to 2496 LOC. This PR is
the first cut of TD-1 (split jobs.py): extract the cohesive group of
pure module-state / preferred-lock helpers into a sibling module
`_module_state.py` (195 LOC) and re-import them in jobs.py so all
existing import paths and `mock.patch("app.api.routes.generate.jobs._foo")`
call sites continue to resolve unchanged.
Result
- backend/app/api/routes/generate/jobs.py: 2496 → 2358 LOC
- backend/app/api/routes/generate/_module_state.py: new, 195 LOC
Extracted (12 names + 1 re-export of _bootstrap_registry)
- _track_bootstrap, _get_model_health_summary
- _DEFAULT_REQUESTED_MODULES, _SNAKE_TO_CAMEL, _CAMEL_TO_SNAKE, _IDENTITY_KEYS
- _now_ms, _apply_preferred_lock
- _default_module_states, _merge_module_states
- _normalize_requested_modules, _module_has_content
Why this cut first
- These are pure functions / module-level constants with no DB
dependency. The only side-effecting helper (_track_bootstrap) is a
thin shim over bootstrap_registry which moves with it.
- The rest of jobs.py (persistence helpers, runtime executor, route
handlers) is more entangled with DB clients, Temporal handles, and
SSE streaming — those should follow in subsequent PRs after a deeper
survey, not in a single 2500-line megamove.
Validation
- pytest tests/test_generate_jobs_smoke.py tests/test_task_registry.py
tests/temporal/ -x → 109 passed in 2.94s
- All `mock.patch("app.api.routes.generate.jobs._foo")` paths still
resolve (verified by the smoke test suite which patches several of
these names).
- _BOOTSTRAP_TASKS still aliases the same set object that the
bootstrap_registry uses internally (confirmed in module re-export
block).
Stacked on PR #36 (m12-pr18). Base branch: m12-pr18-test-temporal-resume.
Follow-ups
- m12-pr20+: extract persistence helpers (_persist_*, _ensure_*, _sync_*)
- m12-pr21+: extract runtime executor (_run_generation_job_*)
- Final TD-1 close-out: route handlers remain in jobs.py
There was a problem hiding this comment.
Pull request overview
This PR is the first TD-1 split of the large backend/app/api/routes/generate/jobs.py, extracting “module-state / preferred-lock” pure helpers into a new sibling module while preserving the legacy app.api.routes.generate.jobs._foo import paths via re-imports.
Changes:
- Added
backend/app/api/routes/generate/_module_state.pycontaining module-state constants/helpers and the bootstrap task shim. - Removed the extracted helper implementations from
jobs.pyand re-imported them from._module_stateto keep existing imports /mock.patch()targets stable. - Updated
context/CHANGELOG_INTELLIGENCE.mdto record m12-pr19 in the PR ledger.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| context/CHANGELOG_INTELLIGENCE.md | Adds a ledger entry for m12-pr19 and updates m12-pr18’s PR reference. |
| backend/app/api/routes/generate/jobs.py | Removes inlined helper implementations and re-imports them from ._module_state to reduce file size while preserving legacy symbol paths. |
| backend/app/api/routes/generate/_module_state.py | New module containing the extracted “pure” helpers/constants and bootstrap shim. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # resolve and existing `mock.patch("app...jobs._foo")` call sites stay | ||
| # valid. The bootstrap registry is also imported through the helper | ||
| # module so `_BOOTSTRAP_TASKS` keeps its alias to the same set. | ||
| from ._module_state import ( |
Comment on lines
+71
to
+75
| _DEFAULT_REQUESTED_MODULES, | ||
| _SNAKE_TO_CAMEL, | ||
| _CAMEL_TO_SNAKE, | ||
| _IDENTITY_KEYS, | ||
| _now_ms, |
Comment on lines
+76
to
+77
| _apply_preferred_lock, | ||
| _default_module_states, |
Comment on lines
+3
to
+4
| Extracted from `app.api.routes.generate.jobs` to bring that module under | ||
| 2 kLOC. These are pure utilities for module-key normalisation, default |
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
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.
m12-pr19: TD-1 first cut — extract module-state helpers from routes/generate/jobs.py
The routes/generate/jobs.py module had grown to 2496 LOC. This PR is
the first cut of TD-1 (split jobs.py): extract the cohesive group of
pure module-state / preferred-lock helpers into a sibling module
_module_state.py(195 LOC) and re-import them in jobs.py so allexisting import paths and
mock.patch("app.api.routes.generate.jobs._foo")call sites continue to resolve unchanged.
Result
Extracted (12 names + 1 re-export of _bootstrap_registry)
Why this cut first
dependency. The only side-effecting helper (_track_bootstrap) is a
thin shim over bootstrap_registry which moves with it.
handlers) is more entangled with DB clients, Temporal handles, and
SSE streaming — those should follow in subsequent PRs after a deeper
survey, not in a single 2500-line megamove.
Validation
tests/temporal/ -x → 109 passed in 2.94s
mock.patch("app.api.routes.generate.jobs._foo")paths stillresolve (verified by the smoke test suite which patches several of
these names).
bootstrap_registry uses internally (confirmed in module re-export
block).
Stacked on PR #36 (m12-pr18). Base branch: m12-pr18-test-temporal-resume.
Follow-ups