fix(inspect): suppress banner and spinner in JSON mode#745
Merged
Conversation
Click 8.4 removed the mix_stderr parameter and always mixes stderr into CliRunner.result.output. The banner and spinner added in #718 write to stderr via _stderr_console, which contaminates the JSON payload in result.output — causing all 20 JSON-parsing e2e tests to fail with JSONDecodeError. Suppress the banner and spinner when --format json is requested, matching the expectation that JSON consumers get clean stdout.
zhenchaoni
approved these changes
May 26, 2026
For multimodal models like CLIP, resolve_loader_config narrows the
hf_config to a sub-config (e.g. CLIPTextConfig with model_type
"clip_text_model"). The narrowed type is correct for internal
registry lookups (TasksManager, I/O specs), but the user-facing
InspectResult should show the parent model_type ("clip").
Use parent_hf_config.model_type for the display field while keeping
the narrowed model_type for all internal resolution paths.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
timenick
reviewed
May 26, 2026
Collaborator
timenick
left a comment
timenick
left a comment
There was a problem hiding this comment.
One question on the trade-off: was a test-side fix considered before reaching for the production-side suppression?
The previous fix used only parent_hf_config.model_type, which works when model_id is provided (step 1 loads the parent config). But for --model-type clip (no model_id), parent_hf_config falls back to the narrowed hf_config at line 340, so display_model_type was still "clip_text_model". Add model_type_override as the first precedence level: when the user explicitly passes --model-type, use that value directly. Precedence chain: 1. model_type_override (user's --model-type flag) 2. parent_hf_config.model_type (pre-narrowing, when model_id given) 3. loader_config.model_type (narrowed fallback)
…suppression Revert the banner/spinner suppression in JSON mode — the original design (PR #718) deliberately puts them on stderr so pipe-to-jq users still see progress feedback. The real issue is Click 8.4's result.output mixing both streams; fix the tests to read result.stdout (which preserves the split), matching the pattern already used in test_sys_e2e.py.
…on-side suppression" This reverts commit 8464416.
timenick
approved these changes
May 26, 2026
timenick
added a commit
that referenced
this pull request
May 27, 2026
…757) ## Summary - Folds the seven fixes merged after the v0.1.0 release notes into the existing v0.1.0 changelog section: #745, #746 (inspect), #747, #753 (perf), #750 (eval), #744/#752, #754 (analyze/compile e2e). - Trims the v0.1.0 section to focus on user-visible changes: - Removed telemetry mentions (#728 preview bullet and #693 fix line). - Removed pure-internal CI/test items (CI display name rename, ADO pipeline UI plumbing, mock fixes, branch back-merge).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #734
Two fixes for
winml inspecte2e test failures:1. JSON mode: suppress banner and spinner (20 failures)
mix_stderrand always mixes stderr intoCliRunner.result.output_stderr_console, contaminating the JSON payload and causingJSONDecodeErrorin all JSON-parsing tests--format jsonis requested — JSON consumers expect clean stdout2. CLIP model_type: use parent config type (2 failures)
resolve_loader_confignarrows the hf_config to a sub-config (e.g.clip_text_model), which is correct for internal registry lookupsInspectResult.model_typeshould show the parent type (clip), not the narrowed sub-config typeparent_hf_config.model_typefor the display field while keeping the narrowed type for internal resolution🤖 Generated with Claude Code