Remove redundant python-dataviz imports from daily reporting workflows#41158
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
python-dataviz imports from daily reporting workflows
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #41158 does not have the 'implementation' label and has 0 new lines of code in business logic directories (4 files changed, all shell/python skill scripts removing redundant imports). |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. PR #41158 only modifies workflow files (.github/workflows/daily-issues-report.lock.yml, .github/workflows/daily-issues-report.md, .github/workflows/portfolio-analyst.lock.yml, .github/workflows/portfolio-analyst.md). Test Quality Sentinel skipped. |
There was a problem hiding this comment.
Pull request overview
This pull request reduces duplicated ambient prompt context in two agentic workflows by removing a redundant direct import of shared/python-dataviz.md where it is already brought in transitively via shared/trends.md, and re-generates the compiled lockfiles to reflect the new import graph.
Changes:
- Removed the direct
shared/python-dataviz.mdimport fromportfolio-analystanddaily-issues-reportworkflow markdown. - Kept
shared/trends.mdas the single source for the dataviz dependency (confirmedshared/trends.mdimportsshared/python-dataviz.md). - Recompiled the affected workflows so each lockfile includes only a single
runtime-importofshared/python-dataviz.md.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/portfolio-analyst.md | Removes the redundant direct shared/python-dataviz.md import, relying on shared/trends.md for the transitive dependency. |
| .github/workflows/portfolio-analyst.lock.yml | Updates compiled prompt/runtime import list to remove the duplicate dataviz import (now present once, transitively). |
| .github/workflows/daily-issues-report.md | Removes the redundant direct shared/python-dataviz.md import while keeping trend + NLP imports. |
| .github/workflows/daily-issues-report.lock.yml | Updates compiled prompt/runtime import list to remove the duplicate dataviz import (now present once, transitively). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 0
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /zoom-out and /grill-with-docs — commenting with three observations, no blockers.
📋 Key Themes & Highlights
Key Themes
- Silent
python-nlp.mdreordering: moving it from beforetrends.mdto after is arguably correct but is an undocumented side effect — worth a sentence in the PR description python-dataviz.mdpositioned afterreporting.mdin the compiled prompt fordaily-issues-report: unusual ordering that is probably fine at runtime (LLMs read full context) but worth a verify pass on the next scheduled run- Prompt-size framing: the lockfile diff reveals a net reordering of the single
python-dataviz.mdruntime-import entry rather than a removal, suggesting the compiler was already deduplicating — the benefit of this change is source-level DRY, not a token reduction
Positive Highlights
- ✅
trends.mdconfirmed to importpython-dataviz.md— the transitive dependency assumption is sound - ✅
portfolio-analyst.mdchange is minimal and precisely matches the stated goal - ✅ Both lockfiles recompiled cleanly with updated metadata hashes
- ✅ Step reordering in
daily-issues-report.lock.ymlis safe — both venv-creation steps useif [ ! -d /tmp/gh-aw/agent/venv ]guards
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 76.7 AIC · ⌖ 9.7 AIC · ⊞ 6.5K
| {{#runtime-import .github/workflows/shared/mcp/grafana.md}} | ||
| {{#runtime-import .github/workflows/shared/python-dataviz.md}} | ||
| {{#runtime-import .github/workflows/shared/trends.md}} | ||
| {{#runtime-import .github/workflows/shared/python-dataviz.md}} |
There was a problem hiding this comment.
[/grill-with-docs] The lockfile diff shows python-dataviz.md moved from before trends.md to after it — a net reordering, not a net removal. Before this PR the lockfile already contained exactly one {{#runtime-import .github/workflows/shared/python-dataviz.md}} entry (the compiler deduplicates transitive imports at compile time).
💡 Implication for the PR description
The PR description states that the duplicate import "inflated first-request prompt size". If the compiler was already deduplicating, the prompt size was not inflated at runtime — the source declaration was simply redundant. The benefit of this change is source-level DRY and clarity (removing a misleading explicit import), which is still a valid reason to merge, but the token-reduction framing may overstate the impact.
Consider updating the PR description to note: "The compiler already deduplicates transitive imports, so runtime prompt size is unchanged; this change removes the misleading direct import declaration from the source workflow."
The ambient-context optimizer flagged duplicated prompt surface in two workflows:
shared/python-dataviz.mdwas imported both directly and transitively throughshared/trends.md. This trims that duplicate context from the affected workflows and updates the compiled lockfiles accordingly.Problem
daily-issues-reportandportfolio-analysteach pulledshared/python-dataviz.mdtwice:imports:shared/trends.mdChange
shared/python-dataviz.mdimport from:.github/workflows/daily-issues-report.md.github/workflows/portfolio-analyst.mdshared/trends.mdas the single source of the dataviz dependency.Compiled output
.lock.ymlfiles no longer embed the duplicate runtime import path..github/workflows/daily-issues-report.lock.yml.github/workflows/portfolio-analyst.lock.ymlImport shape
Effect
shared/trends.mdalready brings inshared/python-dataviz.mdtransitively.