Skip to content

Stage the renderer components, and stop a missing one being silent - #34

Merged
n1ckyb merged 1 commit into
release/v0.0.2-rcfrom
fix/stage-renderer-components
Aug 10, 2026
Merged

Stage the renderer components, and stop a missing one being silent#34
n1ckyb merged 1 commit into
release/v0.0.2-rcfrom
fix/stage-renderer-components

Conversation

@n1ckyb

@n1ckyb n1ckyb commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Closes #22.

CI ran the whole suite with zero renderers loaded and reported green:

WARNING intentumdiff.plugins.registry: Failed to load renderer plugin 'html': ...
WARNING ...                                                          'llm' ...
WARNING ...                                                        'patch' ...
WARNING ...                                                     'terminal' ...

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.

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, so 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 loses one silently and still reports green — the exact failure being fixed — so found < 4 is an error.

The loader refuses an incomplete set — opt in

INTENTUMDIFF_REQUIRE_ALL_COMPONENTS=1 turns a failed renderer load into a PluginLoadError instead 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_compile passed on a version where neither os nor PluginLoadError was 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

…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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +233 to +235
runs = _json.loads(
get(f"{api}/repos/{org}/intentumdiff-core/actions/runs"
f"?status=success&per_page=20")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

Comment on lines +1072 to +1075
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."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@n1ckyb
n1ckyb merged commit 3662a91 into release/v0.0.2-rc Aug 10, 2026
1 check passed
@n1ckyb
n1ckyb deleted the fix/stage-renderer-components branch August 10, 2026 08:10
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.

1 participant