Dataface Tasks

Finish visual workflow migration to local review and remove GitHub approval gate

IDINFRA_TOOLING-FINISH_VISUAL_WORKFLOW_MIGRATION_TO_LOCAL_REVIEW_AND_REMOVE_GITHUB_APPROVAL_GATE
Statuscompleted
Priorityp1
Milestonem1-ft-analytics-analyst-pilot
Ownersr-engineer-architect
Completed bydavefowler
Completed2026-03-24

Problem

Complete the unfinished visual-regression workflow migration. Make local/mac review the source of truth, remove the separate GitHub visual approval gate, stop hard-blocking PRs on Linux visual snapshot validation, and update docs/workflows so CI reflects the intended non-blocking review flow from PR 772.

Context

  • PR #772 and its task promised a non-blocking local-review workflow with no separate GitHub visual-approvals gate, but the repo still keeps both .github/workflows/test.yml Linux visual validation and .github/workflows/approve-visuals.yml.
  • Current docs and test messaging still describe Linux CI as the source of truth, which is the opposite of the intended migration.
  • A later rendering PR (#774, serif chart titles) changed snapshot output but did not land approved goldens. Because GitHub still runs Linux visual checks, unrelated PRs inherit those failures.
  • Relevant files:
  • .github/workflows/test.yml
  • .github/workflows/approve-visuals.yml
  • justfile
  • tests/visual/test_visual_snapshots.py
  • docs/docs/contributing/visual-tests.md
  • PR templates and scripts/visual-report

Possible Solutions

  • Recommended: Make local reviewed snapshots authoritative, remove GitHub visual approval automation, and stop running visual snapshot validation in CI. Keep just check-visuals, just approve-visuals, and just visual-report as local tools used before opening or updating a PR. This matches the task intent from #772 and prevents unrelated PR blockage from Linux drift.
  • Keep Linux CI validation but delete only the approval workflow. This removes one layer of friction but preserves the main source of false failures, so it does not solve the inherited-red-PR problem.
  • Move validation to macOS GitHub runners. This would make CI closer to the desired source of truth, but it still keeps snapshots as a hard PR gate and adds runner cost/latency. It also exceeds the narrower migration implied by the unfinished task.

Plan

  1. Remove GitHub-side visual approval and Linux visual validation from the CI workflows.
  2. Update local commands/docs so mac-local review is the documented source of truth.
  3. Update snapshot failure messaging so it no longer points contributors at a removed GitHub approval workflow.
  4. Validate task metadata plus targeted repo checks for the updated workflow.

Implementation Progress

  • 2026-03-24: Created follow-up task after confirming the original #772 task was marked complete even though the GitHub approval gate and Linux CI visual validation were still present.
  • 2026-03-24: Chosen implementation is to finish the migration by removing the GitHub workflow gate and making visual review an explicit local pre-PR step.
  • 2026-03-24: Updating CI workflows, docs, and snapshot failure guidance in the repo so behavior matches the intended non-blocking model.
  • 2026-03-24: Removed .github/workflows/approve-visuals.yml and deleted Linux visual validation from .github/workflows/test.yml; CI now aligns with the local-review model instead of blocking unrelated PRs on snapshot drift.
  • 2026-03-24: Updated tests/visual/test_visual_snapshots.py, docs/docs/contributing/visual-tests.md, justfile, and docs/agent/cbox.md so contributor guidance points at local review/reporting rather than the deleted GitHub approval flow.
  • 2026-03-24: Verification confirmed the repo still has pending visual debt from the serif-title rendering change: uv run pytest tests/visual/test_visual_snapshots.py -q fails with 31 snapshot diffs until a human reviews and updates the goldens on macOS. That debt is now non-blocking in GitHub CI, which is the intended outcome of this migration task.

QA Exploration

N/A — workflow/docs/task-system change; no browser feature behavior changed.

  • [x] QA exploration completed (or N/A for non-UI tasks)

Review Feedback

  • [ ] Review cleared