Skip to content

Add CI quality gates and extract tested publish workflow modules#118

Open
jonaraphael wants to merge 6 commits into
mainfrom
repo-hardening-ci-publish-schema
Open

Add CI quality gates and extract tested publish workflow modules#118
jonaraphael wants to merge 6 commits into
mainfrom
repo-hardening-ci-publish-schema

Conversation

@jonaraphael

Copy link
Copy Markdown
Contributor

First of three planned repo-hardening PRs (next: reusable constrained Terraform apply for the IAM-sync workflows; then ingestion/services dedup).

What this does

CI quality gates

  • New lint job in ci.yml: ruff check, terraform fmt -check, and terraform validate for terraform/envs/prod and terraform/envs/preview (Terraform pinned to the same 1.8.5 as the deploy workflows).
  • Ruff added to the dev dependency group with the default ruleset; per-file E402 ignore for the repo's sys.path-bootstrap script idiom. All 30 pre-existing findings fixed, including a dead catalog_site.normalize_locale_code helper that referenced an undefined FIELD_SAFE_LOCALE_RE (zero callers — deleted).

Publish workflow extraction

  • The ~16 inline Python heredocs in publish-dataset.yml (1,114 → 331 lines) and dataset-breaking-change-alert.yml (450 → 157 lines) moved to scripts/publish_workflow.py (14 subcommands, stdlib-only at import; only promote needs uv run). Five diverging inline copies of is_schema_target/catalog_row became one definition.
  • Bug fix: the live publish breaking-alert step built its non-dry-run argv by filtering the summary filename before deleting the --summary-json pair, which swallowed --pr-number and left an orphan positional — the live Slack send would have crashed whenever a breaking publish change was detected on a PR. Regression-tested (test_live_send_arguments_stay_paired).
  • tests/test_publish_workflow.py (28 tests) covers schema-target rules, alert dedup, generation-verified deletes, and catalog-row collection; test_publish_dataset_workflow.py now asserts step wiring/ordering/protections instead of grepping heredoc text.

Concierge split + catalog loader consolidation

  • Field profiling engine (~950 lines) extracted from publishing_concierge.py (3,975 → ~3,000) into scripts/concierge_profiling.py; the state-machine CLI is unchanged.
  • scripts/catalog_csv.py is now the single owner of catalog CSV parsing; the four divergent load_catalog() implementations delegate to it.

Asset-doc frontmatter schema gate

  • catalog_docs.py now rejects unknown top-level and admission: frontmatter keys, and tests/test_repo_asset_docs.py validates the real docs/assets/ pre-merge (previously catalog_docs.py check never ran in CI).
  • Four docs migrated from concierge-dialect admission keys (estimated_published_footprint, deprecation_exit_policy, shared_datasets_rationale) to the canonical template names with numeric GB values.
  • Bug fix: admission_check.parse_footprint_gb was unit-blind ("Roughly 90 MB" parsed as 90 GB). Footprint parsing is now unit-aware with one owner in admission_check.py; the concierge delegates to it.

Docs

  • README: skill routing table completed (4 missing skills + temp-workspace standard), layout tree corrected, authority ordering deferred to AGENTS.md as single owner; new local lint/terraform instructions.
  • scripts/README.md: sections for publish_workflow.py and the shared internal modules.

Remote paths changed

None. No canonical GCS objects are touched; no publish plan is included or needed. Workflow behavior changes take effect only for future runs after merge.

Validation

  • Full suite at this commit in an isolated worktree: 605 passed, 4 skipped (skips are pre-existing GDAL/Tippecanoe gates).
  • ruff check ., terraform fmt -check/validate (both envs, Terraform 1.8.5), repo_guardrails.py check-static, catalog_docs.py check (22 asset docs current) all pass.
  • Live-workflow caveat: the extracted publish steps are verified by unit tests and CLI parsing, not a live promotion; worth watching the first post-merge dataset mutation run.

Review

Self-authored by jonaraphael; GitHub blocks requesting review from the PR author, so per AGENTS.md this records the restricted self-acceptance path.

🤖 Generated with Claude Code

