✅ test(spec): put `docs/spec.md` examples under CI by nstarman · Pull Request #733 · GalacticDynamics/coordinax

nstarman · 2026-08-18T20:41:23Z

--ignore-glob=docs/spec.md carried the comment # exclude the spec file and no reason. It was hiding 17 stale examples out of 274.

This is not hypothetical rot. #716 hit one of them directly and had to note that docs/spec.md still printed the sphere diagonal as a bare Array — "which nothing caught because pytest carries --ignore-glob=docs/spec.md". Removing the glob is the fix for the next one.

before:  17 failed, 257 passed   (not run in CI)
after:      274 passed           (run in CI)

The drift the exclusion let accumulate

what	was	is
chart reprs	`Spherical3D()`	`Spherical3D(M=Rn(3))`
`default_chart`	property	method — 4 examples printed `<bound method ...>`
metric-level `norm`	`norm(v, chart)`	needs `at=`, since #715 made metrics point-evaluated
sphere metric diagonal	`Array([1., 1.])`	`QM([1., 1.], '(, )')` — the #716 one
`weak_type`	inconsistent	moved in both directions

One ```python block was illustrative pseudo-code — manifold, point, chart are undefined — and is now an unlabelled fence so sybil leaves it alone.

Three were not cosmetic

Called out rather than papered over, because two of them are behaviour the spec asserted and the code does not have.

1. no_manifold.ndim is 0, not -1. Recorded as 0, with the caveat that it therefore does not distinguish the sentinel from a genuine zero-dimensional manifold — Rn(0).ndim is 0 too — so callers must test isinstance(M, NoManifold). The source doctest pins 0 and is already under CI, so this follows it rather than contradicting it. If -1 was the intent, this is a bug, not a doc edit.

2. guess_manifold({"theta": ..., "phi": ...}) returns NoManifold(), where the spec claimed HyperSphericalManifold(ndim=2). Component names alone do not infer a sphere — though x/y/z do infer Rn(3), and the chart overload resolves correctly. Recorded, with a line saying to hand over the chart. Same caveat: if the spec was right, this is a gap in guess_manifold.

3. TwoSphereIn3D defaults its ambient to Spherical3D, so pt_embed returns (r, theta, phi), not the (x, y, z) the example showed. Documented the default and how to ask for Cartesian.

That third one is worth a note on method: passing ambient=cxc.cart3d to preserve the example's original intent was tried first, and rejected — it routes through cos(pi/2) and puts 1.2246468e-16 into the expected output. Enshrining platform-dependent float noise in a file that is about to start gating CI is precisely the brittleness this PR exists to stop.

Verification

docs/spec.md: 274 passed, and confirmed collected by a bare pytest run (277 items) rather than only when named explicitly
Full suite: 10704 passed, 10 skipped, 1 xfailed — up from 10430 on main, the difference being exactly these examples
prek run --all-files clean; uv run --frozen nox -s docs (warnings as errors) succeeds
NORMALIZE_WHITESPACE is set in conftest.py, so the two re-wrapped dict outputs match legitimately rather than by luck

Note on stacking

Touches docs/spec.md, as does #732. They edit different regions and are independent; whichever lands second may need a trivial rebase. If this one lands first, #732's examples become CI-verified for free.

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR makes the repository’s authoritative spec (docs/spec.md) examples part of the regular pytest/Sybil run by removing the --ignore-glob=docs/spec.md exclusion, and updates the spec’s examples/text to match current behavior so the file can reliably gate CI.

Changes:

Remove the pytest --ignore-glob=docs/spec.md exclusion so docs/spec.md is collected and executed under CI.
Refresh numerous docs/spec.md examples to match current reprs/API (e.g., chart reprs including M=..., default_chart() as a method, metric.norm(..., at=...)).
Clarify/document behavior where the spec previously drifted (e.g., guess_manifold fallback for angle-name dicts, no_manifold.ndim, TwoSphereIn3D ambient default).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pyproject.toml	Stops excluding `docs/spec.md` from pytest collection, bringing spec examples under CI.
docs/spec.md	Updates and clarifies spec examples/output to align with current library behavior and CI execution.

Suppressed comments (2)

docs/spec.md:3109

This block was changed to an unlabelled fence to avoid Sybil execution, but elsewhere in this spec the established pattern is to keep ```python for syntax highlighting and precede it with  (e.g. around line ~1105). Using the skip marker here would keep consistent formatting while still preventing CI execution.

    ```
    import coordinaxs.api.manifolds as cxmapi

    g = cxmapi.metric_matrix(manifold, point, chart)  # → DiagonalMetric or DenseMetric
    ```

docs/spec.md:3526

The narrative sentence here still refers to default_chart as an attribute (.default_chart == ...), but the examples in this PR show it is now a method. Updating this prose avoids reintroducing confusion even though the doctest below is correct.

    3
    >>> M.default_chart()
    Cart3D(M=Rn(3))

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

codecov · 2026-08-18T21:19:03Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.59%. Comparing base (c79e7c1) to head (f787d83).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #733      +/-   ##
==========================================
+ Coverage   96.56%   96.59%   +0.03%     
==========================================
  Files         266      266              
  Lines        8852     8934      +82     
==========================================
+ Hits         8548     8630      +82     
  Misses        304      304

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The `coordinax.manifolds.lorentzian` sub-namespace had no entry in the spec at all -- not the namespace, and none of its four verbs. `causal_character` and `rapidity_between` were not so much as mentioned; `proper_time` and `proper_distance` appeared once each, inside `geodesic_distance`'s "use these instead" line. Five sections: the namespace, then a verb each. The namespace section carries what they share, so the verb entries stay short -- the gate is `AbstractLorentzianMetricField`, meaning *exactly one* timelike direction, and the reason each refuses rather than returning something readable as a classification. Also why the namespace is named for the signature: `galileanct` is a 4-dimensional spacetime and is not Lorentzian, so a `spacetime` namespace would promise membership these verbs refuse, while a `minkowski` one would exclude Schwarzschild and FLRW, which carry the marker and acquire all four. Worth pinning, and now pinned: - `causal_character` returns a Python `str`, not an enum and not a number, so it cannot be accidentally arithmetic. It takes `atol`, because a null separation is measure-zero in floating point and needs a band rather than an equality test. - `proper_time` divides by `c` and `proper_distance` does not, so one is a duration and the other a length out of the same length-squared interval. - Each refuses the other's causal class with `ValueError` rather than `nan`: no clock travels between spacelike-separated events, so the question does not apply rather than being undefined. - `rapidity_between` is the odd one -- tangents rather than points, `at=` required because there is no first point to take the base from, and a bare dimensionless `Array` rather than an `Angle`, since rapidity is unbounded and `rad` would wrongly suggest it wraps. It exists because `angle_between` refuses timelike pairs correctly: both `g(u,u)` and `g(v,v)` are negative, so `arccos` would clip and report two observers in relative motion as parallel. All 34 examples were executed under the harness that will run them, not a bare interpreter. That distinction is load-bearing here: `JAX_ENABLE_X64 = "1"` is set in `[tool.pytest.ini_options]`, so an ad-hoc `python` session yields `float32` and every literal would have been wrong -- `Q(1.6341238e-08, 's')` against the `Q(1.63412366e-08, 's')` the suite actually produces. Explanatory prose sits outside the `pycon` fences rather than inside them, per the review on GalacticDynamics#733: everything within a fence renders as code, taking its inline markup with it. The 17 pre-existing failures elsewhere in the file are GalacticDynamics#733's subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Four verbs that were public and unspecified: `metric_representation`, `pt_embed`, `pt_project` and `embedded_twosphere`. Placed with the machinery they belong to rather than in the verb cluster -- `metric_representation` after `metric_matrix`, whose sibling it is, and the embedding trio after `EmbeddedManifold`. `metric_representation` is the one most worth writing down, because its signature invites the wrong reading. It returns a **class**, `DiagonalMetric` or `DenseMetric`, not a populated matrix, and takes no base point: sparsity structure belongs to the (manifold, chart) pair, not to where you stand on it. That is what lets `metric_matrix` keep the O(n) diagonal path instead of contracting a full einsum, and why an `EmbeddedManifold` declares dense -- a pullback through an arbitrary embedding has no reason to be diagonal even when the intrinsic chart is orthogonal. `pt_project` is a projection and not merely an inverse: it is defined on all of the ambient and round-trips exactly only for points already on the surface, which is why it carries overloads `pt_embed` does not. One inconsistency found and recorded rather than smoothed over: the container of a projected angle tracks the ambient chart. Round-tripping through `Spherical3D` gives `Angle(1.04719755, 'rad')`; through `Cart3D`, the same value and unit in a plain `Q(1.04719755, 'rad')`. Written down as observed behaviour with a note that an angle is an element of S^1 and the container is what carries that, so the ambient chart arguably should not decide it. Two things this cost, both worth naming: - The `pt_embed` example first used the Cartesian ambient at the equator, which routes through `cos(pi/2)` and puts `1.2246468e-16` in the output. Moved to the north pole, where the same route is exact. - A `!!! warning` with a nested ```pycon block made sybil raise `ValueError: <Region ...> overlaps <Region ...>` at collection -- the outer indented block and the inner fence both parse as doctests. Flattened to prose plus a plain example. Explanatory prose sits outside the `pycon` fences, per the review on GalacticDynamics#733. All 24 examples verified under `pytest docs/spec.md`, which is the harness that sets `JAX_ENABLE_X64`; 0 failures inside the four added sections. The 17 pre-existing failures elsewhere in the file are GalacticDynamics#733's subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`--ignore-glob=docs/spec.md` carried the comment "exclude the spec file" and no reason. It was hiding 17 stale examples out of 274. GalacticDynamics#716 hit one of them directly -- it had to note that spec.md still printed the sphere diagonal as a bare `Array`, "which nothing caught because pytest carries `--ignore-glob=docs/spec.md`". Removing the glob is the fix for the next one. Most of the 17 are drift the exclusion let accumulate: charts now repr with their manifold (`Spherical3D()` -> `Spherical3D(M=Rn(3))`), `default_chart` is a method rather than a property so four examples printed `<bound method ...>`, `weak_type` moved in two directions, and the metric-level `norm` grew its required `at=` when GalacticDynamics#715 made metrics point-evaluated. One ```python block was illustrative pseudo-code -- `manifold`, `point`, `chart` are undefined -- and is now an unlabelled fence so sybil leaves it alone. Three were not cosmetic, and are called out rather than papered over: 1. `no_manifold.ndim` is `0`, not the `-1` the spec asserted. Recorded as `0` with the caveat that it therefore does *not* distinguish the sentinel from a genuine zero-dimensional manifold -- `Rn(0).ndim` is `0` too -- so callers must test `isinstance(M, NoManifold)`. Whether the sentinel *should* be distinguishable is a separate question; the source doctest pins `0` and is already under CI, so this commit follows it rather than contradicting it. 2. `guess_manifold({"theta": ..., "phi": ...})` returns `NoManifold()`, where the spec claimed `HyperSphericalManifold(ndim=2)`. Component names alone do not infer a sphere, though `x/y/z` do infer `Rn(3)` and the chart overload resolves correctly. Recorded, with a line saying to hand over the chart. 3. `TwoSphereIn3D` defaults its ambient to `Spherical3D`, so `pt_embed` returns `(r, theta, phi)`, not the `(x, y, z)` the example showed. Documented the default and how to ask for Cartesian. Passing `ambient=cxc.cart3d` was tried first and rejected: it routes through `cos(pi/2)` and puts `1.2246468e-16` into the expected output, which is precisely the brittleness this change exists to stop. (1) and (2) are behaviour the spec asserted and the code does not have. This commit makes the spec describe what is true so the file can run; if the original claims were the intent, they are two bugs rather than two doc edits. before: 17 failed, 257 passed (not run in CI) after: 274 passed (run in CI) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Review caught explanatory sentences sitting inside ```pycon blocks. Everything in a fence renders as code, so those lines came out monospaced and their inline markup -- ``x/y/z``, *names* -- rendered literally. Fixed by closing the fence around the prose rather than by prefixing it with `>>> #`, which would have kept it inside the block and dropped the markup entirely instead of just failing to render it. Six occurrences, not the one flagged: one is this branch's, four came in with GalacticDynamics#732, and `Points move by ``delta * tau``` predates both. Same defect and the same file, and this is the branch that puts the file under CI, so fixing the class rather than the instance belongs here. Sibling fences are fine for sybil -- the namespace is per document, so state carries across the split. (Nesting a fence inside an admonition is what raises `ValueError: <Region ...> overlaps <Region ...>`; that is a different shape and not what this does.) 300 passed, `prek run --all-files` clean, docs build succeeds.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

docs/spec.md:3532

NoManifold.has_chart is described as always returning False, but the implementation returns True for charts explicitly bound to NoManifold (i.e., hasattr(chart, "M") and isinstance(chart.M, NoManifold)). This makes the spec statement inaccurate for consumers constructing charts with M=no_manifold.

    - `ndim == 0` (sentinel for "no manifold specified"). Note this does *not*
      distinguish it from a genuine zero-dimensional manifold: `Rn(0).ndim` is
      also `0`. Test with `isinstance(M, NoManifold)`, never on `ndim`.
    - `has_chart(chart)` always returns `False`.

The `coordinax.manifolds.lorentzian` sub-namespace had no entry in the spec at all -- not the namespace, and none of its four verbs. `causal_character` and `rapidity_between` were not so much as mentioned; `proper_time` and `proper_distance` appeared once each, inside `geodesic_distance`'s "use these instead" line. Five sections: the namespace, then a verb each. The namespace section carries what they share, so the verb entries stay short -- the gate is `AbstractLorentzianMetricField`, meaning *exactly one* timelike direction, and the reason each refuses rather than returning something readable as a classification. Also why the namespace is named for the signature: `galileanct` is a 4-dimensional spacetime and is not Lorentzian, so a `spacetime` namespace would promise membership these verbs refuse, while a `minkowski` one would exclude Schwarzschild and FLRW, which carry the marker and acquire all four. Worth pinning, and now pinned: - `causal_character` returns a Python `str`, not an enum and not a number, so it cannot be accidentally arithmetic. It takes `atol`, because a null separation is measure-zero in floating point and needs a band rather than an equality test. - `proper_time` divides by `c` and `proper_distance` does not, so one is a duration and the other a length out of the same length-squared interval. - Each refuses the other's causal class with `ValueError` rather than `nan`: no clock travels between spacelike-separated events, so the question does not apply rather than being undefined. - `rapidity_between` is the odd one -- tangents rather than points, `at=` required because there is no first point to take the base from, and a bare dimensionless `Array` rather than an `Angle`, since rapidity is unbounded and `rad` would wrongly suggest it wraps. It exists because `angle_between` refuses timelike pairs correctly: both `g(u,u)` and `g(v,v)` are negative, so `arccos` would clip and report two observers in relative motion as parallel. All 34 examples were executed under the harness that will run them, not a bare interpreter. That distinction is load-bearing here: `JAX_ENABLE_X64 = "1"` is set in `[tool.pytest.ini_options]`, so an ad-hoc `python` session yields `float32` and every literal would have been wrong -- `Q(1.6341238e-08, 's')` against the `Q(1.63412366e-08, 's')` the suite actually produces. Explanatory prose sits outside the `pycon` fences rather than inside them, per the review on GalacticDynamics#733: everything within a fence renders as code, taking its inline markup with it. The 17 pre-existing failures elsewhere in the file are GalacticDynamics#733's subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Four verbs that were public and unspecified: `metric_representation`, `pt_embed`, `pt_project` and `embedded_twosphere`. Placed with the machinery they belong to rather than in the verb cluster -- `metric_representation` after `metric_matrix`, whose sibling it is, and the embedding trio after `EmbeddedManifold`. `metric_representation` is the one most worth writing down, because its signature invites the wrong reading. It returns a **class**, `DiagonalMetric` or `DenseMetric`, not a populated matrix, and takes no base point: sparsity structure belongs to the (manifold, chart) pair, not to where you stand on it. That is what lets `metric_matrix` keep the O(n) diagonal path instead of contracting a full einsum, and why an `EmbeddedManifold` declares dense -- a pullback through an arbitrary embedding has no reason to be diagonal even when the intrinsic chart is orthogonal. `pt_project` is a projection and not merely an inverse: it is defined on all of the ambient and round-trips exactly only for points already on the surface, which is why it carries overloads `pt_embed` does not. One inconsistency found and recorded rather than smoothed over: the container of a projected angle tracks the ambient chart. Round-tripping through `Spherical3D` gives `Angle(1.04719755, 'rad')`; through `Cart3D`, the same value and unit in a plain `Q(1.04719755, 'rad')`. Written down as observed behaviour with a note that an angle is an element of S^1 and the container is what carries that, so the ambient chart arguably should not decide it. Two things this cost, both worth naming: - The `pt_embed` example first used the Cartesian ambient at the equator, which routes through `cos(pi/2)` and puts `1.2246468e-16` in the output. Moved to the north pole, where the same route is exact. - A `!!! warning` with a nested ```pycon block made sybil raise `ValueError: <Region ...> overlaps <Region ...>` at collection -- the outer indented block and the inner fence both parse as doctests. Flattened to prose plus a plain example. Explanatory prose sits outside the `pycon` fences, per the review on GalacticDynamics#733. All 24 examples verified under `pytest docs/spec.md`, which is the harness that sets `JAX_ENABLE_X64`; 0 failures inside the four added sections. The 17 pre-existing failures elsewhere in the file are GalacticDynamics#733's subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Three agent-facing documents, layered so each fact has one home: - `skills/coordinax/SKILL.md` — using coordinax from outside the repo: the chart/representation/vector/manifold layering, the tangent anchor and `coord_basis` vs `phys_basis` rules, `==` vs `equivalent`, deliberate `Distance`/`Angle` degradation, the static-chart performance rule, and troubleshooting and version-notes tables. - `AGENTS.md` — working inside the repo: spec authority, workspace layout, commands, the conventions that bite, and the testing contract. - `.github/skills/code-review/SKILL.md` — reviewing a change, organised around the two properties that generate most defects here: a missing route composes rather than failing, and scalar-first code meets batched callers. Each check cites the PRs it came from. Each file cites the others rather than restating them. `docs/spec.md` stays authoritative and is cross-linked, not duplicated. `AGENTS.md` and the review skill carry a doc-sync rule from both sides: a change that alters public semantics, adds a hazard, or invalidates an example is not done until these files match it, and review flags the omission. #733 is the precedent — 17 examples in `docs/spec.md` had gone stale because nothing forced the update. The skill's examples are tests. `skills/` joins `testpaths` and the `coordinax` nox package paths, so Sybil executes all 49 `pycon` blocks under `nox -s test` and in CI. `.github/copilot-instructions.md` becomes a pointer to `AGENTS.md`; everything load-bearing moved across. `/AGENTS.md` is dropped from `.gitignore`, where it had reserved a slot for an untracked local file — the tracked file is now canonical, matching quax, quaxed, and quax-blocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings August 18, 2026 20:41

