Stage the renderer components, and stop a missing one being silent - #34
Conversation
…silent Closes #22. CI ran the whole suite with ZERO renderers loaded and reported green. Every test touching --format html|patch|llm|terminal was skipping, hitting a Python fallback, or asserting on degraded output, and nothing distinguished those from real coverage: WARNING intentumdiff.plugins.registry: Failed to load renderer plugin 'html': ... WARNING ... 'llm': ... 'patch': ... 'terminal': ... ## Why nothing fetched them provision_build_inputs.py enumerates repos ending in `-parser`. Renderers are not parser repos - they are crates in intentumdiff-core - so no code path fetched them at all, and the existing fail-closed check counts only parser repos. Zero missing parsers read as a successful provisioning while four renderers were absent. core#26 now publishes them as the `renderer-components` artifact. This stages it. ## Two properties worth stating Registry pins are enforced where they EXIST. Renderers are not all pinned today; where a pin exists it is checked exactly as for parsers, and where it does not the component is staged with a printed note - so an unpinned component is visible rather than indistinguishable from a verified one. A partial set fails. Staging 3 of 4 renderers loses one silently and still reports green, which is the failure being fixed, so `found < 4` is an error. ## The loader now refuses an incomplete set - opt in INTENTUMDIFF_REQUIRE_ALL_COMPONENTS=1 turns a failed renderer load into a PluginLoadError instead of a warning and a smaller adapter list. CI sets it. Deliberately opt-in, not always-on: a user with a partial install should still get a working diff. Degrading is right for THEM and wrong for a gate - the bug was never that it degraded, it was that a gate degraded silently. ## A note on verification `py_compile` passed on a version where neither `os` nor `PluginLoadError` was imported - it checks syntax, not names. Confirmed by actually importing the module instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63f056cf24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| runs = _json.loads( | ||
| get(f"{api}/repos/{org}/intentumdiff-core/actions/runs" | ||
| f"?status=success&per_page=20") |
There was a problem hiding this comment.
Pin renderer artifacts to the staged core revision
When the newest successful core run belongs to another branch or commit, this unfiltered query selects its renderer artifact even though stage_core() builds the native host from CORE_REF; the publish workflow also invokes this mode when constructing PyPI wheels. This can therefore package renderer components from an incompatible revision, and any unpinned renderers will not be rejected by the checksum check. Filter the workflow runs using the exact revision staged in CORE_DEST, as the parser artifact flow already does.
Useful? React with 👍 / 👎.
| raise PluginLoadError( | ||
| f"{len(failed)} renderer plugin(s) failed to load: {', '.join(sorted(failed))}. " | ||
| "INTENTUMDIFF_REQUIRE_ALL_COMPONENTS=1 is set, so an incomplete component set is " | ||
| "an error rather than a silent downgrade - stage the components, or unset it." |
There was a problem hiding this comment.
Pass both arguments required by PluginLoadError
Whenever a renderer fails to load while INTENTUMDIFF_REQUIRE_ALL_COMPONENTS=1 is set, this call raises TypeError instead of the intended PluginLoadError, because PluginLoadError.__init__ requires separate wasm_path and detail arguments. Thus the newly enabled CI failure path masks the aggregate renderer diagnostic and violates the exception contract expected by callers.
Useful? React with 👍 / 👎.
Closes #22.
CI ran the whole suite with zero renderers loaded and reported green:
Every test touching
--format html|patch|llm|terminalwas skipping, hitting a Python fallback, or asserting on degraded output — and nothing distinguished those from real coverage.Why nothing fetched them
provision_build_inputs.pyenumerates repos ending in-parser. Renderers are not parser repos — they are crates inintentumdiff-core— so no code path fetched them at all. And the existing fail-closed check counts only parser repos, so zero missing parsers read as a successful provisioning while four renderers were absent.core#26 now publishes them as the
renderer-componentsartifact. This stages it.Two properties worth stating
Registry pins are enforced where they exist. Renderers are not all pinned today; where a pin exists it is checked exactly as for parsers, and where it does not the component is staged with a printed note — so an unpinned component is visible rather than indistinguishable from a verified one.
A partial set fails. Staging 3 of 4 loses one silently and still reports green — the exact failure being fixed — so
found < 4is an error.The loader refuses an incomplete set — opt in
INTENTUMDIFF_REQUIRE_ALL_COMPONENTS=1turns a failed renderer load into aPluginLoadErrorinstead of a warning plus a smaller adapter list. CI sets it.Deliberately opt-in, not always-on: a user with a partial install should still get a working diff. Degrading is right for them and wrong for a gate — the bug was never that it degraded, it was that a gate degraded silently.
A note on verification
py_compilepassed on a version where neitherosnorPluginLoadErrorwas imported — it checks syntax, not names. Confirmed by importing the module instead.Expected effect
CI currently reports 2237 passed / 271 skipped against 2416 / 169 locally. This should close a large part of that gap, and anything still skipping afterwards will be skipping for a reason we can name.
🤖 Generated with Claude Code