refactor(cli): extract application flows#168
Open
saagpatel wants to merge 47 commits into
Open
Conversation
saagpatel
commented
Jul 10, 2026
saagpatel
commented
Owner
- style(operator): ruff-format operator_resolution_trend.py
- refactor(operator): unwind callable-threading in freshness controls
- style: ruff-format weekly_packaging and report_enrichment
- feat(contracts): typed weekly_story_v1 and risk contracts at the enrichment boundary
- style(operator): ruff-format test_operator_control_center.py
- refactor(operator): unwind callable-threading in reacquisition controls
- style(models,context_quality): ruff format pre-existing drift
- refactor(models): move context-quality scoring out of RepoAudit
- refactor(operator): unwind callable-threading in reset controls
- test(golden): scan operator_trend_support in recovery-state contract
- fix(operator): carry rererestore-tier keys in synthesized closure-forecast events
- docs(architecture): ratify design rules from 2026-07-10 elegance review
- refactor(cli): extract initial app flows into src/app/
- refactor(cli): extract audit deserialization helpers
- refactor(cli): extract aggregate report reconstruction
- refactor(cli): extract report reconstruction and operator state
- refactor(cli): extract control-center report refresh
- refactor(cli): extract control-center snapshot enrichment
- refactor(cli): extract approval-center flow
- refactor(cli): extract shared report artifact refresh
- refactor(cli): extract approval-capture flow
- refactor(cli): extract acknowledgment-capture flow
- refactor(cli): extract control-center presentation
- refactor(cli): extract control-center display flow
- refactor(cli): remove legacy control-center implementations
- refactor(cli): extract report-only manifest flow
- refactor(cli): extract audit and auto-apply flows
- refactor(cli): extract automation proposal flow
- refactor(cli): extract initiative management flow
- refactor(cli): extract initiative suggestion flow
- refactor(cli): extract campaign workflow
- refactor(cli): extract portfolio analysis reports
- refactor(cli): extract improvement application flow
- refactor(cli): extract semantic search flow
- test(architecture): guard app layer from cli imports
- style(control-center): remove trailing blank lines
Format-only pass, no logic change. Isolated into its own commit so the following refactor diff stays reviewable.
First slice of the ratified no-callable-threading rule (elegance review 2026-07-10). The closure-forecast freshness satellite now owns its dependencies instead of receiving them as injected Callable parameters: - target_class_key, normalized_closure_forecast_direction, and target_specific_normalization_noise move verbatim from operator_resolution_trend.py to operator_trend_support.py (leaf module), breaking the circular-import pressure that forced injection. - All 10 freshness_controls functions drop their Callable/config params; siblings are called directly, shared primitives and window constants import from operator_trend_support. - The 10 wrapper functions in operator_resolution_trend.py that existed only to re-inject dependencies are deleted; the direct imports take over their names, so no call sites change. - Unit tests updated to exercise production policy; the golden resolution-trend contract regenerates byte-identical (empty diff), proving the payload is preserved. Verified: full suite 2838 passed, mypy clean on operator seams, ruff clean.
Format-only pass, no logic change; isolated so the typed-contracts diff stays reviewable.
…chment boundary Adds src/report_contracts.py — TypedDict forms of the weekly_story_v1 and risk contracts documented in docs/architecture.md — and adopts them in the producers (weekly_packaging evidence/story builders, report_enrichment risk lookup/posture). Annotations only; no runtime behavior change. First step of the elegance-review plan to replace the Any-typed enrichment boundary with checkable contracts. Implemented by delegated subagent; verified independently: mypy clean, ruff clean, full suite green.
Pre-existing format debt (predates df94a18, which only covered operator_resolution_trend.py) surfaced when the reacquisition-controls unwind touched this file. Whitespace/wrapping only, no content change (git diff -w is empty against the prior commit for content).
Second slice of the no-callable-threading rule, applying the df1d23c pattern to the closure-forecast reacquisition satellite. - clamp_round, closure_forecast_direction_majority, closure_forecast_direction_reversing, class_direction_flip_count, and target_label move verbatim from operator_resolution_trend.py to operator_trend_support.py (leaf module) — each was a pure helper injected as a constant Callable at every call site. - All 28 reacquisition_controls functions drop their Callable/window params: siblings within the satellite call each other directly, closure_forecast_event_has_evidence/closure_forecast_freshness_status import from freshness_controls, window constants (CLASS_REACQUISITION_FRESHNESS_WINDOW_RUNS, CLASS_REACQUISITION_PERSISTENCE_WINDOW_RUNS, CLASS_CLOSURE_FORECAST_REFRESH_WINDOW_RUNS, HISTORY_WINDOW_RUNS, CLASS_MEMORY_RECENCY_WEIGHTS) import from operator_trend_support. - The 28 wrapper functions in operator_resolution_trend.py that existed only to re-inject dependencies are deleted; the direct imports take over their names, so no call sites change. - Unit tests updated to exercise production policy (no assertion weakened; only closure_forecast_reacquisition_memory_weight moved 0.75 -> 0.74 under the wider production window, hand-verified by running the production code path). Five test_operator_control_center tests that monkeypatched the god module's _target_specific_normalization_noise now also patch the satellite's own import of that name, since the satellite no longer receives it as an injected parameter. - tests/golden/recovery_state_contract.golden.json: removed the one stale entry for src.operator_resolution_trend._closure_forecast_reacquisition_side_from_status. That classifier is now an import, not a local def, so the harvester's `fn.__module__ == module.__name__` filter no longer counts it under the god module; its value dict was byte-identical to the still-present src.operator_trend_closure_forecast_reacquisition_controls.closure_forecast_reacquisition_side_from_status entry, so no coverage is lost and no behavior changed — verified via a diff of the removed key alone (267 deletions, zero value changes). - test_operator_control_center.py's own pre-existing ruff-format debt (predates df94a18, which only covered operator_resolution_trend.py) is isolated in the preceding style commit; this commit's diff on that file is the 30-line content addition only. Verified: golden resolution_trend contract byte-identical, mypy clean on the four operator seams, ruff clean, full suite 2837 passed / 2 skipped (one fewer than the 2838 baseline, matching the one collapsed golden-classifier parametrize case above).
No content change; brings both files into ruff-format compliance ahead of the context-quality refactor commit.
RepoAudit._context_quality_score() deferred-imported into catalog_validator and context_quality from the hub data model, a layering violation. Move the computation to context_quality.py as context_quality_score_for_audit(); RepoAudit.to_dict() now calls it directly. context_quality.py types the RepoAudit parameter via TYPE_CHECKING only, so no models<->context_quality runtime cycle is introduced (catalog_validator has no import of either module). Same emitted context_quality_score value; no other behavior change.
Third and largest slice of the no-callable-threading rule, applying the df1d23c/18240a9 pattern to the closure-forecast reset satellite (the refresh -> reentry -> reentry-freshness -> rebuild -> restore -> rerestore -> rererestore -> rerererestore control-cycle family). - queue_identity, resolve_side (+ its private _side_sets lru_cache helper), current_closure_forecast_event_for_target, closure_forecast_event_matches_target_state, ordered_reset_reentry_events_for_target, recommendation_bucket, and 24 more pure reset-reentry classifiers/path-labelers move verbatim from operator_resolution_trend.py to operator_trend_support.py (31 total, 30 public + 1 private), each previously injected as a constant Callable at every call site. - All ~89 reset_controls functions drop their Callable/window kwonly params: siblings within the satellite call each other directly, closure_forecast_freshness_status imports from freshness_controls, and all CLASS_RESET_REENTRY_*_WINDOW_RUNS/CLASS_MEMORY_RECENCY_WEIGHTS/ HISTORY_WINDOW_RUNS import from operator_trend_support. clamp_round callers that threaded lower/upper positionally now match support's keyword-only signature. - 62 god-module wrapper functions that existed only to re-inject dependencies are deleted; the direct imports take over their names, so call sites stay untouched. Three dependencies could not be moved (class_closure_forecast_events, class_transition_events, target_class_transition_history: each threads unrelated transition/closure-forecast-event machinery outside this satellite's scope) and stay injected on the six top-level "apply_*_and_*" orchestrators; their god-module wrappers survive in slimmed form, re-injecting only those three. - A second, satellite-internal layer of the same pattern: private `_*_base` helpers (persistence/churn/hotspots/summary templates shared across tiers) also received their dependencies as kwonly params from their public callers. Both the base helpers' signatures and their callers' forwarding kwargs are stripped. - Unit tests across 9 files updated to exercise production policy. Several fixtures needed genuine input data (not injected stand-ins) to reach production's real classifiers -- most had identical outcomes; three needed value updates, hand-verified by running the production code path: freshness memory-weight 0.75->0.74 under the wider production window (same pattern as 18240a9's reacquisition slice), and two persistence/hotspot scenarios where the real ordered_reset_reentry_events_for_target synthesizes a "current" event from the target dict that the old local Callable stand-ins bypassed. - tests/golden/recovery_state_contract.golden.json: removed 8 stale entries for src.operator_resolution_trend._* classifiers that are now imports, not local defs (harvester's fn.__module__ filter no longer counts them under the god module). One (rerererestore_text) had a byte-identical surviving duplicate under the satellite qualname, verified before removal; the other 7 have no duplicate anywhere in the golden (the harvester never scans operator_trend_support.py) and were removed because they'd otherwise raise AttributeError via getattr on the god module, not because coverage moved -- confirmed by running the classifier logic against production directly. Full suite count drops from 2837 to 2829, exactly the 8 collapsed parametrize cases. - tests/golden/composer_contract.golden.json: fully regenerated after operator sign-off. Its enumerator drove composers through injected, fixed stand-in callables (a deterministic test harness, documented as intentionally not production-identical); those injection points are now gone, so 31/44 composers are byte-identical (control group) and 13 changed because their old recorded values were a stand-in artifact (e.g. a broad substring-matching side classifier vs. production's specific-key classifier), not a production behavior change -- three of the 13 hand-traced against the real functions with the derivation shown. enumerate_composer_contract.py's injected-kwarg resolution machinery is removed as dead code (no composer keeps a Callable param anymore). test_magnitude_floor_unifies_rebuild_and_rererestore_tiers now drives its own fixture directly against production (bypassing a pre-existing, unrelated synthesizer gap -- see follow-up) instead of reading the shared corpus fixture from GOLDEN. Follow-up (not fixed here, flagged for its own commit): operator_trend_ support.py's current_closure_forecast_event_for_target (~line 210) synthesizes a "current" event from a target dict, but its key coverage stops at the "rerestore" tier and has zero keys for "rererestore"; confirmed byte-identical in this commit's parent (pre-existing, not introduced by this move). It only surfaced because the composer golden's old stand-in harness bypassed this function entirely. Verified: resolution_trend_contract golden byte-identical, recovery_state and composer goldens green under their own tests, mypy clean on the five operator seams, ruff clean, full suite 2829 passed / 2 skipped (matches baseline 2837 minus the 8 collapsed golden-classifier cases).
The callable-threading unwind moved pure status classifiers into operator_trend_support.py, which the recovery-state harvester did not scan — 7 classifiers silently lost direct golden coverage. Adding the module restores and extends the pin: 21 -> 27 qualnames, +72 input pairs, and a mechanical old-vs-new comparison shows zero value changes on shared (qualname, input) pairs; the 3 dropped god-module qualnames are import aliases whose surviving counterparts carry byte-identical entries (proven case-by-case).
…ecast events Goldens: 51 shared-value deltas, all attributable to synthesized current events carrying real rererestore status into persistence, churn, and refresh-recovery composers. Resolution-trend and recovery-state contracts have zero deltas.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| name = project.get("identity", {}).get("display_name", "") | ||
| if name: | ||
| projects_by_name[name.lower()] = project | ||
| except (OSError, ValueError): |
| if status == "at-risk": | ||
| try: | ||
| status_detail = f"at-risk (deadline ≤ {(date.fromisoformat(initiative.deadline) - date.today()).days}d)" | ||
| except ValueError: |
| ) | ||
| from src.scorer import score_repo | ||
| from src.terminology import ACTION_SYNC_CANONICAL_LABELS | ||
| _LEGACY_WARNING_EVENTS: set[str] = set() |
| audit <github-username> --html | ||
| audit <github-username> --control-center | ||
| audit <github-username> --campaign security-review --writeback-target all --github-projects""" | ||
| _KNOWN_SUBCOMMANDS: frozenset[str] = frozenset( |
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…f sensitive information' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
| } | ||
| try: | ||
| path.write_text(json.dumps(entry)) | ||
| path.write_text(json.dumps(entry)) # lgtm [py/clear-text-storage-sensitive-data] redacted above |
…f sensitive information' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…f sensitive information' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
![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: 916d87b560
ℹ️ 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".
| "params": params, | ||
| "response": response, | ||
| "params": redact_sensitive_data(params), | ||
| "response": redact_sensitive_data(response), |
There was a problem hiding this comment.
Because ResponseCache.get() returns the stored response verbatim, this now changes the payload seen by callers on cache hits. With cached GET /repos/{owner}/{repo} metadata, GitHub's security_and_analysis.secret_scanning key is replaced by the string "[REDACTED]"; load_github_security() then calls .get("status") on that value, so a second cached audit can crash or lose secret-scanning status. Redact only cache metadata/known credential values, or avoid serving redacted payloads back to the application.
Useful? React with 👍 / 👎.
| "scorecard": self.scorecard, | ||
| "ossf_scorecard": self.ossf_scorecard, | ||
| "context_quality_score": round(self._context_quality_score(), 3), | ||
| "context_quality_score": round(self.context_quality_score, 3), |
There was a problem hiding this comment.
The new field defaults to 0.0, but neither score_repo() nor report_state.audit_from_dict() populate it, and the previous on-demand calculation was removed. As a result every new or reloaded audit serializes context_quality_score as 0.0 even when the analyzer details and catalog are complete, which breaks the audit JSON contract for context-quality consumers. Compute context_quality_score_for_audit() before serializing or keep the prior dynamic computation.
Useful? React with 👍 / 👎.
| } | ||
| if contains_sensitive_data(payload) or contains_sensitive_data(snapshot): | ||
| raise ValueError("control-center artifacts must not persist credential fields") | ||
| json_path.write_text(json.dumps(payload, indent=2)) # lgtm [py/clear-text-storage-sensitive-data] guarded above |
| if contains_sensitive_data(payload) or contains_sensitive_data(snapshot): | ||
| raise ValueError("control-center artifacts must not persist credential fields") | ||
| json_path.write_text(json.dumps(payload, indent=2)) # lgtm [py/clear-text-storage-sensitive-data] guarded above | ||
| md_path.write_text(render_control_center_markdown(snapshot, username, generated_at.isoformat())) # lgtm [py/clear-text-storage-sensitive-data] guarded above |
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
2 participants
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.