Skip to content

Split configured cycle summary and history helpers#649

Merged
ktalpay merged 1 commit into
developfrom
codex/extract-cycle-summary-and-history-helpers
Jun 2, 2026
Merged

Split configured cycle summary and history helpers#649
ktalpay merged 1 commit into
developfrom
codex/extract-cycle-summary-and-history-helpers

Conversation

@ktalpay

@ktalpay ktalpay commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Reduce configured_cycle_runner.py responsibilities by extracting summary output and run-history persistence helpers to improve modularity and avoid circular imports.
  • Preserve existing runtime behavior, stdout text contract, and public imports so callers and tests remain compatible.

Description

  • Added src/carbonfactor_parser/pipeline/configured_cycle_summary.py and moved emit_configured_cycle_summary, _download_status_value, and _parse_status_value into it while preserving exact emitted lines and redaction semantics.
  • Added src/carbonfactor_parser/pipeline/configured_cycle_history.py and moved _persist_configured_cycle_history logic as persist_configured_cycle_history that returns a ConfiguredCycleResult and preserves non-fatal, sanitized persistence behavior.
  • Introduced src/carbonfactor_parser/pipeline/configured_cycle_models.py to host ConfiguredCycleResult and ConfiguredCycleRunnerResult to avoid circular imports and keep dataclass APIs unchanged, and preserved backwards-compatible exports from configured_cycle_runner.py for these symbols and emit_configured_cycle_summary.
  • Updated configured_cycle_runner.py to import and use emit_configured_cycle_summary and persist_configured_cycle_history, keeping the runner focused on startup, cycle loop, cycle construction, and aggregation only; narrowed a type-check import in ingestion_run_history_mapping.py to the new models module.

Testing

  • Ran focused runner tests with python -m pytest tests/test_configured_cycle_runner.py tests/test_run_ingestion_summary_cli.py and they passed.
  • Ran run-history tests with python -m pytest tests/test_ingestion_run_history_mapping.py tests/test_ingestion_run_history_repository.py tests/test_ingestion_run_history_reader.py and they passed.
  • Ran full test suite with python -m pytest and it passed (2148 passed, 8 skipped in this environment).
  • Ran linting with python -m ruff check src/carbonfactor_parser/pipeline/configured_cycle_runner.py src/carbonfactor_parser/pipeline/configured_cycle_summary.py src/carbonfactor_parser/pipeline/configured_cycle_history.py tests/test_configured_cycle_runner.py which passed; also validated git diff --check with no issues.
  • Added a focused test to assert history persistence exceptions are sanitized and non-fatal, and verified import-compatibility assertions for moved symbols.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 62cbf0f889

ℹ️ 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".

class ConfiguredCycleRunnerResult:
"""All cycles run by one application invocation."""

status: ConfiguredCycleRunnerStatus

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep runner status resolvable at runtime

When ConfiguredCycleRunnerResult is imported from the new models module, the status annotation is stored as the string ConfiguredCycleRunnerStatus, but that name only exists under TYPE_CHECKING. Runtime consumers that introspect this public result model with typing.get_type_hints(ConfiguredCycleRunnerResult) now get NameError: name 'ConfiguredCycleRunnerStatus' is not defined; this worked before because the enum and dataclass lived in the same module. This affects serializers/schema builders or other API tooling that resolves annotations, so the enum should be moved/re-exported into a runtime-resolvable namespace instead of being only type-checking imported.

Useful? React with 👍 / 👎.

@ktalpay
ktalpay merged commit 89c4a1c into develop Jun 2, 2026
2 checks passed
@ktalpay
ktalpay deleted the codex/extract-cycle-summary-and-history-helpers branch June 2, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant