Skip to content

fix(platform): show a failed website pages read with Try again - #4301

Merged
yannickmonney merged 1 commit into
mainfrom
fix/website-pages-read-error
Oct 11, 2026
Merged

yannickmonney merged 1 commit into
mainfrom
fix/website-pages-read-error

Conversation

@yannickmonney

@yannickmonney yannickmonney commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

What changed

The Website details dialog showed No pages crawled yet when the first websites/actions:fetchPages read 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.

  • Failed read kept, not shown as empty (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 shared CatalogLoadError: "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.
  • Focus-safe Try again: it re-sends the first window for the open state (for All, the same { websiteId, offset: 0, limit: 20 } as on open). It first moves focus onto the section through EntityViewSection's focusRef, as the knowledge-entry version history does. If a scan refresh heals the read while Try again has focus, onFocusLost moves focus to the section. A busy retry stays focusable, and each new failure is announced again (failureKey).
  • No double message: a failure the list shows itself raises no toast. A failed read for a state the reader has already left (All → Failed) is ignored.
  • Later pages: rows already shown stay, and the toast still fires. Load more now asks for the window right after the shown rows. Before, the failed attempt had already advanced 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.
  • Copy: websites.pagesDialog.loadFailed in EN/DE/FR. de-CH has no websites block and inherits DE.
  • Manual: KNOW-B24 in tests/manual/suites/knowledge.md (current header cost 71 → 72), plus its row in reference/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 a KNOW-B22 already, and the guide linter refuses duplicate IDs.

How I verified

All runs are local, jsdom, bunx vitest with one worker.

  • website-view-dialog.test.tsx at 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/main 9e8f87f's source gives 9 failed / 30 passed:

    • the 5 failure-path tests;
    • the EN/DE/FR test, once per locale;
    • the updated alert test.

    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):

    Mutant Tests red
    Try again without moving focus first 2
    No onFocusLost 1
    Old Load more offset (offset + PAGE_SIZE) 1
    Toast on every failure 1
    No left-state guard 1
    Empty state ignores the failure 3
  • 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 oxlint on both files, oxfmt --check, opengrep and conflict markers: clean. commitlint passes.

  • Isolated tsc --noEmit over the changed files (with router, env, UI setup and platform .d.ts): exit 0.

  • bun run knip:check: exit 0. bun run lint:manual and check-guide.ts on knowledge.md: OK.

Not run here: the whole platform suite, workspace tsc and the real-browser path. CI owns the first two, and KNOW-B24 covers 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 ec8e7ea7fcd7a522607ea6b0c16695b6f5aa19cf onto main d1373d84cd56972501403f62145ec52e6f65d44a, 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.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

ACCEPT — independent review of PR #4301

Reviewer: Codex #6, agent 8c78cac9-4b65-4630-b901-0a52f2317ff3, run f3da6afb (TALE-662), distinct from author agent #4 / run 17c7ade9.

Exact head: c488f902a431bb772d831e8d072c3b35c9348ea1, unchanged on final readback at 12:46Z, 4 October 2026. Contract: TALE-280 / #3848. PR: #4301.

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

  • Initial failure: readFailures distinguishes a failed read from a successful empty result. With no loaded row the shared CatalogLoadError renders localized failure copy and Try again; the normal empty copy is suppressed. The action opts out of the default toast, and its callback only toasts when rows already exist.
  • Retry/focus: Retry calls the first window of the selected state (websiteId, offset 0, limit 20) and focuses the named Website pages section before the control can disappear. focusRef makes that section programmatically focusable. onFocusLost handles a background refresh healing the error while Retry holds focus. Repeated failures update failureKey without replacing the button. The shared primitive stays focusable/inert while busy.
  • Later-page failure: existing rows and hasMore survive. Load more uses the shown-row count, not the offset of the failed attempt, so retry requests offset 20 rather than skipping to 40. The test verifies 20 retained rows, the toast, and 30 rows after recovery.
  • True empty: a successful zero-page read still renders No pages crawled yet, without an alert or Retry. This control passes on both head and main's source.
  • Locales: new EN/DE/FR catalog entries match the intended error. Existing shared Retry translations are reused. Sparse de-CH contains no websites block and inherits DE. Three locale component cases and full catalog parity checks pass.
  • Stale filter error: an error for a state the reader has left is ignored, while an error for the current filter renders the failure.

Independently run

Installed Node 24.21.0, one Vitest worker, 1 GiB Node heap; isolated local checkout and synthetic/mock data only.

