Skip to content

Refactor schedule heatmap styling to use centralized theme styles with NO_COLOR-aware degradation - #48223

Merged
pelikhan merged 14 commits into
mainfrom
copilot/replace-inline-lipgloss-style
Jul 27, 2026
Merged

Refactor schedule heatmap styling to use centralized theme styles with NO_COLOR-aware degradation#48223
pelikhan merged 14 commits into
mainfrom
copilot/replace-inline-lipgloss-style

Conversation

CopilotAI commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

pkg/cli/compile_schedule_calendar.go still had direct, inline Lipgloss style construction for heatmap cells, diverging from the shared pkg/styles path and bypassing standard color degradation behavior. This PR aligns the calendar renderer with centralized schedule style tokens and ensures rendered output respects environment-driven color suppression.

  • Style source consolidation

    • Removed inline fallback style construction from intensityStyle.
    • intensityStyle now only maps intensity buckets to centralized styles.ScheduleCalendar* variables.
  • TTY + color-profile rendering path

    • Added a small renderer helper that:
      • returns plain text for non-TTY output
      • otherwise renders with centralized style and passes through colorwriter.Degrade(...) using process environment
    • This routes calendar cell/legend coloring through the same NO_COLOR/COLORTERM-aware degradation path used elsewhere.
  • Call-site simplification

    • Removed isTerminal from intensityStyle API.
    • Updated calendar grid and legend rendering to use the new centralized render helper.
  • Behavioral illustration

    funcrenderScheduleCalendarCell(countint, textstring, isTerminalbool, environ []string) string {
    if!isTerminal {
    returntext
    }
    returncolorwriter.Degrade(intensityStyle(count).Render(text), environ)
    }

Run: https://github.com/github/gh-aw/actions/runs/30221620246

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.7 AIC · ⌖ 7.23 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30223671533

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.5 AIC · ⌖ 7.96 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30224763533

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 9.68 AIC · ⌖ 9.81 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30228781602

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.4 AIC · ⌖ 8.55 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30230557448

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 4.62 AIC · ⌖ 7.86 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30233154470

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 13 AIC · ⌖ 7.3 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30240308794

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 23.9 AIC · ⌖ 8.09 AIC · ⊞ 7.1K ·
Comment /souschef to run again

…gradation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title [WIP] Refactor compile_schedule_calendar.go to use centralized style varsRefactor schedule heatmap styling to use centralized theme styles with NO_COLOR-aware degradationJul 26, 2026
CopilotAI requested a review from pelikhanJuly 26, 2026 20:34
@pelikhan
pelikhan marked this pull request as ready for review July 26, 2026 20:48
CopilotAI review requested due to automatic review settings July 26, 2026 20:48
@github-actions

github-actionsBot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actionsBot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

@github-actions

github-actionsBot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #48223 does not have the 'implementation' label and has only 27 new lines of code in business logic directories (threshold: 100).

@github-actions

github-actionsBot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Centralizes schedule heatmap styling and adds environment-aware color degradation.

Changes:

  • Reuses shared schedule calendar styles.
  • Suppresses ANSI output for non-TTY and NO_COLOR environments.
  • Adds rendering-path tests.
Show a summary per file
FileDescription
pkg/cli/compile_schedule_calendar.goCentralizes and degrades cell styling.
pkg/cli/compile_schedule_calendar_test.goTests plain-text rendering paths.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Medium

Comment threadpkg/cli/compile_schedule_calendar.go Outdated
Comment threadpkg/cli/compile_schedule_calendar_test.go

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The refactor looks correct and well-structured.

  • intensityStyle now cleanly maps buckets to centralized style tokens with no inline fallbacks.
  • TTY-gating and NO_COLOR/COLORTERM degradation are handled in the new renderScheduleCalendarCell helper, consistent with the rest of the codebase.
  • os.Environ() is captured once per displayScheduleCalendar call, not per cell.
  • Tests cover both non-TTY and NO_COLOR suppression paths.

No actionable issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 18.4 AIC · ⌖ 7.99 AIC · ⊞ 5K

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /codebase-design — approving with one minor observation on redundant test assertions.

📋 Key Themes & Highlights

Key Themes

  • Style consolidation is clean: intensityStyle now delegates entirely to centralized styles.Schedule* tokens — good deep module behavior.
  • renderScheduleCalendarCell is a well-named, single-responsibility helper. Moving the TTY gate and colorwriter.Degrade call here is the right seam.
  • Test coverage is solid — both the non-TTY path and the NO_COLOR degradation path are explicitly exercised.

Minor Observation (inline comment)

  • The NotContains(t, got, "\x1b[") assertion after an Equal(t, got, text) assertion is redundant — if they're equal, the ANSI check can't independently fail. Keeping only the Equal assertion produces a cleaner failure message.

Positive Highlights

  • ✅ Clean removal of the isTerminal parameter from intensityStyle — simpler API
  • os.Environ() captured once and threaded through, avoiding repeated syscalls
  • ✅ Both call sites (grid and legend) updated consistently

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 20.6 AIC · ⌖ 4.65 AIC · ⊞ 6.7K
Comment /matt to run again

Comment threadpkg/cli/compile_schedule_calendar_test.go
@github-actionsgithub-actionsBot mentioned this pull request Jul 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100 — Excellent

Analyzed 2 test(s): 2 design, 0 implementation, 0 violation(s).

📊 Metrics (2 tests)
MetricValue
Analyzed2 (Go: 2, JS: 0)
✅ Design2 (100%)
⚠️ Implementation0 (0%)
Edge/error coverage2 (100%)
Duplicate clusters0
InflationNo (0.93:1)
🚨 Violations0

Test Classification:

TestFileDesign InvariantClassification
TestRenderScheduleCalendarCell_NoANSIWhenNotTerminalcompile_schedule_calendar_test.go:208Non-TTY outputs must strip ANSI escape codesdesign_test / high_value
TestRenderScheduleCalendarCell_NoANSIWhenNoColorcompile_schedule_calendar_test.go:217NO_COLOR environment variable must be honored, stripping ANSIdesign_test / high_value

Quality Signals

Clean design refactoring — Both tests verify the new renderScheduleCalendarCell function's core contract: degrading styled output to plain text under non-TTY or NO_COLOR conditions. Each test exercises 5 intensity levels, ensuring the behavior is consistent across the supported range.

Edge-case coverage — Tests explicitly verify the behavioral boundary: that ANSI escapes are absent in constrained environments, not just that plain text is returned.

No test inflation — Test file adds 13 lines against 14 lines in production (0.93:1 ratio), indicating focused, non-redundant test additions.

Verdict

passed. 0% implementation tests (threshold: 30%), 100% design tests, no violations.

🧪 Test quality analysis by Test Quality Sentinel · haiku45 · 12.4 AIC · ⌖ 7.64 AIC · ⊞ 8.1K ·
Comment /review to run again

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 100/100. 0% implementation tests (threshold: 30%), 100% design tests, no violations.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One non-blocking issue

The refactor cleanly centralizes style tokens and adds NO_COLOR-aware degradation. One concern worth addressing: intensityStyle().Render() now always executes on a TTY, even when colorwriter.Degrade will immediately discard the ANSI output (e.g. NO_COLOR=1). The previous design short-circuited before Render in that case. See the inline comment for a suggested fix.

🔎 Code quality review by PR Code Quality Reviewer · sonnet46 · 27.9 AIC · ⌖ 4.68 AIC · ⊞ 5.7K
Comment /review to run again

Comment threadpkg/cli/compile_schedule_calendar.go
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, bring this branch to closure on the remaining review feedback below, and reply with whether it is ready for maintainer review.

Unresolved review feedback (newest first):

Run: https://github.com/github/gh-aw/actions/runs/30220597059

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 16.5 AIC · ⌖ 6.1 AIC · ⊞ 7.1K ·
Comment /souschef to run again

CopilotAIand others added 3 commits July 26, 2026 21:23
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

🤖 PR Triage

FieldValue
Categoryrefactor
Risk🟢 low
Priority Score38 / 100
Score BreakdownImpact: 18 · Urgency: 10 · Quality: 10
Recommended Action⚡ fast_track

Summary: Aligns schedule heatmap calendar renderer to use centralized pkg/styles theme tokens instead of inline Lipgloss — improves NO_COLOR-awareness and style consistency. Multiple AI APPROVALs (github-actions[bot] ✅). No pending CHANGES_REQUESTED. CI status unknown (no check-runs data).

Next action: Fast-track — verify CI passes then merge.

Generated by 🔧 PR Triage Agent · sonnet46 · 49 AIC · ⌖ 8.25 AIC · ⊞ 5.7K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address the unresolved review feedback below, and rerun checks once the branch is up to date.

Unresolved review threads:

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.4 AIC · ⌖ 8.55 AIC · ⊞ 7.1K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

CopilotAI commented Jul 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please run the pr-finisher skill, address the unresolved review feedback below, and rerun checks once the branch is up to date.

