refactor: migrate output helpers to drift-output package (ADR-100 phase 4c)#720
Open
mick-gsk wants to merge 1 commit into
Open
refactor: migrate output helpers to drift-output package (ADR-100 phase 4c)#720mick-gsk wants to merge 1 commit into
mick-gsk wants to merge 1 commit into
Conversation
Owner
Author
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR continues the ADR-100 monorepo split by moving output/rendering helpers into a new drift-output workspace package and leaving thin compatibility stubs under src/drift/output.
Changes:
- Replaced most modules under
src/drift/outputwith import-forwarding stubs todrift_output.*. - Added a new
packages/drift-outputpackage containing the moved output/rendering implementations. - Updated workspace/dependency metadata in
pyproject.tomland added a minimal smoke test for the new package.
Reviewed changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/drift/output/tui_renderer.py | Legacy module replaced with re-export stub. |
| src/drift/output/session_renderer.py | Legacy module replaced with re-export stub. |
| src/drift/output/rich_output.py | Legacy module replaced with re-export stub. |
| src/drift/output/prompt_generator.py | Legacy module replaced with re-export stub. |
| src/drift/output/pr_comment.py | Legacy module replaced with re-export stub. |
| src/drift/output/markdown_report.py | Legacy module replaced with re-export stub. |
| src/drift/output/llm_output.py | Legacy module replaced with re-export stub. |
| src/drift/output/junit_output.py | Legacy module replaced with re-export stub. |
| src/drift/output/json_output.py | Legacy module replaced with re-export stub. |
| src/drift/output/interactive_review.py | Legacy module replaced with re-export stub. |
| src/drift/output/guided_output.py | Legacy module replaced with re-export stub. |
| src/drift/output/grouping.py | Legacy module replaced with re-export stub. |
| src/drift/output/github_format.py | Legacy module replaced with re-export stub. |
| src/drift/output/fix_plan_rich.py | Legacy module replaced with re-export stub. |
| src/drift/output/csv_output.py | Legacy module replaced with re-export stub. |
| src/drift/output/badge_svg.py | Legacy module replaced with re-export stub. |
| src/drift/output/init.py | Legacy output package replaced with top-level re-export stub. |
| pyproject.toml | Added workspace package dependencies and uv workspace config; version changed. |
| packages/drift-output/tests/test_drift_output_smoke.py | Added smoke imports for the new package. |
| packages/drift-output/src/drift_output/tui_renderer.py | Moved TUI renderer implementation into new package. |
| packages/drift-output/src/drift_output/session_renderer.py | Moved session renderer implementation into new package. |
| packages/drift-output/src/drift_output/prompt_generator.py | Moved prompt generation implementation into new package. |
| packages/drift-output/src/drift_output/pr_comment.py | Moved PR comment formatter into new package. |
| packages/drift-output/src/drift_output/markdown_report.py | Moved markdown report formatter into new package. |
| packages/drift-output/src/drift_output/llm_output.py | Moved LLM/plaintext formatter into new package. |
| packages/drift-output/src/drift_output/junit_output.py | Moved JUnit formatter into new package. |
| packages/drift-output/src/drift_output/json_output.py | Moved JSON/SARIF formatter into new package. |
| packages/drift-output/src/drift_output/interactive_review.py | Moved interactive review flow into new package. |
| packages/drift-output/src/drift_output/guided_output.py | Moved guided output helpers into new package. |
| packages/drift-output/src/drift_output/grouping.py | Moved grouping helpers into new package. |
| packages/drift-output/src/drift_output/github_format.py | Moved GitHub Actions annotation formatter into new package. |
| packages/drift-output/src/drift_output/fix_plan_rich.py | Moved fix-plan renderer into new package. |
| packages/drift-output/src/drift_output/csv_output.py | Moved CSV formatter into new package. |
| packages/drift-output/src/drift_output/badge_svg.py | Moved SVG badge renderer into new package. |
| packages/drift-output/src/drift_output/init.py | New package exports public output API. |
| packages/drift-output/pyproject.toml | Defines the new distributable drift-output package. |
| [project] | ||
| name = "drift-analyzer" | ||
| version = "2.50.0" | ||
| version = "2.49.0" |
| "drift-sdk", | ||
| "drift-engine", | ||
| "drift-config", | ||
| "rich>=13.0", |
| "drift-sdk", | ||
| "drift-engine", | ||
| "drift-config", | ||
| "rich>=13.0", |
Comment on lines
+10
to
+15
| dependencies = [ | ||
| "drift-sdk", | ||
| "drift-engine", | ||
| "drift-config", | ||
| "rich>=13.0", | ||
| ] |
| def test_core_modules_importable() -> None: | ||
| importlib.import_module("drift_output.json_output") | ||
| importlib.import_module("drift_output.rich_output") | ||
| importlib.import_module("drift_output.agent_tasks") |
Comment on lines
+1038
to
+1039
| "FALSE-FIX CHECK: if new upward imports appear elsewhere," | ||
| " the violation was shifted not fixed", |
Comment on lines
+1049
to
+1050
| "FALSE-FIX CHECK: renaming without body change will not resolve this" | ||
| " — body hash must differ", |
Comment on lines
+1074
to
+1075
| "SIDE-EFFECT NOTE: repair commits may cause transient TVS findings" | ||
| " — this stabilizes over time", |
Comment on lines
+1123
to
+1124
| "FALSE-FIX CHECK: renaming without behaviour change may be valid if" | ||
| " the new name accurately describes the implementation", |
| "text": rule_rec.description, | ||
| "markdown": f"**{rule_rec.title}**: {rule_rec.description}", | ||
| } | ||
| except (ImportError, AttributeError, KeyError, TypeError): |
| if msg_rec: | ||
| combined = f"{base_text} | {msg_rec.title}" | ||
| base_text = combined[:400] if len(combined) > 400 else combined | ||
| except (ImportError, AttributeError, KeyError, TypeError): |
| meta = get_meta(signal_type) | ||
| if meta: | ||
| return meta.signal_name | ||
| except (ImportError, AttributeError, KeyError, TypeError): |
| rec = generate_recommendation(finding) | ||
| if rec: | ||
| return rec.title[:90] | ||
| except (ImportError, AttributeError, KeyError, TypeError): |
| minutes = int(duration // 60) | ||
| seconds = int(duration % 60) | ||
| console.print(f" Duration: {minutes}m {seconds}s") | ||
| except (TypeError, ValueError): |
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.
Split from #576 (ADR-100 monorepo migration). Part of the PR decomposition into atomic, reviewable units.