nstarman mentioned this pull request Aug 18, 2026

📝 docs(spec): specify the lorentzian verbs #734

Merged

github-actions Bot added 🔧 Add / update configuration Add or update configuration files. 📝 Add / update documentation Add or update documentation. ✅ Add / update / pass tests Add, update, or pass tests. labels Aug 18, 2026

Copilot started reviewing on behalf of nstarman August 18, 2026 20:49 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Comment thread docs/spec.md

nstarman mentioned this pull request Aug 18, 2026

📝 docs(spec): specify the embedding and metric-representation verbs #735

Merged

nstarman added this to the v0.24.0 milestone Aug 18, 2026

nstarman force-pushed the claude/spec-doctests-under-ci branch from 126a467 to 57438d2 Compare August 18, 2026 21:48

nstarman requested a lite review from Copilot August 18, 2026 22:29

nstarman and others added 2 commits August 18, 2026 18:39

Copilot started reviewing on behalf of nstarman August 18, 2026 22:40 View session

nstarman force-pushed the claude/spec-doctests-under-ci branch from 57438d2 to f787d83 Compare August 18, 2026 22:40

Copilot AI reviewed Aug 18, 2026

View reviewed changes

nstarman merged commit 0d5c7b9 into GalacticDynamics:main Aug 18, 2026
4 of 5 checks passed

nstarman deleted the claude/spec-doctests-under-ci branch August 18, 2026 22:42

This was referenced Aug 19, 2026

🐛 fix(manifolds): infer the manifold from a chart *class*, not only an instance #739

Merged

📝 docs(spec): an angle carries no branch cut; the context does #740

Merged

📝 docs: add agent skills, AGENTS.md, and a doc-sync rule #758

Merged

nstarman mentioned this pull request Aug 19, 2026

📝 docs(spec): Shear matches the code, and Linear gets a section #762

Merged

Navigation Menu

Uh oh!

✅ test(spec): put `docs/spec.md` examples under CI - #733

✅ test(spec): put `docs/spec.md` examples under CI#733
nstarman merged 2 commits into
GalacticDynamics:mainfrom
nstarman:claude/spec-doctests-under-ci

nstarman commented Aug 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nstarman commented Aug 18, 2026

The drift the exclusion let accumulate

Three were not cosmetic

Verification

Note on stacking

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Aug 18, 2026 •

edited

Loading