Unresolved review threads:...

Completed this pr-finisher pass.

  • The listed review threads are now resolved in GitHub (all four links).
  • Branch is up to date with main (d4872c2 on origin/main, already included).
  • Re-ran checks:
    • make fmt
    • make lint
    • make test-unit ✅ (impacted package run)
    • make test ❌ due pre-existing unrelated failure: TestFallbackAWFilesMatchesLocalAWDirectory in pkg/cli/copilot_agents_test.go (fallback AW file list still expects linter-workflows.md).
  • Reviewed workflow run 30228781602: completed success; get_job_logs(failed_only=true) shows no failed jobs.

No additional PR-file changes were made in this pass. CI on the current agent head remains unverified until a maintainer re-triggers workflows.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address unresolved review feedback and any failing checks, then rerun the relevant checks once the branch is ready for maintainer review.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 4.62 AIC · ⌖ 7.86 AIC · ⊞ 7.1K ·
Comment /souschef to run again

CopilotAI commented Jul 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please run the pr-finisher skill, address unresolved review feedback and any failing checks, then rerun the relevant checks once the branch is ready for maintainer review.

...

Completed a fresh pr-finisher pass.

  • Unresolved in-scope review threads: none (all current Copilot/GitHub Actions threads on this PR are resolved).
  • Branch status: up to date with main (current branch HEAD contains origin/main at d4872c2).
  • CI run check: reviewed run 30230557448; get_job_logs(failed_only=true) reports no failed jobs.
  • Local checks rerun:
    • make fmt
    • make lint
    • make test-unit ✅ (after fetching origin/main for merge-base)
    • go test ./pkg/cli -run ScheduleCalendar -count=1
    • make test ❌ with pre-existing unrelated failure: TestFallbackAWFilesMatchesLocalAWDirectory in pkg/cli/copilot_agents_test.go (fallback AW file list expects linter-workflows.md).

No additional PR-file changes were required in this pass. From the scope of this PR, it is ready for maintainer review; CI on the latest agent head still requires maintainer re-trigger for fresh required-check status.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, update the branch if GitHub allows it, and move this PR toward merge readiness.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 13 AIC · ⌖ 7.3 AIC · ⊞ 7.1K ·
Comment /souschef to run again

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address the failed checks and confirm whether any unresolved review feedback remains on the current head.

Failed checks:

Please summarize whether this PR needs code changes, only a re-run on current head, or is blocked by an unrelated baseline issue.

Generated by 👨🍳 PR Sous Chef

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 23.9 AIC · ⌖ 8.09 AIC · ⊞ 7.1K ·
Comment /souschef to run again

Comment threadpkg/cli/compile_schedule_calendar.go Outdated
// presence of NO_COLOR disables color regardless of its value.
func hasNoColorEnviron(environ []string) bool {
for _, envVar := range environ {
if strings.HasPrefix(envVar, "NO_COLOR=") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot there are helpers to detect no color envs, reuse

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Updated in 4eb3843: removed the local hasNoColorEnviron helper and now reuse console.IsAccessibleMode() in renderScheduleCalendarCell for no-color/accessibility suppression.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Jul 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot please run the pr-finisher skill, address the failed checks and confirm whether any unresolved review feedback remains on the current head.

Failed checks:...

Ran a fresh pr-finisher pass on current head.

  • Code changes needed: yes, one minimal fix for unresolved review feedback from @pelikhan (4eb3843) to reuse existing no-color/accessibility helper logic.
  • Failed checks in run 30229323446: both test and impacted-go-tests fail on TestFallbackAWFilesMatchesLocalAWDirectory (pkg/cli/copilot_agents_test.go), which is unrelated baseline/test-data drift, not caused by this schedule-calendar PR.
  • Local validation on current head: make fmt, make lint, make test-unit, make test, and make recompile all pass.
  • Unresolved review feedback on current head: none in scope beyond the @pelikhan thread now addressed by 4eb3843.

Conclusion: this PR has the required code fix and is ready for maintainer review; CI on latest head still requires maintainer re-trigger for fresh required-check status.

CopilotAI requested a review from pelikhanJuly 27, 2026 06:17
@pelikhan
pelikhan merged commit 5fd0a1b into mainJul 27, 2026
27 checks passed
@pelikhan
pelikhan deleted the copilot/replace-inline-lipgloss-style branch July 27, 2026 06:35
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.83.4

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code Quality] Replace inline lipgloss.NewStyle() in compile_schedule_calendar.go with centralized pkg/styles vars

4 participants

@gh-aw-bot@pelikhan