Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
name: CI

# Run the test suite on every pull request and on pushes to main. Push is
# limited to main so PR branches don't run twice (once for push, once for the PR).
# Run the test suite only when changes land on main (a merged PR or a direct
# push). Tests intentionally do NOT run on pull requests — see ADR-027 for the
# trigger policy and its post-merge-feedback tradeoff. workflow_dispatch allows
# running the full battery grid manually (e.g. to check a branch before merge).
on:
push:
branches: [main]
pull_request:
workflow_dispatch:

# Cancel superseded runs on the same ref (e.g. a new push to an open PR branch).
# Serialize runs on the same ref. Don't cancel an in-progress run: every merge
# to main should be fully tested, since these runs are what gate releases.
concurrency:
group: ci-${{ github.ref }}
cancel-in-progress: true
cancel-in-progress: false

jobs:
tests:
Expand Down
37 changes: 34 additions & 3 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
name: Tests

# Reusable workflow — the single definition of how RAC's test suite runs.
# Called by ci.yml (push / pull_request) and by python-publish.yml (release gate),
# Called by ci.yml (push to main) and by python-publish.yml (release gate),
# so the test steps live in one place rather than being duplicated.
#
# Tests are split into one "battery" per .py service (plus grouped core / cli /
# artifacts) and run across every supported Python version, so the Actions UI
# names the service + version that failed (e.g. "relationships (py3.11)")
# instead of a single opaque "pytest (3.11)". See ADR-027.

on:
workflow_call:
Expand All @@ -12,11 +17,37 @@ permissions:

jobs:
pytest:
name: ${{ matrix.battery.name }} (py${{ matrix.python-version }})
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ["3.11", "3.12", "3.13"]
# One battery per .py service, plus grouped core / cli / artifacts.
# Every tests/test_*.py belongs to exactly one battery (no orphans).
battery:
- name: core
paths: "tests/test_validate.py tests/test_parser.py tests/test_schema.py tests/test_identity.py"
- name: cli
paths: "tests/test_cli.py"
- name: artifacts
paths: "tests/test_design.py tests/test_roadmap.py tests/test_prompt.py tests/test_decision_metadata.py"
- name: diff
paths: "tests/test_diff.py"
- name: improve
paths: "tests/test_improve.py"
- name: index
paths: "tests/test_index.py"
- name: ingest
paths: "tests/test_ingest.py"
- name: inspect
paths: "tests/test_inspect.py"
- name: portfolio
paths: "tests/test_portfolio.py"
- name: relationships
paths: "tests/test_relationships.py tests/test_relationships_cmd.py tests/test_relationship_validation.py"
- name: stats
paths: "tests/test_stats.py"
steps:
- uses: actions/checkout@v4
with:
Expand All @@ -35,5 +66,5 @@ jobs:
# tests/test_ingest.py uses to generate fixtures.
python -m pip install -e .[dev]

- name: Run tests
run: python -m pytest -q
- name: Run ${{ matrix.battery.name }} battery
run: python -m pytest -q ${{ matrix.battery.paths }}
2 changes: 1 addition & 1 deletion rac/decisions/adr-019-asset-management.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# ADR-017 Asset References
# ADR-019: Asset References

## Status

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# ADR-016: RAC Is Not a Content Store
# ADR-024: RAC Is Not a Content Store

## Status

Expand Down
2 changes: 1 addition & 1 deletion rac/decisions/adr-025-hybrid-artifact-metadata.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# ADR-XXX Hybrid Artifact Metadata
# ADR-025: Hybrid Artifact Metadata

## Status

Expand Down
2 changes: 1 addition & 1 deletion rac/decisions/adr-026-opaque-artifact-identities.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# ADR-XXX Opaque System-Assigned Artifact Identity
# ADR-026: Opaque System-Assigned Artifact Identity

## Status

Expand Down
217 changes: 217 additions & 0 deletions rac/decisions/adr-027-ci-test-topology.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
# ADR-027: CI Test Topology — Merge-Gated, Per-Service Batteries

## Status

Accepted

## Category

Process

## Context

RAC runs its test suite in GitHub Actions through a single reusable workflow,
`.github/workflows/tests.yml`, which is consumed by two callers:

- `.github/workflows/ci.yml` — continuous integration.
- `.github/workflows/python-publish.yml` — the release/publish pipeline.

Two aspects of this setup were never written down, and had begun to drift with each
CI edit:

1. **When tests run.** `ci.yml` triggered on every `pull_request` *and* on pushes to
`main`. Every push to an open PR branch therefore re-ran the entire suite, and the
policy for *when* CI should run was never recorded — so it was liable to be widened
or narrowed incidentally.
2. **How the suite is shaped.** `tests.yml` ran the whole suite as a single `pytest`
job parameterized only by Python version (`["3.11", "3.12", "3.13"]`). The Actions
UI showed only `pytest (3.11)`, `pytest (3.12)`, `pytest (3.13)`. A failure named the
Python version but not the **service** at fault, and the run shape did not reflect
RAC's service-oriented architecture (ADR-008), where each capability is an isolated
`.py` service under `src/rac/services/` plus the `core`, `cli`, and artifact layers.

The release path already gated publishing on the reusable test workflow
(`release-build: needs: test`), but that gate was likewise an unrecorded convention
rather than a stated policy.

Absent a recorded decision, each of these choices invites silent regression — a
re-added PR trigger, a collapsed matrix, a release that builds before tests — on the
next person's CI edit. This ADR fixes the CI test topology so future changes are
deliberate.

## Decision

RAC's CI test topology is governed by three rules.

### 1. Tests run on merge to `main`, not on pull requests

