Skip to content

🐛 fix(skills): update the pt_map output for canonical angular containers - #775

Merged
nstarman merged 1 commit into
GalacticDynamics:mainfrom
nstarman:claude/fix-skill-md-angle
Aug 20, 2026
Merged

🐛 fix(skills): update the pt_map output for canonical angular containers#775
nstarman merged 1 commit into
GalacticDynamics:mainfrom
nstarman:claude/fix-skill-md-angle

Conversation

@nstarman

Copy link
Copy Markdown
Contributor

main is red, and every open PR with it.

#752 made pt_map return Angle for angular components rather than Quantity — the point of that change — but the SKILL.md doctest was written against the old behaviour and never updated:

skills/coordinax/SKILL.md:45: SybilFailure
Expected  {'r': Q(3.74165739, 'km'), 'theta': Q(0.64052231, 'rad'),     'phi': Q(1.10714872, 'rad')}
Got       {'r': Q(3.74165739, 'km'), 'theta': Angle(0.64052231, 'rad'), 'phi': Angle(1.10714872, 'rad')}

skills/ is in testpaths, so this fails the entire matrix — every Python version, every OS — on any branch with #752 in its ancestry, which is now all of them. The failure is unrelated to whatever those branches actually change, so none of them can show a real CI signal until this lands.

How it got through

Mine to have caught. #752's doctest sweep ran over src/, packages/, README.md and docs/ — a list I wrote by hand. skills/ was not on it.

The timing hid it: SKILL.md arrived in #758, which merged after #752's branch had its last full-suite run and before #752 itself merged. So no run on that branch ever saw the file, and the conflict only existed once both were in main.

The lesson is narrower than "run the full suite" — I did run it. It is that a hand-enumerated sweep can silently omit a path the config already knows about. pyproject.toml lists testpaths; the sweep should have read it rather than restated it.

Scope

Checked the rest of skills/ for the same pattern rather than fixing only the reported line — that was the only occurrence. skills/: 49 passed. nox -s precommit clean. Full suite running.

🤖 Generated with Claude Code

…iners

`main` is red, and every open PR with it. GalacticDynamics#752 made `pt_map` return `Angle` for
angular components rather than `Quantity` -- the point of that change -- but the
`SKILL.md` doctest was written against the old behaviour and not updated:

    Expected  {'r': Q(...), 'theta': Q(...),     'phi': Q(...)}
    Got       {'r': Q(...), 'theta': Angle(...), 'phi': Angle(...)}

`skills/` is in `testpaths`, so this fails the whole matrix on every branch that
has GalacticDynamics#752 in its ancestry, which is now all of them. It is unrelated to whatever
those branches touch.

Mine to have caught. GalacticDynamics#752's doctest sweep ran over `src/`, `packages/`,
`README.md` and `docs/`, and `skills/` was not in that list -- it arrived in
GalacticDynamics#758, which merged after that branch's last full run and before GalacticDynamics#752 did, so no
run on the branch ever saw the file. A sweep enumerated by hand missed a
testpath the config already knew about; it should have read `testpaths`.

Checked the rest of `skills/` for the same pattern rather than fixing only the
reported line; that was the only occurrence.
Copilot AI lite review requested due to automatic review settings August 20, 2026 12:00
@github-actions github-actions Bot added the 🐛 Fix a bug Fix a bug. label Aug 20, 2026
@nstarman nstarman added this to the v0.24.0 milestone Aug 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the coordinax user skill doctest to reflect the post-#752 behavior where pt_map returns Angle containers for angular components, fixing a Sybil doctest failure that currently breaks CI for all branches based on main.

Changes:

  • Adjusts the pt_map(cart3d -> sph3d) example output in skills/coordinax/SKILL.md to expect Angle(...) for theta/phi.
  • Ensures the skill doctests align with current container canonicalization semantics introduced in #752.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.71%. Comparing base (ff1167a) to head (bdb8d67).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #775      +/-   ##
==========================================
+ Coverage   96.57%   96.71%   +0.13%     
==========================================
  Files         267      268       +1     
  Lines        8963     9125     +162     
==========================================
+ Hits         8656     8825     +169     
+ Misses        307      300       -7     

☔ 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.

@nstarman
nstarman merged commit 015e547 into GalacticDynamics:main Aug 20, 2026
16 checks passed
@nstarman
nstarman deleted the claude/fix-skill-md-angle branch August 20, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 Fix a bug Fix a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants