Repository navigation
fix(platform): show a failed website pages read with Try again - #4301
Conversation
ACCEPT — independent review of PR #4301Reviewer: Codex #6, agent Exact head: No blocking findings in the reviewed change. Acceptance is an independent source/regression verdict, not a merge authorization or a claim that queued CI passed. Root's rule remains: a blocking finding overrides acceptance until independently closed with exact-head proof. Acceptance evidence
Independently runInstalled Node 24.21.0, one Vitest worker, 1 GiB Node heap; isolated local checkout and synthetic/mock data only.
Main comparison: fetched Unrun proof / handoff
All retained review files live in |
|
Merge state: this PR now conflicts with #4274 merged as Resolution: keep The ACCEPT at this head (TALE-662) covers source that a doc-only merge leaves unchanged. The new head still needs green CI, and an interdiff confirmation that only |
c488f90 to
201b37a
Compare
8a11394 to
ec8e7ea
Compare
9ba6abb to
dd78431
Compare
dd78431 to
2967ad5
Compare
2967ad5 to
4484261
Compare
4484261 to
041a9f0
Compare
What changed
The Website details dialog showed No pages crawled yet when the first
websites/actions:fetchPagesread failed, even under a row counting crawled pages. The only other signal was a toast, and the dialog had to be reopened to read again.website-view-dialog.tsx): the dialog now keeps the failed read (readFailures) until a read succeeds. With no page on screen, the Website pages section shows the sharedCatalogLoadError: "Couldn't load the website's pages." with Try again. No pages crawled yet now appears only after a read that succeeded with no page.{ websiteId, offset: 0, limit: 20 }as on open). It first moves focus onto the section throughEntityViewSection'sfocusRef, as the knowledge-entry version history does. If a scan refresh heals the read while Try again has focus,onFocusLostmoves focus to the section. A busy retry stays focusable, and each new failure is announced again (failureKey).offset, so the next Load more asked for the window after the failed one. Its answer was then dropped (data.offset !== shownPages.current), and the list could not grow again.websites.pagesDialog.loadFailedin EN/DE/FR.de-CHhas nowebsitesblock and inherits DE.KNOW-B24intests/manual/suites/knowledge.md(current header cost 71 → 72), plus its row inreference/automation.md. It is B24, not B22, because open PRs fix(platform): show failed contact, product and website list reads #4290 and fix(platform): show a dash for an unset product price and stock #4274 each add aKNOW-B22already, and the guide linter refuses duplicate IDs.How I verified
All runs are local, jsdom,
bunx vitestwith one worker.website-view-dialog.test.tsxat head: 39/39 passed. That includes 9 new tests under "a pages read that failed" (one runs per shipped locale: EN/DE/FR) and the updated "stands on the row … shows once it failed", which now expects the scan alert plus the read failure.Fails on main: the head's test file on
origin/main9e8f87f's source gives 9 failed / 30 passed:The real-empty control ("says a site whose read found no page has none, with nothing to retry") passes on main, as a control should.
Mutants of the fix, each red (run against the six failure-path tests from before the locale test was added):
onFocusLostoffset + PAGE_SIZE)lib/i18n/messages.test.ts: 24/24 passed. It enforces key parity, so a missing DE/FR key fails there, not only as an EN fallback.Tests that import the dialog (
websites-table.test.tsx,website-row-actions.test.tsx): 15/15.Type-aware
oxlinton both files,oxfmt --check, opengrep and conflict markers: clean. commitlint passes.Isolated
tsc --noEmitover the changed files (with router, env, UI setup and platform.d.ts): exit 0.bun run knip:check: exit 0.bun run lint:manualandcheck-guide.tsonknowledge.md: OK.Not run here: the whole platform suite, workspace
tscand the real-browser path. CI owns the first two, andKNOW-B24covers the browser path.Closes #3848
Rebased the independently accepted feature onto main
b3976a03f49b65ee3f39fe9a41991f29f6005354. The sole conflict was the manual suite's declared count: main corrected its 71 existing boxes, and this feature adds KNOW-B24, so the combined suite declares and contains72. All six original source/test/locale/knowledge blobs are unchanged; the automation reference is exactly current main plus this feature's original coverage row. A distinct reviewer confirmed preservation, stable IDs and exact source composition. Fresh selected dialog tests pass52/52; the manual guard passes1585boxes and configured formatting passes the two unchanged TSX files. Markdown and YAML are excluded by the repository formatter. These are local proofs; current-head native checks remain mandatory. No new full-stack/browser route-blocking proof is claimed.Current-main rebase
Replayed the previously accepted source
ec8e7ea7fcd7a522607ea6b0c16695b6f5aa19cfonto maind1373d84cd56972501403f62145ec52e6f65d44a, including the merged shared CI repair in #4625. The accepted feature payload and all current-main changes are preserved in one atomic commit. Configured commit and conflict checks pass; earlier behavioral proof remains recorded above. All seven native required checks and full merge-group validation remain required for this new source.Maintenance replay: preserves the accepted feature payload on current main 7d178ca. Includes the merged #4649 Knip cleanup and the exact independently accepted one-line shared CLI inventory repair from #4654 (252f0df), which is still pending native merge on main. The fixed suite inventory keeps its discovery and source/compiled phase guards. Existing feature proof is retained; no fresh full-feature/full-workspace test or hosted-green claim. Native required checks remain mandatory.