Skip to content

feat: expose whether the version profile is exact - #62

Merged
CameronBrooks11 merged 3 commits into
mainfrom
fix/61-expose-profile-exact
Sep 5, 2026
Merged

CameronBrooks11 merged 3 commits into
mainfrom
fix/61-expose-profile-exact

Conversation

@CameronBrooks11

@CameronBrooks11 CameronBrooks11 commented Sep 5, 2026 •

Copy link
Copy Markdown
Member

Closes #61.

get_profile() already returned (profile, exact). OpenRocketInstance.__init__
consumed exact, logged a warning, and discarded it — so no caller could tell an
exact profile from a nearest-older fallback. SimulationPool had the same defect
one layer up: it builds an instance to validate the jar and fire that warning once
in the parent, then threw the instance away.

A log line is not a surface. An application that raises its log level silences the
only signal, leaving no exception, no return value and no attribute. And the obvious
reconstruction is wrong: parse_version maps 24.12.RC.01 to (24, 12), an exact
match whose version strings differ, so or_version != profile.version_string reports
a fallback on every RC and point release.

Adds

  • OpenRocketInstance.profile_exact — True when a profile for the jar's exact
    version is checked in, False on a nearest-older fallback.
  • SimulationPool.profile_exact — the same verdict on the pool, which is the
    monte-carlo path where running on stale enums matters most.

Both are additive. tests/test_api_surface.py freezes module-level __all__, which
is unaffected.

Docs

Four pages described the fallback as a warning, which would have left the fix
undiscoverable to anyone not reading source. docs/guides/multi-version.md — the
page a caller reads for this question — now shows the flag with a worked example and
states why the string comparison is not equivalent. docs/index.md, README.md and
docs/maintainers.md updated to match.

The class docstring uses :ivar: rather than an RST definition list: mkdocs.yml
sets docstring_style: sphinx, and the definition-list form was passed through as
description text and collapsed by Markdown into a single run-on paragraph.
mkdocs build --strict does not catch that.

Verification

5 new test functions, 7 cases with parametrisation. Reverting both assignments and
keeping the tests:

7 failed, 224 passed, 2 skipped, 73 deselected

All seven are red-capable; none is a presence check. Both directions are pinned, so a
hardcoded constant fails too, and test_string_comparison_is_not_a_substitute_for_profile_exact
also kills the mutant that implements profile_exact as the naive string comparison.

Restored: just check && just test — 231 passed, 2 skipped. mkdocs build --strict
clean.

Not in this change

Helper.get_events drops unknown flight-event types and returns a dict that looks
complete — the same defect a layer up, filed as #63. Helper._warn_absent_once and
_warn_on_profile_drift are weaker variants of it. One logical change per commit, and
#63 needs its own decision about the return shape.

get_profile already returns (profile, exact), but __init__ consumed the
flag, logged a warning and discarded it. Nothing on the instance said
whether it was running on the jar's own profile or a nearest-older
fallback, so the one signal was a log line an application can silence by
raising its log level.

Store it as profile_exact. Comparing or_version to
profile.version_string is not a substitute: parse_version maps
24.12.RC.01 to (24, 12), an exact match whose strings differ, so that
reconstruction reports a fallback on every RC and point release.

Closes #61
Review found the flag was shipped but undiscoverable, and that the
docstring did not render.

- The four docs that describe the fallback still taught the log warning
  as the mechanism. multi-version.md is the page a caller reads for this
  question; it now shows the flag and states why the string comparison
  is not equivalent.
- The class docstring used an RST definition list, which griffe's sphinx
  parser passes through as description text and Markdown then collapses
  into one paragraph. Converted to :ivar:, the repo's existing style,
  and verified it renders as a table.
- SimulationPool built an instance to validate the jar and discarded it,
  so a pool caller still had only the log line. It keeps the verdict now.
- The RC case asserted nothing about the comparison its comment claimed
  to demonstrate. That claim is now an executable assertion.
@CameronBrooks11 CameronBrooks11 changed the title fix: expose whether the version profile is exact feat: expose whether the version profile is exact Sep 5, 2026
@CameronBrooks11

CameronBrooks11 commented Sep 5, 2026 •

Copy link
Copy Markdown
Member Author

Updated after independent review

The reviewer verified every claim in the original body (gate counts, the red-capable demonstration, the suppressed-logger reproduction, the RC trap, and that test_api_surface.py freezes module __all__ only) and then found four things worth fixing. All are addressed in 18f48bc.

1. The fix was undiscoverable. Four docs still taught the log warning as the mechanism — docs/guides/multi-version.md being the page a caller reads for exactly this question. Shipping the surface while every doc describes the log line leaves the defect in place for anyone not reading source. multi-version.md now shows the flag with a worked example and states why the string comparison is not equivalent; docs/index.md, README.md and docs/maintainers.md rows updated.

2. The docstring did not render. mkdocs.yml sets docstring_style: sphinx; my RST definition-list block was passed through as description text and Markdown collapsed it into a single run-on <p>. --strict does not catch this. Converted to :ivar: — the style helper.py and jiterator.py already use — and verified against the built HTML that it now renders as a table and the collapsed-paragraph form is gone.

3. SimulationPool had the identical defect. It builds an OpenRocketInstance to validate the jar and fire the warning once in the parent, then discarded it — so a pool caller, on the monte-carlo path where stale enums matter most, still had only the log line. It keeps the verdict now. Two tests added, both shown to fail without the change.

4. A comment claimed a demonstration the test did not make. The RC case's comment said it showed why or_version != profile.version_string is wrong, but the test never performed that comparison — prose asserting something with no executable assertion behind it. It is now its own test that actually asserts the trap.

Also: PR title moved from fix: to feat: (it adds public API, and the CHANGELOG files it under Added), and the CHANGELOG reference style now matches the file's bare (#n) convention instead of introducing a stranded link definition.

Gate after the changes: just check && just test — 231 passed, 2 skipped (was 228; three new tests). mkdocs build --strict clean.

Follow-ups filed separately rather than folded in here — the reviewer found other log-only sites, notably Helper.get_events dropping unknown event types from the returned dict with only a log line.

The previous edit put the flag in front of the parenthetical, making
'checks exactly this' read as a claim about CI coverage. canary.yml runs
happy/summary/declarative_keys/motor_swap/wind_profile against the newest
upstream release and asserts none of them against profile_exact -- it
exercises the fallback path, it does not check the flag.
@CameronBrooks11
CameronBrooks11 merged commit 6ebbaad into main Sep 5, 2026
14 checks passed
@CameronBrooks11
CameronBrooks11 deleted the fix/61-expose-profile-exact branch September 5, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The nearest-older profile fallback is log-only: no caller can branch on it

1 participant