Skip to content

docs(ci): document the GitHub Actions workflow graph - #898

Merged
JarryShaw merged 1 commit into
mainfrom
worktree-agent-a7f54313e7e5975f3
Sep 29, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
worktree-agent-a7f54313e7e5975f3

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • docs — documentation only

Description of your pull request and other information

  Closes #897. Adds docs/source/workflows.rst, documenting the eight
GitHub Actions workflows and, above all, the edges between them: a trigger
table, a mermaid graph of every push/pull_request/schedule trigger plus
the workflow_run and uses: edges, and sections walking through each. Also
documents the #888 skip cascade (create-release.yml's unit-tests job
skipping when the triggering Vendor Update run's conclusion wasn't
success, cascading through every publish job), and maps the six required
status-check contexts on ruleset 23497679 to the job/matrix leg emitting
each. Cross-references releasing.rst for the release pipeline itself
rather than duplicating it. No workflow, code, or test files touched.

  docs/source/index.rst's toctree gets one line, next to releasing.

@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) ci Pull requests that change CI or workflow configuration (ci: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 29, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES on 7a4774da0 — cross-review (opus; author sonnet). Three factual errors about the workflows, one mermaid node that renders as its own id, and one diagram that contradicts the prose beside it. I verified all five myself against the YAML before posting.

1. :208-209 — gate-only: true does not select the gate job alone. The changelog job (unit-tests.yml:707, name: Changelog drift) carries no if:, and its source comment at :695-701 says it is "Deliberately not gated on gate-only, unlike the four jobs above … so an ungated job runs on the release path". Two jobs run, not one — confirmed empirically on gate-only run 36210743295, which produced Changelog drift and Gate (full suite, Python 3.14) as successes with four matrix jobs skipped. This is the one thing on the page the release path actually depends on. Same sentence's "rather than its five-version matrix" is also wrong: four differently-shaped matrix jobs are skipped, not one.

2. :315-317 — it stands in for 17 of the 22, not 22. The comment the page correctly cites (unit-tests.yml:824) says "pypcap-parity slots (17 of the 22) -- the five live Compat names stay required". 5+5+5+5+2 = 22, minus the five Compat = 17. The page's own table thirty lines earlier lists those five as separately required, so the sentence contradicts the page.

3. :308-312 — the "only place" claim is false. python-compatibility.yml:69 is also a job name:, Compat Python 3.15 (scheduled). The table's mapping survives (different literal, not a required context), but the sentence does not — and the page cites that very job twenty lines later at :333-334 for its continue-on-error.

4. :246-255 — mermaid node PY is never labelled. It appears only in VC -->|needs| PY, GH -->|needs| PY and the class list, so it renders as a box reading PY beside siblings reading "github (GitHub Release)". releasing.rst:146 labels the same node properly.

5. :241-251 — the skip-cascade graph omits the version_check → conda edge while the prose at :236-237 correctly states conda needs [tag, github, version_check] (create-release.yml:406, verified). Draw all of version_check's direct dependents or none — releasing.rst:153-158 takes the "none" option deliberately and says why, so the two pages now disagree without explanation.

Everything else checked out, including every row of the trigger table, all four environment: citations, the four workflow_run and four uses: edges, the six required contexts against ruleset 23497679, the :doc: cross-reference resolving in built HTML, and both production run ids. #897's five requirements are all met. The page also caught an error in my own brief — there are four workflow_run edges, not the three I gave it.

Two things worth knowing beyond this PR. The build reports 58 warnings on a clean build, not the 42 the author measured — most likely a warm doctree cache suppressing re-emission; zero are attributed to the new page either way. And mermaid gets no build-time validation at all here: with no mermaid_output_format set and no mmdc installed, Sphinx copies the block verbatim into a <div class="mermaid"> without parsing it. That is precisely why an unlabelled node reached review, and it means mermaid correctness has to be read rather than built.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 29, 2026
@JarryShaw
JarryShaw force-pushed the worktree-agent-a7f54313e7e5975f3 branch from 7a4774d to c8d4869 Compare September 29, 2026 03:25
@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Sep 29, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES on c8d486992 — re-review (opus; author sonnet). Four of the five fixes are correct. The fifth introduced a fresh miscount in the sentence it was rewriting, and that miscount contradicts the page 146 lines later.

The new defect, at :208-210: "gate-only: true selects two of unit-tests.yml's six jobs, not one … It skips the other four". unit-tests.yml has seven jobs, verified by reading the job keys:

:43 test   :129 integration   :323 engine-tests   :601 pypcap-parity
:707 changelog   :734 gate   :944 required-checks

Under gate-only: true, two run and five skip — the four matrix jobs (if: ${{ inputs.gate-only != true }} at :51, :131, :325, :603) plus required-checks, whose if: ${{ always() && inputs.gate-only != true }} sits at :947.

And the page already knows this: at :354-358 it says of required-checks that "the gate-only: true reusable calls documented above skip it … so it is never produced -- and never expected -- on those paths." Two-plus-four-equals-six only holds if required-checks does not exist, and the page asserts both.

The likely origin makes it actionable: run 36210743295 shows six job records, because required-checks produced no record at all while the four matrix jobs did appear as skipped. Counting the run gives six; the sentence is a claim about the file, which has seven. Minimal fix: "two of unit-tests.yml's seven jobs … It skips the other five — the four matrix jobs … and required-checks (see below)."

The other four fixes verified correct, each against the YAML rather than the prose: the changelog job is at :707 with no if: and the :695-701 citation is faithful; the four skipped matrix shapes are right; run 36210743295 was independently re-checked and every claim the page makes about it is true; the 17-of-22 arithmetic now reconciles with the page's own table, which it explicitly cross-references; the Compat Python grep claim is now exactly right at three lines with :69 acknowledged and the correct parallel Required checks passed sentence undisturbed; every node in both mermaid blocks is now labelled, checked programmatically as declared-set == used-set rather than spot-checking PY; and the cascade diagram's edge set is byte-identical to releasing.rst:139-148's, with a paragraph that genuinely explains the omission.

Delta is clean — only workflows.rst moved (67+/30−), still one commit, and docs/source/releasing.rst is untouched, so #905's file is safe.

The 42-versus-58 warning dispute is settled, and 42 is the quotable number. Sphinx's own summary line reads build succeeded, 42 warnings. The discrepancy is entirely a counting-method artefact and it closes exactly:

Sphinx's own summary                                              42
lines matching WARNING|ERROR                                      45
  of those, pcapkit RUNTIME logger lines (missing optional deps)  −3
                                                                  42  ✔
naive `grep -c WARNING`                                           43   (misses 2 real ERROR: diagnostics)

So a grep -c WARNING over the log picks up pcapkit's own runtime logger and drops the two ERROR: diagnostics Sphinx counts in its tally. The author was right and both earlier reviewers were wrong — including me for relaying 58. The count of non-Sphinx runtime lines is environment-dependent, which is why a reviewer with fewer optional dependencies installed greps a higher number. Use Sphinx's summary line, never a grep.

grep -c 'workflows\.rst\|index\.rst' over the log is 0, so the page still introduces nothing.

One non-blocking imprecision, inherited verbatim from releasing.rst and not worth churn: GH→CD is itself implied by GH→TAGJ→CD, so neither page's diagram is a strict transitive reduction despite both calling it one. Matching the sibling is the right call; flagging only so it is not mistaken for a new error.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 29, 2026
- Add docs/source/workflows.rst covering the eight GHA workflows: trigger
  table (push/pull_request/schedule/workflow_dispatch), a mermaid graph of
  every trigger plus the workflow_run and uses: edges, and per-section detail
  on each.
- Enumerate all four workflow_run edges (cron-vendor.yml, cron-conda.yml,
  deploy-pages.yml, create-release.yml -- one more than the three named in
  the issue) and all four uses: calls, each with file:line citations. All four
  uses: calls invoke unit-tests.yml with gate-only: true, which runs its
  `gate` and `changelog` jobs (2 of unit-tests.yml's 7) and skips the other 5
  (the four matrix jobs plus `required-checks`).
- Document the #888 skip cascade: create-release.yml's unit-tests job skips
  when the triggering Vendor Update run's conclusion wasn't 'success', which
  cascades through version_check to every publish job via needs:, reporting
  the whole run skipped rather than failed. Verified against the YAML and
  against production runs 36511610205 and 36510774776.
- Map the six required-status-check contexts from ruleset 23497679 to the
  job/matrix leg that emits each, via grep across every workflow file.
  `Required checks passed` stands in for 17 of the ruleset's originally-named
  22 contexts; the five live `Compat Python 3.10`-`3.14` contexts stay
  required separately.
- List the four environment:-gated jobs (all in create-release.yml) that can
  pause for approval, cross-referencing releasing.rst for the approval story
  rather than duplicating it.
- Wire the page into index.rst's toctree next to releasing.

Built with Sphinx 9.1.0 from this worktree (sphinxcontrib-mermaid was already
a docs dependency, matching python-version 3.14 and pip install -e .[all] as
in deploy-pages.yml); a fresh BUILDDIR reports Sphinx's own "build succeeded,
42 warnings", none attributable to the new page.
@JarryShaw
JarryShaw force-pushed the worktree-agent-a7f54313e7e5975f3 branch from c8d4869 to 6d3fa3a Compare September 29, 2026 04:02
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO on 6d3fa3a66. The miscount is fixed, and the wording is better than the correction I suggested.

"selects **two** of ``unit-tests.yml``'s **seven** jobs, not one … It skips the other
 **five** -- the four matrix jobs … -- and ``required-checks`` …, gated out by its
 own ``if:`` rather than by having already run."

That last clause is the part I did not ask for and it is the most useful sentence in the passage: it names why the two kinds of skip differ. The four matrix jobs skip because they already ran for this commit; required-checks skips because its own if: excludes it. That distinction is exactly what made the original count wrong — a run listing shows six job records because required-checks produces none at all, while the file has seven.

Verified myself: the delta since c8d486992 is docs/source/workflows.rst alone, 10 insertions and 8 deletions, still one commit above main. And I swept the page for every other job-count claim rather than trusting the one that was fixed:

:208  two of seven               <- corrected
:211  the other five             <- corrected
:224  the four matrix jobs       correct
:291  their six jobs             correct -- Create Release's own six on main, and a claim
                                 about the two skipped RUN records, both of which showed six
:337  two job name: lines        correct -- python-compatibility.yml :31 and :69

The other four fixes from the previous round stand, each already confirmed against the YAML: the changelog job at :707 with no if: and a faithful paraphrase of :695-701; run 36210743295 independently re-checked; the 17-of-22 arithmetic reconciling with the page's own table; the Compat Python grep claim exactly right with :69 acknowledged; every node in both mermaid diagrams labelled, checked as declared-set == used-set; and the cascade diagram byte-identical to releasing.rst:139-148's edge set.

And the author used Sphinx's own summary line this round rather than a grep — build succeeded, 42 warnings, zero attributable to workflows.rst or index.rst. That was the right call and it is now the settled method here: a grep -c WARNING picks up pcapkit's runtime logger and drops real ERROR: diagnostics Sphinx counts, which is why three measurements disagreed earlier today.

mergeStateStatus: BEHIND is expected — main keeps moving during the release window — and mergeable: MERGEABLE, no conflict. Ready for you to merge. I am not merging.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Sep 29, 2026
@JarryShaw
JarryShaw merged commit f35e02e into main Sep 29, 2026
63 checks passed
@JarryShaw
JarryShaw deleted the worktree-agent-a7f54313e7e5975f3 branch September 29, 2026 04:48
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Sep 29, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
@JarryShaw JarryShaw moved this to Done in PyPCAPKit Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix) docs Pull requests that change documentation only (docs: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

docs(ci): document the GitHub Actions workflow graph, including the workflow_run edges

1 participant