jonaraphael and others added 4 commits July 2, 2026 13:38
- CI: add a lint job (ruff, terraform fmt/validate for prod and preview
  envs) and fix all existing lint findings, including a dead catalog_site
  helper that referenced an undefined regex.
- Extract the inline Python from the approved dataset mutation and planned
  breaking-change workflows into scripts/publish_workflow.py with unit
  tests: one owner for the schema-target and catalog-row rules, and a fix
  for the live breaking-alert argv bug that swallowed --pr-number.
- Split field profiling out of publishing_concierge.py into
  scripts/concierge_profiling.py and consolidate catalog CSV loading in
  scripts/catalog_csv.py.
- Enforce canonical asset-doc frontmatter: unknown top-level and admission
  keys now fail catalog docs validation (gated pre-merge by a new
  real-docs test), migrate four docs to canonical admission field names,
  and make footprint parsing unit-aware so 90 MB no longer reads as 90 GB.
- README: complete the skill routing table and defer authority ordering
  to AGENTS.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merge origin/main (#116, #117) and migrate the two new DRC asset docs to
the canonical admission field names the frontmatter gate enforces. Gate
the planned breaking-alert send step on a detected plan so the workflow
is inert on PRs without publish/delete plans; it previously ran
unconditionally and failed on this PR because the trusted main checkout
does not yet contain scripts/publish_workflow.py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…edup

Address the PR #118 review findings:

- admission_check: footprint values now parse comma grouping correctly
  ("10,240 MB" was read as 0.24 GB, silently bypassing the >= 10 GB
  large-data-exception gate) and reject garbled numbers like "1.5.2 GB"
  loudly instead of misreading them. Regression tests added.
- publish_workflow: command_promote gains unit tests pinning stat->copy->stat
  ordering, --replace-generation conditionality, and the catalog.json
  idempotence skip; the deleted single-object-fallback assertNotIn guards are
  restored in the workflow tests.
- plan_asset_slug now fails loudly when no plan file exists; the workflow
  steps that call it only run after plan extraction, so the silent exit-0
  skip hid wiring bugs.
- The catalog.json equality contract has one owner again: promote reuses
  catalog_drift_guard.normalize_web_catalog_payload instead of a duplicate
  normalizer.
- catalog_csv gains text-based readers; publish_workflow and the concierge's
  detect_existing_asset stop hand-rolling csv.DictReader over catalog rows.
- The concierge admission evidence contract adopts the canonical field names
  (shared_rationale, deprecation_policy, estimated_published_size_gb) that
  catalog_docs now enforces, with a temporary legacy-key bridge for
  pre-rename persisted workflow states.
- Dead profiling helpers (duplicate_counts, read_csv_rows, read_ogr_rows)
  deleted; templates document source_license_citation_status; the DRC ASM
  mask doc records its real ~0.03 GB footprint instead of 1 GB.

Validation: full suite 624 passed / 4 skipped, ruff clean, catalog_docs.py
check current for 24 docs, stdlib-only import verified on system Python.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jonaraphael and others added 2 commits July 3, 2026 08:56
The catalog viewer silently reset the colorize dropdown to None when a
metadata color field could not load. The refusal reason (for example a
sidecar larger than the autoload budget) is now rendered as a note in
the color-legend area. The default browser autoload budget rises from
5 MiB to 24 MiB, integer measures such as fatalities keep numeric
gradients instead of flipping to categorical mode, and the detail panel
shows the asset slug. Preview catalog builds inject a 32 MiB budget via
a new catalog_site.py --metadata-sidecar-autoload-max-bytes flag wired
into both preview workflows only; the production catalog web deploy is
unchanged and pinned by test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The validate-artifacts required-evidence template now documents the
enforced tool_versions field, evidence-array validation errors use
0-based indexes matching JSON positions and name the offending object
URI, and the stage-scratch and preview-upload steps state the per-role
content-type and cache-control requirements up front instead of only
enforcing them at confirm time. Regression tests pin the template and
message contracts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jonaraphael jonaraphael deployed to shared-datasets-production July 3, 2026 12:57 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant