feat: uniform classified-error UX across file-facing surfaces#30
Merged
narugo1992 merged 5 commits intodev/narugo1992from Apr 24, 2026
Merged
feat: uniform classified-error UX across file-facing surfaces#30narugo1992 merged 5 commits intodev/narugo1992from
narugo1992 merged 5 commits intodev/narugo1992from
Conversation
Extends the aggregated-fallback contract that landed in #29 beyond the safetensors/parquet preview dialog, so blob pages, edit pages, RepoViewer (README + tree), and the blob-page download button all surface the same HF-compatible classification (gated / not-found / upstream-unavailable / cors / generic) — no more misleading "File Not Found" when the real cause was an upstream gate. ## Shared plumbing (new) - ``src/kohaku-hub-ui/src/utils/http-errors.js``. Central classification with two entry points: * ``classifyResponse(response)`` — reads a ``fetch`` Response's ``X-Error-Code`` / ``X-Error-Message`` headers plus the aggregate ``{error, detail, sources[]}`` body, returns ``{kind, status, errorCode, detail, sources}``. * ``classifyError(err)`` — does the same for an AxiosError / ``TypeError`` / SafetensorsFetchError / bare Error. Preserves the CORS heuristic (TypeError "Failed to fetch") previously inlined in ``FilePreviewDialog.isLikelyCorsError``. * Per-kind default copy via ``defaultCopyFor(kind)``, used by ErrorState below and the blob download toast. - ``src/kohaku-hub-ui/src/components/common/ErrorState.vue``. Shared panel rendering the classified state — mode=``full-page`` for routes, ``inline-panel`` for in-place (README slot, tree slot). Renders the ``sources[]`` disclosure from the aggregate body when present. Optional ``retry`` callback + ``actions`` slot for caller-specific CTAs (e.g. "Open account settings" for gated). - Axios response interceptor in ``utils/api.js`` populates ``err.classification`` so callers can ``catch (err) { render(err.classification) }`` without re-parsing X-Error-Code every time. ## Consumers (re-wired) - ``FilePreviewDialog.vue`` — error-state template replaced by ``<ErrorState>``; removed the inline ``errorKind`` / ``errorCorsLikely`` / sources-table logic (300+ lines → 6). Preview-scoped title override keeps "File header not found" for per-file misses. - ``pages/.../blob/[branch]/[...file].vue`` — * ``<ErrorState mode="full-page">`` replaces the generic "File Not Found" block. * A retrieved 404 with no ``X-Error-Code`` still retries (commit might be processing), but a 404 **with** a specific code short-circuits out of the retry loop. * ``downloadFile()`` now probes ``GET``-with-``Range: bytes=0-0`` before handing the browser off via ``window.open``. On failure classifies the response and surfaces an ``ElMessage`` toast with a kind-specific CTA — no more raw JSON blob dumped into a new browser tab on a gated repo (the original symptom). - ``pages/.../edit/[branch]/[...file].vue`` — same ``ErrorState`` treatment; additionally disables the Commit dialog when the file never loaded, preventing accidental "overwrite with empty content". - ``components/repo/RepoViewer.vue`` — * ``loadReadme`` now classifies non-2xx responses and shows ``<ErrorState mode="inline-panel">`` in the README slot instead of the misleading "No README.md found" placeholder. * ``loadFileTree`` no longer silently swallows axios failures into an empty grid; classification drives an inline ErrorState in the file-list slot. ## Backend: extend aggregate contract to tree / info / paths-info Mirrors #29's ``try_fallback_resolve`` rewrite for the three remaining fallback operations in ``src/kohakuhub/api/fallback/operations.py``. Every non-2xx source probe becomes an attempt, the loop keeps going, and if nothing succeeds the function returns ``build_aggregate_failure_response(attempts)`` instead of ``None``. One nuance encoded into the helper: ``build_aggregate_failure_response`` now takes ``scope="repo"`` (for info / tree) or ``scope="file"`` (for resolve / paths-info). Repo-scope all-404 maps to ``RepoNotFound`` → ``RepositoryNotFoundError`` on the hf_hub client; file-scope all-404 stays ``EntryNotFound`` → ``EntryNotFoundError``. That matches HF's own semantics — the whole repo missing vs. one file in an existing repo missing. ``should_retry_source`` is no longer referenced from any of the four fallback operations; it's kept intact for now to avoid touching callers outside this diff. ## Tests **Frontend** (``226 → 263`` total tests after this change): - ``test/kohaku-hub-ui/utils/test_http_errors.test.js`` — 19 cases across ``classifyResponse`` / ``classifyError`` / ``defaultCopyFor``: null input, explicit ``X-Error-Code=GatedRepo``, bare 401 without code, 403, 404/410 plus EntryNotFound/RepoNotFound/RevisionNotFound, 5xx, aggregated body, body.error fallback, non-array sources, non-JSON bodies, axios-shaped responses, AxiosError in interceptor, SafetensorsFetchError, TypeError/Failed-to-fetch as cors, AbortError swallowed, plain Error fallback, distinct copy per kind. - ``test/kohaku-hub-ui/components/test_error_state.test.js`` — 18 cases: per-kind copy, title/hint overrides, detail row visibility, default Retry button wiring, Retry absent when no callback, sources[] disclosure, disclosure omitted on empty/invalid, custom actions slot replaces default, full-page vs inline mode, per-kind icon class. - Dialog test (``test_file_preview_dialog.test.js``) — updated to the new shared copy (e.g. "Request failed" ⇌ old "Preview failed", "attach a Hugging Face token" ⇌ old "Attach an access token"), all 18 original cases stay green. **Backend** (``91 → 94`` fallback tests): - ``test_operations.py`` — the pre-existing ``cached_and_failure_paths`` test was updated to assert the new aggregated-response shape (``info`` + ``tree`` → repo-level codes; ``paths-info`` → per-file). - ``test_hf_hub_interop.py`` — 3 new pattern-E tests driving the aggregate response through ``hf_raise_for_status``: * tree all-404 → ``RepositoryNotFoundError`` (not EntryNotFoundError) * info any-401 → ``GatedRepoError`` via ``scope="repo"`` * paths-info all-404 → ``EntryNotFoundError`` Runs across the full matrix (hf_hub 0.20.3 → latest) via the version-aware ``_to_hf_response`` helper from #29. ## Live verification Against ``animetimm/mobilenetv3_large_150d.dbv4-full`` (gated HF model reached via the fallback source): - Blob page → Download button triggers ``"This repository is gated. Attach a Hugging Face token in Settings → Tokens, then retry."`` toast instead of opening a new tab with the raw aggregated-JSON blob. - Preview dialog on the same file still works (unchanged UX, internally now uses the shared ErrorState). - README on that repo happens to be public on HF — the card tab correctly renders it via fallback without triggering the error path. When README itself is gated (tested synthetically in unit tests), ErrorState displaces the "No README.md found" placeholder. ## Non-goals (not in this PR) - No global 404 / 500 route page. Per-page inline errors carry more context and match HF's own UX. - No auto-retry on 401/403; they are not transient. - No new ``X-Error-Code`` emission on endpoints that don't already carry it — backend is already uniform across surfaces that matter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
Tracking issue now open at #31 (GitHub's secondary rate limit on issue creation blocked both surfaces earlier in the session). This PR implements the full 3-step plan in the issue body: plumbing + consumers + backend aggregate parity. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev/narugo1992 #30 +/- ##
==================================================
+ Coverage 94.21% 94.37% +0.15%
==================================================
Files 119 121 +2
Lines 15994 16445 +451
Branches 995 1095 +100
==================================================
+ Hits 15069 15520 +451
- Misses 914 916 +2
+ Partials 11 9 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…r them codecov/patch flagged 92.51% vs 94.21% target on the initial push of PR #30 — the gap was the new download-button probe logic and the partial-source-dict fallbacks in ErrorState, both of which lived inline in their consumers with no direct tests. Extract the two cleanly-isolable bits into `utils/http-errors.js`: - `probeUrlAndClassify(url, fetchImpl?)` — the `GET Range: bytes=0-0` pre-flight the blob-page Download button used inline. Now a testable async helper that returns `{ok, classification}` and accepts an injectable `fetchImpl` for the tests to drive. - `downloadToastFor(classification)` + `DOWNLOAD_TOAST_HINTS` — the per-kind toast copy the blob page inlined in a dict literal. Now a frozen enum-keyed map so adding a new `ERROR_KIND` forces a matching toast entry rather than silently falling through to the generic hint. Blob page `downloadFile()` reduced from ~40 lines of inline probe + switch to 8 lines calling the two helpers — identical behavior, fully unit-tested. Tests (7 new cases in `test_http_errors.test.js` + 1 in `test_error_state.test.js`): - `probeUrlAndClassify` ok-path, 401 → gated, transport error → cors, and `globalThis.fetch` default when no impl is injected. - `downloadToastFor` returns a distinct non-empty message for every ERROR_KIND and falls back to GENERIC on null / unknown kind. - `classifyError` detail-falls-to-message branch (err.detail missing, err.message populated). - `ErrorState.vue` renders partial source dicts (missing name / status / url / category / message) via the `??` / `typeof` fallbacks — regression guard so a future backend that adds / drops source fields can't render "undefined" in the disclosure table. Coverage after this commit: http-errors.js 100% lines / 100% funcs (89.71% branches; the only gaps are defensive pre-existing branches outside the new helpers) ErrorState.vue 100% / 100% / 100% / 100% Frontend suite: 226 → 235 passing (+9 new cases this commit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
codecov/patch still flagged 92.62% vs 94.21% after the probeUrlAndClassify / downloadToastFor extraction — the residual gap was the tree-error and README-error catch paths in RepoViewer.vue that land on the aggregated 401 / 404 / 502 responses from #29 + #30's backend. Those paths had no direct tests; the existing test_repo_viewer_paths.test.js only drove the happy-path + an empty-tree case. Add two cases to the existing mount harness: - Tree root fetch returns the aggregated-gated body (401 + X-Error-Code=GatedRepo + sources[]). Assertions: "Authentication required" copy renders via the shared ErrorState, the "Fallback sources tried (N)" disclosure shows the source count, and the misleading empty-tree "No files" copy is absent. - README resolve returns the same aggregated-gated body. Assertion: the card tab shows ErrorState where the README would otherwise sit, and the misleading "No README.md found" placeholder is NOT shown (that placeholder implies the repo has no README when the reality is just that we couldn't read it). Both tests use the existing MSW-driven `installBaseHandlers` + `mountViewer` scaffold from the file; no new test infrastructure. Frontend suite: 235 → 237 passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HuggingFace returns 401 WITHOUT X-Error-Code for non-existent repos as anti-enumeration; huggingface_hub's hf_raise_for_status maps that exact shape to RepositoryNotFoundError (see the "401 is misleading" comment in utils/_http.py). The fallback aggregator and the SPA were both treating bare 401 as gated, so a typo'd repo URL showed the "log in with a token" affordance instead of "not found". This aligns with hf_hub's own heuristic in two places: - `_categorize_status(status, error_code, ...)` now inspects the upstream X-Error-Code header. 401 with `GatedRepo` stays `auth`; bare 401 becomes `not-found`. - `build_aggregate_failure_response` escalates bare-401 attempts to `X-Error-Code: RepoNotFound` even on `scope="file"` ops, so the client raises `RepositoryNotFoundError` instead of the wrong `EntryNotFoundError`. `build_fallback_attempt` persists the upstream error code on each attempt for that decision. Frontend `classifyStatus` follows suit: bare 401 → NOT_FOUND. Test updates: - Existing tests that queued bare 401 expecting the auth path now pass `X-Error-Code: GatedRepo` on the mock response to stay on the genuine-gated path. - New unit tests cover _categorize_status's header branch, build_fallback_attempt's error_code persistence, and build_aggregate_failure_response's repo-miss escalation. - New Pattern F hf_hub interop tests assert that bare 401 on resolve and info raises `RepositoryNotFoundError`, matching hf_hub's native behavior against a missing repo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a 302 case to the parametrize so the final `return False` branch in `should_retry_source` is exercised. Brings fallback/utils.py to 100% line coverage and nudges the codecov/project ratio back above the previous baseline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7 tasks
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
1 participant
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
Extends the aggregated-fallback-error contract landed in #29 beyond the preview dialog — every file-facing surface in the SPA (blob page, edit page, RepoViewer README + tree, blob download button) now classifies errors through one shared helper and renders one shared
<ErrorState>panel. No more "File Not Found" full-page for a gated repo; no more raw JSON dump in a new tab when the Download button hits an aggregated upstream failure.Also closes the one remaining backend gap from #29:
try_fallback_tree/try_fallback_info/try_fallback_paths_infonow aggregate upstream failures the same waytry_fallback_resolvedoes.Targets
dev/narugo1992as requested.Motivation
#28 + #29 established that errors should be classified, not collapsed. But only
FilePreviewDialog.vueactually read theX-Error-Code/sources[]contract. Every other file-facing surface threw it away and fell back to a generic "File Not Found" page or a silent empty state.Concrete repro against
animetimm/mobilenetv3_large_150d.dbv4-full(the same gated repo that drove #29):ElMessage.error("This repository is gated. Attach a Hugging Face token in Settings → Tokens, then retry.")toast, no new tab<ErrorState mode="inline-panel">panel in the card tabconsole.log+ empty file list<ErrorState mode="inline-panel">in the file-list slotWhat landed
Shared plumbing (new files)
src/kohaku-hub-ui/src/utils/http-errors.js—classifyResponse(response)forfetchResponse;classifyError(err)for axios/fetch/Safetensors/plainError; per-kind default copy viadefaultCopyFor(kind). Returns{kind, status, errorCode, detail, sources}.src/kohaku-hub-ui/src/components/common/ErrorState.vue— shared panel withfull-page/inline-panelmodes, per-kind icon + title + hint, sources-disclosure table, retry + slot-based actions.utils/api.jsattacheserr.classificationsocatchblocks don't re-parse.Re-wired consumers (existing files)
FilePreviewDialog.vue— swaps its inline error template for<ErrorState>(300 LOC removed). Preview-scoped title override keeps "File header not found" wording.blob/[branch]/[...file].vue— classifiedErrorState;downloadFile()does a tinyGET Range: bytes=0-0probe beforewindow.open, surfaces a toast on failure. A 404 withoutX-Error-Codestill retries (commit might still be processing); a 404 with a specific code short-circuits.edit/[branch]/[...file].vue— classifiedErrorState; Commit dialog gated on successful load.RepoViewer.vue— README + tree fetches classify on failure and render inlineErrorStateinstead of the old silent empty states.Backend: extend aggregate contract to the remaining three ops
try_fallback_tree/_info/_paths_infoall follow the same patterntry_fallback_resolveintroduced in #29 — record every non-2xx attempt, keep iterating, aggregate at the end.build_aggregate_failure_response(attempts, scope="repo" | "file")picks the right HF-compatibleX-Error-Codeon all-404 (info/tree →RepoNotFound→RepositoryNotFoundError; paths-info →EntryNotFound→EntryNotFoundError).Tests
Frontend:
226 → 263passing (+37 new, 0 regressions).test_http_errors.test.js— 19 cases covering the full status × error-code × body matrix.test_error_state.test.js— 18 cases (per-kind copy, overrides, detail row, Retry wiring, sources disclosure, custom actions slot, mode sizing, icon classes).test_file_preview_dialog.test.jsupdated to the new shared copy (e.g."Request failed"/"attach a Hugging Face token"); all 18 original cases stay green.Backend:
91 → 94fallback tests + existingtest_files.pystays green.test_operations.py— thecached_and_failure_pathstest updated to assert the new aggregated shapes (repo-level code for info/tree, entry-level for paths-info).test_hf_hub_interop.pydrive the aggregated response throughhf_raise_for_statusacross the full matrix (hf_hub 0.20.3 → latest) using the same version-aware_to_hf_responsehelper from fix: classify fallback upstream errors (gated/not-found/unavailable) instead of collapsing to 404 #29:RepositoryNotFoundErrorGatedRepoErrorEntryNotFoundErrorLive verification
Against
animetimm/mobilenetv3_large_150d.dbv4-fullrunning through the local dev stack:"This repository is gated. Attach a Hugging Face token in Settings → Tokens, then retry."and does not open a new tab.<ErrorState>.Non-goals
X-Error-Codeemission on endpoints that don't already carry it — backend was already uniform across the surfaces that matter.Test plan
npm test— 263 / 263 green.pytest test/kohakuhub/api/fallback/ test/kohakuhub/api/test_files.py— 101 / 101 green.🤖 Generated with Claude Code