ci(e2e): run the end-to-end suite on every code PR (#1420 §1)

.github/workflows/e2e.yml

@@ -1,16 +1,41 @@
 name: E2E
 
 # End-to-end suite (#334). Triggers:
-#  - PR opt-in via the `e2e` label (label / synchronize / reopened).
+#  - every PR that touches code (#1420 §1).
 #  - push to main (post-merge regression catch).
 # Failure surfacing (#370):
 #  - push:main fail → open issue tagged `attn:e2e-failure` referencing the SHA.
 #  - PR fail → add `attn:e2e-failure` label to the PR.
+#
+# #1420 §1: this used to run only on `push` or when someone added an `e2e`
+# label, so a regression in the *installed-package* path — which the unit
+# suite cannot see, because it imports from the source tree — was caught on
+# `main` after merging rather than on the PR that introduced it. The label
+# opt-in is gone; the `paths` filter below is what keeps docs-only PRs off it.
+#
+# `paths` is safe here specifically because `e2e` is NOT one of the branch's
+# required contexts (those are `history-scan`, `pattern-scan`,
+# `pytest (3.12)`, `pytest (3.13)`, `secrets-scan`). A path-filtered *required*
+# check never reports on a PR it skips and leaves it permanently pending —
+# which is why `ci.yml` deliberately has no `paths` and filters inside the job
+# instead. Do not copy this pattern into a required workflow.
+#
+# The path list mirrors `ci.yml`'s `code` filter. Keep them in sync: a path
+# that can change installed behaviour but is missing here is an e2e regression
+# nobody sees until it is on main.
 
 on:
   pull_request:
-    types: [labeled, synchronize, reopened]
+    types: [opened, synchronize, reopened]
     branches: [main]
+    paths:
+      - 'src/**'
+      - 'tests/**'
+      - 'benchmarks/**'
+      - 'scripts/**'
+      - 'pyproject.toml'
+      - 'uv.lock'
+      - '.github/workflows/e2e.yml'
   push:
     branches: [main]
 
@@ -23,7 +48,6 @@ concurrency:
 
 jobs:
   e2e:
-    if: github.event_name == 'push' || contains(github.event.pull_request.labels.*.name, 'e2e')
     runs-on: ubuntu-latest
     timeout-minutes: 8
     strategy:

CHANGELOG/v4.md

@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- **End-to-end coverage now runs on every code PR, not after merge ([#1420](https://github.com/robotrocketscience/aelfrice/issues/1420) §1).** `e2e.yml` was gated `if: github.event_name == 'push' || contains(...labels..., 'e2e')`, so the end-to-end suite ran on `main` **after** a change merged, or on a PR only if someone remembered to add an `e2e` label. The unit suite structurally cannot cover what e2e covers — it imports from the source tree, while e2e exercises the three installed paths (`uv-tool`, `pipx`, `venv-pip`) — so an installed-package regression had no pre-merge gate at all. The label opt-in is removed and the trigger is path-filtered to the same globs as `ci.yml`'s `code` filter, with `opened` added to the event types (with only `[labeled, synchronize, reopened]`, a freshly-opened PR never fired it). Trigger-level `paths` is safe here **because `e2e` is not a required context** — a path-filtered required check never reports on a PR it skips and leaves it permanently pending, which is exactly why `ci.yml` has no `paths` and filters inside the job instead; three new tests in `tests/test_ci_path_filter.py` pin that distinction, the absence of the label gate, and the agreement between the two path lists, so the "keep them in sync" comment is enforced rather than hoped for. Mutation-checked: restoring the label gate and dropping `benchmarks/**` from the e2e paths each turn exactly one of the new tests red.
+
 ### Added
 
 - **The spine divergence meter reports both directions, and its denominator no longer charges the key for a writer defect ([#1356](https://github.com/robotrocketscience/aelfrice/issues/1356), [#1283](https://github.com/robotrocketscience/aelfrice/issues/1283) AC4).** `aelf spine verify` shipped in #1336 without either half of the ratified constraint (2). It iterated `shipped - recomputed` only, so links the recompute produces that the shipped spine lacks were never named — **2,102** of them on the development store; a meter that sees one direction cannot distinguish "the recompute missed links" from "the recompute invented links", and those have opposite fixes. And the **546** fan-in > 1 successors sat inside `reproduced_share`, which is precisely the single depressed percentage the constraint forbade: a chain gives each successor exactly one predecessor, so on a fan-in-2 successor the recompute can only ever reproduce one of the two shipped edges. Both of a fan-in successor's edges now leave the denominator, not just the missed one — dropping only the miss moves numerator and denominator by different amounts and *inflates* the share rather than correcting it. **The published figure moves, and it is a denominator correction, not a movement in fidelity: on this store `reproduced_share` is `93.69%` (39,335 / 41,984) under the old denominator and `94.86%` (38,789 / 40,892 fan-in-1 eligible) under the corrected one. Do not compare the two.** Both come from one `spine_divergence()` call on one store, which is what makes them a pair; the `93.68%` published in #1336 is a *different* measurement (39,280 / 41,929, a snapshot 55 edges smaller) and is not this store's before-figure. The direction is data-dependent rather than fixed — the share rises when the excluded edges were reproducing worse than the overall rate and falls when better; on this store they reproduced at 50% against an overall 93.7%, so it rises, while on the module's own CLI fixture the same correction moves 40% → 33.33%. Per the project rule that published numbers ship their script, `benchmarks/spine_fan_in_baseline.py` re-derives every figure and writes `benchmarks/spine_fan_in_baseline.json`. Third gap: "non-increasing" existed only as prose in a docstring and in printed output, with no baseline committed anywhere, so nothing could assert it — `fan_in_regressed_against` now compares to the committed baseline, and treats *equal* as passing, since the surplus is a standing writer defect this issue measures rather than fixes.

tests/test_ci_path_filter.py

@@ -131,3 +131,97 @@ def test_docs_only_changes_still_skip_the_suite() -> None:
             f"the full matrix — reversing #413/#427. Intentional? Confirm a "
             f"test actually depends on those files first."
         )
+
+
+# --- e2e trigger coverage (#1420 §1) -------------------------------------
+
+_E2E_WORKFLOW = _REPO / ".github" / "workflows" / "e2e.yml"
+
+
+def _e2e_pull_request_paths() -> list[str]:
+    """The glob list under `on.pull_request.paths` in e2e.yml.
+
+    Parsed the same way `_code_filter_globs` parses ci.yml, and asserted
+    non-empty for the same reason: a parser that quietly finds nothing turns
+    every assertion below green for free.
+    """
+    lines = _E2E_WORKFLOW.read_text(encoding="utf-8").splitlines()
+    starts = [i for i, line in enumerate(lines) if line.strip() == "paths:"]
+    assert len(starts) == 1, (
+        f"expected exactly one `paths:` key in {_E2E_WORKFLOW.name}, found "
+        f"{len(starts)}"
+    )
+    start = starts[0]
+    indent = len(lines[start]) - len(lines[start].lstrip())
+    globs: list[str] = []
+    for line in lines[start + 1 :]:
+        stripped = line.strip()
+        if not stripped or stripped.startswith("#"):
+            continue
+        if len(line) - len(line.lstrip()) <= indent:
+            break
+        matched = re.fullmatch(r"-\s*'([^']+)'", stripped)
+        assert matched, f"unparsed entry in the e2e paths list: {stripped!r}"
+        globs.append(matched.group(1))
+    assert globs, "the e2e paths list parsed empty"
+    return globs
+
+
+def test_e2e_runs_on_code_prs_without_an_opt_in_label() -> None:
+    """#1420 §1: e2e must not require a label to run on a PR.
+
+    Before this, the job carried
+    `if: github.event_name == 'push' || contains(...labels..., 'e2e')`, so an
+    installed-package regression — which the unit suite structurally cannot
+    see, because it imports from the source tree — was caught on `main` after
+    merge instead of on the PR. Asserting the absence of the label condition
+    is the whole point; a test that only checked the workflow parses would
+    have passed in the broken state.
+    """
+    text = _E2E_WORKFLOW.read_text(encoding="utf-8")
+    assert "labels.*.name, 'e2e'" not in text, (
+        "e2e is gated on an opt-in label again — a PR without the label gets "
+        "no end-to-end coverage and the regression lands on main"
+    )
+    assert "types: [opened, synchronize, reopened]" in text, (
+        "e2e's pull_request trigger must include `opened`; with only "
+        "[labeled, synchronize, reopened] a freshly-opened PR never fires it"
+    )
+
+
+def test_e2e_paths_cover_the_ci_code_filter() -> None:
+    """The two path lists must agree, or e2e silently under-triggers.
+
+    `e2e.yml` says to keep its `paths` in sync with ci.yml's `code` filter.
+    A comment cannot enforce that: a path added to ci.yml and forgotten here
+    means a PR that changes installed behaviour runs the unit suite and skips
+    e2e, with nothing red to show for it.
+
+    Each list may legitimately reference its *own* workflow file, so those are
+    excluded from the comparison rather than the assertion being loosened.
+    """
+    ci_globs = {g for g in _code_filter_globs() if not g.startswith(".github/")}
+    e2e_globs = {g for g in _e2e_pull_request_paths() if not g.startswith(".github/")}
+    missing = sorted(ci_globs - e2e_globs)
+    assert not missing, (
+        f"{missing} are in ci.yml's `code` filter but not in e2e.yml's "
+        f"`paths`. A PR touching only those runs pytest but not e2e."
+    )
+
+
+def test_e2e_is_path_filtered_at_the_trigger_not_inside_the_job() -> None:
+    """Pin *why* the trigger-level filter is safe here.
+
+    A path-filtered **required** context never reports on a PR it skips and
+    leaves that PR permanently pending — which is exactly why ci.yml has no
+    `paths` and filters inside the job instead. e2e is not a required context,
+    so filtering at the trigger is fine and cheaper. If e2e is ever promoted
+    to required, this must move inside the job first.
+    """
+    ci_text = _CI_WORKFLOW.read_text(encoding="utf-8")
+    assert "dorny/paths-filter" in ci_text, (
+        "ci.yml stopped filtering inside the job; if it gained a trigger-level "
+        "`paths` it would leave docs-only PRs permanently pending on the "
+        "required pytest contexts"
+    )
+    assert "dorny/paths-filter" not in _E2E_WORKFLOW.read_text(encoding="utf-8")