Check Observed result Evidence
Dialog plus direct consumers (table, row actions) 54/54 pass (39 dialog + 15 consumers) head-ui-tests.log
Dialog alone at head 39/39 pass head-dialog-tests.log
Head test file/catalogs with main's exact component 9 fail / 30 pass, expected exit 1 main-regression.log
Platform locale/parity suite 24/24 pass i18n-tests.log
Shared CatalogLoadError/View suite 12/12 pass, including busy-focus retention, repeated announcements, focus handoff and no focus theft shared-error-tests.log
Scoped oxlint, one thread (not type-aware) exit 0 lint.log
oxfmt check, one thread exit 0; formatter reports the two TS/TSX files matched format.log
Restored source and whitespace git diff --exit-code for component and git diff --check: exit 0 terminal readback

Main comparison: fetched origin/main is 9e8f87f6832cf352eb70a0ad666bbec6fd9d1a57. The temporary component's Git blob hash was 64c87b13246848e5fe280d08463426248801ce06, equal to main's component blob. Head tests and catalogs stayed unchanged. Main fails the initial-error/retry, repeat-error focus, background focus recovery, left-filter rejection, EN/DE/FR and later-page retry regressions, plus the updated scan-alert expectation. The component was then restored exactly to the reviewed head.

Unrun proof / handoff

  • Not independently run: real-browser/full-stack HTTP rejection and keyboard/layout/screen-reader path, manual KNOW-B24, full platform suite, workspace or isolated TypeScript check, type-aware lint, SAST, manual-guide gates or author's mutation experiments. The dialog tests invoke mocked action callbacks and keep isPending: false; shared-component tests separately prove the busy state, not end-to-end hook integration. The author's reported broader local checks are not presented as my own observations.
  • CI: five exact-head Candidate source / Resolve source checks are still queued/pending at 12:46Z; no green CI claim. See ci-summary.txt and ci-check-runs.json. No workflow was rerun or cancelled.
  • Next owner: TALE-359 / fleet manager retains CI and normal merge coordination; admitted local QA retains any required browser/manual proof. A future head change requires a fresh exact-head review. No merge, push, source commit or card move was performed.

All retained review files live in /agent/output/1391a23b-37e7-43a3-992f-bc5599eff89e/.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

Merge state: this PR now conflicts with main in the manual suite only. From agent #2, TALE-359 run d6483ad7.

#4274 merged as 9f3c2541b and appended KNOW-B22 (A product without a price reads as unpriced) after the same suites/knowledge.md box this PR appends KNOW-B24 after. At this head (c488f902a431bb772d831e8d072c3b35c9348ea1), git merge-tree against main 9f3c2541b conflicts only in services/platform/tests/manual/suites/knowledge.md. No source file conflicts.

Resolution: keep main's KNOW-B22 box and put this PR's KNOW-B24 box after it. The two IDs don't collide. Run bun run lint:manual before pushing: CI doesn't run it, and duplicate box IDs pass both git and CI.

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 knowledge.md changed. I pushed nothing.

@yannickmonney
yannickmonney force-pushed the fix/website-pages-read-error branch from c488f90 to 201b37a Compare October 9, 2026 02:26
@yannickmonney
yannickmonney force-pushed the fix/website-pages-read-error branch 4 times, most recently from 8a11394 to ec8e7ea Compare October 9, 2026 10:51
@yannickmonney
yannickmonney force-pushed the fix/website-pages-read-error branch 2 times, most recently from 9ba6abb to dd78431 Compare October 9, 2026 15:01
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@yannickmonney
yannickmonney removed this pull request from the merge queue due to a manual request Oct 9, 2026
@yannickmonney
yannickmonney force-pushed the fix/website-pages-read-error branch from dd78431 to 2967ad5 Compare October 9, 2026 23:50
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 10, 2026
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@yannickmonney
yannickmonney removed this pull request from the merge queue due to a manual request Oct 10, 2026
@yannickmonney
yannickmonney force-pushed the fix/website-pages-read-error branch from 2967ad5 to 4484261 Compare October 10, 2026 09:07
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 10, 2026
@yannickmonney
yannickmonney removed this pull request from the merge queue due to a manual request Oct 10, 2026
@yannickmonney
yannickmonney force-pushed the fix/website-pages-read-error branch from 4484261 to 041a9f0 Compare October 11, 2026 00:05
@yannickmonney
yannickmonney added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit d082518 Oct 11, 2026
64 checks passed
@yannickmonney
yannickmonney deleted the fix/website-pages-read-error branch October 11, 2026 02:15
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.

bug(platform): Website details shows “No pages crawled yet” after an initial pages read failure

1 participant