`ci.yml` triggers on `push:` to `main` (a merged PR or a direct push) and on
`workflow_dispatch` (manual). It does **not** trigger on `pull_request`.

The explicit, accepted consequence: **a pull request receives no automated test
feedback before it merges.** A regression is caught by the post-merge run on `main`
and then blocks the next release through the gate (rule 2), rather than being caught
on the PR itself. `workflow_dispatch` is the escape hatch — the full battery grid can
be run against any branch on demand from the Actions tab.

Runs on `main` use `concurrency` with `cancel-in-progress: false`, so every merge is
fully tested and a later merge does not cancel an in-flight run.

### 2. Releases are gated on the full suite

`python-publish.yml` runs the reusable test workflow as a `test` job and makes
`release-build` depend on it (`needs: test`). Nothing is built or published unless
**every** battery passes. This protects the public contracts a release ships — the CLI
command surface (ADR-005) and the JSON output (ADR-007).

### 3. The suite is organized as per-service batteries

`tests.yml` defines a two-axis matrix — supported Python version × **battery** — where
each battery is one `.py` service (`diff`, `improve`, `index`, `ingest`, `inspect`,
`portfolio`, `relationships`, `stats`), plus the grouped `core`, `cli`, and `artifacts`
layers. Each battery runs on every supported Python version (currently 3.11–3.13, per
`pyproject` `requires-python`). Job names surface service and version, e.g.
`relationships (py3.11)`.

The battery list is an **explicit, static enumeration** in the workflow — not
discovered dynamically — and every `tests/test_*.py` file maps to exactly one battery.

## Principles

### Principle 1 — Test topology mirrors the service architecture

ADR-008 makes each capability a reusable service. CI makes each service a battery, so a
red check names the responsible module instead of an opaque interpreter version.

### Principle 2 — One reusable workflow is the single source of truth

`tests.yml` is the only definition of *how* tests run. Both CI and the release gate
consume it, so day-to-day testing and release testing can never diverge.

### Principle 3 — The matrix is explicit and deterministic

The battery list is readable in the YAML and changes only by deliberate edit. This is
consistent with RAC's preference for deterministic, inspectable structure over
generated or dynamically discovered behavior.

### Principle 4 — Trigger policy is recorded, not incidental

*When* tests run is a decision with real tradeoffs. It lives here so a future CI edit
changes it on purpose, with this context in view, rather than by accident.

## Consequences

### Positive

- A failed check names the service and Python version, so triage starts at the right
module without opening logs.
- The run shape matches the architecture; adding a service is a one-line battery
addition.
- Releases cannot ship on a red suite, and the gate shares one definition with CI.
- PR pushes no longer trigger repeated full-suite runs, reducing Actions usage on
in-flight branches.

### Negative

- Pull requests get no pre-merge test signal by default; regressions surface
post-merge on `main`. Mitigated by `workflow_dispatch` and the release gate.
- More jobs per run (batteries × versions ≈ 33). They are short and run in parallel,
but the checks list is longer.
- The battery list must be kept in sync: a new `tests/test_*.py` that is not added to a
battery will not run in CI. Guarded by a coverage check at change time; a CI
self-check could enforce it later.

## Alternatives Considered

### Run tests on every pull request

Keep the `pull_request` trigger so PRs are checked before they merge.

#### Pros

- Regressions are caught before landing — standard CI practice.

#### Cons

- The stated goal was to stop running the suite on every push to in-flight branches.
- Doubles runs (push + PR) and spends Actions minutes on branches that may be rebased
or abandoned.

Deferred, not rejected — to be reconsidered if outside contributors need pre-merge
gating (see Review Date).

### Single job parameterized only by Python version (the prior shape)

#### Pros

- Fewer jobs; simplest possible matrix.

#### Cons

- A failure shows only the Python version, not the service.
- The run shape ignores the service architecture.

Rejected — it is exactly the opacity this decision removes.

### Per-test-file batteries

One job per `tests/test_*.py`.

#### Pros

- Finest possible granularity.

#### Cons

- ~19 batteries × 3 versions, with near-duplicates (e.g. three relationship files) and
no service-level grouping; noisier than per-service for no extra signal.

Rejected.

### Dynamically discovered matrix

Generate the battery list from the filesystem at run time.

#### Pros

- No manual sync; new test files are included automatically.

#### Cons

- A generated matrix is opaque to read and non-deterministic; it contradicts RAC's
preference for explicit, inspectable structure.

Rejected — the sync cost is instead paid by an explicit list plus a coverage check.

## Relationship to Other ADRs

### ADR-008 Agent-Ready Architecture

Capabilities live in reusable services. The per-service battery topology is the CI
projection of that architecture — one service, one battery.

### ADR-005 CLI First and ADR-007 JSON Contract Stability

The CLI and JSON outputs are RAC's public contracts. The release gate (rule 2) exists
so no release ever ships with those contracts broken.

### ADR-003 Structured Outputs First

Typed, structured outputs are what make services testable in isolation, which is what
lets the suite split cleanly along service lines.

## Success Measures

Evidence that this decision is working:

- A red CI check identifies the failing service and Python version without opening logs.
- Adding a new service adds exactly one battery entry — a one-line diff.
- No release builds or publishes while any battery is failing.
- Every `tests/test_*.py` belongs to exactly one battery; no test is silently unrun.
- The trigger policy changes only via a deliberate edit to this decision.

## Review Date

Review before v1.0.0, or sooner if RAC accepts outside contributors who need pre-merge
test feedback — which would warrant re-adding a `pull_request` trigger (rule 1) — or if
the job count from the battery × version grid becomes burdensome.
Loading