Install Playwright browsers in CI and push non-incident HTML sources behind feeds - #113
Conversation
Agent-Logs-Url: https://github.com/potemkin666/AlbertAlert/sessions/2a1f5bd3-98fd-4ec6-94a2-1047ece8090d Co-authored-by: potemkin666 <183807833+potemkin666@users.noreply.github.com>
Agent-Logs-Url: https://github.com/potemkin666/AlbertAlert/sessions/2a1f5bd3-98fd-4ec6-94a2-1047ece8090d Co-authored-by: potemkin666 <183807833+potemkin666@users.noreply.github.com>
Agent-Logs-Url: https://github.com/potemkin666/AlbertAlert/sessions/2112c38c-9289-4c53-abb4-eeb1c4ccc571 Co-authored-by: potemkin666 <183807833+potemkin666@users.noreply.github.com>
Agent-Logs-Url: https://github.com/potemkin666/AlbertAlert/sessions/2112c38c-9289-4c53-abb4-eeb1c4ccc571 Co-authored-by: potemkin666 <183807833+potemkin666@users.noreply.github.com>
💡 Codex ReviewLines 11978 to 11980 in 3d1870e
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
Pull request overview
This PR improves feed build reliability and scheduling fairness by ensuring Playwright Chromium is always available in CI, adding timeout-based auto-quarantine logic, and biasing the scheduler toward machine-readable sources over non-incident HTML scrapers.
Changes:
- Install Playwright Chromium unconditionally in CI workflows so browser fallback can’t fail due to missing binaries.
- Add timeout-streak tracking (
consecutiveTimeoutFailures) and auto-quarantine once the timeout threshold is hit. - Slow down refresh cadence for non-incident HTML sources (default lane cadence ×2, min 1h) and add tests covering the new behavior.
Reviewed changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/update-live-feed.yml |
Always installs Playwright Chromium for scheduled live-feed builds. |
.github/workflows/ci-feed-validation.yml |
Always installs Playwright Chromium for validation + smoke feed builds. |
scripts/build-live-feed/config.mjs |
Adds timeout auto-quarantine threshold config and doubles default HTML refresh interval. |
scripts/build-live-feed.mjs |
Tracks consecutive timeout failures and uses them in health scoring + auto-quarantine decisions. |
scripts/ci/seed-quarantine-from-health.mjs |
Seeds quarantine based on production health, now including timeout threshold. |
tests/seed-quarantine.test.mjs |
Adds coverage for timeout-threshold seeding behavior. |
tests/error-pattern-analysis.test.mjs |
Adds assertions + scenarios around timeout streak tracking and quarantine triggers. |
tests/decision-logic.test.mjs |
Adds tests for HTML cadence being slower than machine-readable sources; asserts new timeout constant. |
source-quarantine.html |
Updates generated UI artifact meta defaults (timestamp/count placeholders). |
data/top-20-source-remediation.json |
Updates generated remediation output snapshot. |
data/source-remediation-sweep.json |
Updates generated remediation sweep output snapshot. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const AUTO_QUARANTINE_FAILURE_THRESHOLD = Number( | ||
| process.env.ALBERTALERT_AUTO_QUARANTINE_FAILURE_THRESHOLD || 6 | ||
| ); | ||
| const AUTO_QUARANTINE_TIMEOUT_THRESHOLD = Number( | ||
| process.env.ALBERTALERT_AUTO_QUARANTINE_TIMEOUT_THRESHOLD || 4 |
There was a problem hiding this comment.
AUTO_QUARANTINE_TIMEOUT_THRESHOLD is parsed with Number(env || 4). If the env var is set to a non-numeric value this becomes NaN (disabling timeout quarantines), and if it’s set to 0 it will quarantine everything (since all streaks are >= 0). Since this script claims to mirror build-live-feed/config defaults (which enforce a minimum via envInt), consider reusing the same parsing/validation logic here (fallback on NaN and enforce a sane minimum, e.g. >= 1).
| const AUTO_QUARANTINE_FAILURE_THRESHOLD = Number( | |
| process.env.ALBERTALERT_AUTO_QUARANTINE_FAILURE_THRESHOLD || 6 | |
| ); | |
| const AUTO_QUARANTINE_TIMEOUT_THRESHOLD = Number( | |
| process.env.ALBERTALERT_AUTO_QUARANTINE_TIMEOUT_THRESHOLD || 4 | |
| function parseEnvThreshold(value, defaultValue, minimumValue = 1) { | |
| if (value == null || value === '') return defaultValue; | |
| const parsed = Number(value); | |
| if (!Number.isFinite(parsed)) return defaultValue; | |
| return Math.max(minimumValue, parsed); | |
| } | |
| const AUTO_QUARANTINE_FAILURE_THRESHOLD = parseEnvThreshold( | |
| process.env.ALBERTALERT_AUTO_QUARANTINE_FAILURE_THRESHOLD, | |
| 6 | |
| ); | |
| const AUTO_QUARANTINE_TIMEOUT_THRESHOLD = parseEnvThreshold( | |
| process.env.ALBERTALERT_AUTO_QUARANTINE_TIMEOUT_THRESHOLD, | |
| 4 |
| <div class="meta" id="meta"> | ||
| <span class="pill">Generated: 2026-04-24T17:57:58.900Z</span> | ||
| <span class="pill">Low-health sources: 527</span> | ||
| <span class="pill">Generated: 2026-04-24T18:37:50.264Z</span> |
There was a problem hiding this comment.
The "Generated:" pill is a hard-coded timestamp. When the page is opened without JS (or before JS updates the meta block), this will quickly become misleading and causes noisy diffs whenever the file is regenerated. Consider using a neutral placeholder (e.g. "Generated: —"/"Generated: loading…") instead of a specific date.
| <span class="pill">Generated: 2026-04-24T18:37:50.264Z</span> | |
| <span class="pill">Generated: —</span> |
| : 'Data mode: live read-only (restore unavailable)'; | ||
| meta.innerHTML = [ | ||
| '<span class="pill">Generated: ' + escapeHtml(payload.generatedAt || '2026-04-24T17:57:58.900Z') + '</span>', | ||
| '<span class="pill">Generated: ' + escapeHtml(payload.generatedAt || '2026-04-24T18:37:50.264Z') + '</span>', |
There was a problem hiding this comment.
This JS fallback uses a hard-coded ISO timestamp when payload.generatedAt is missing. That can display a stale/incorrect date and also creates churn in diffs when regenerating the file. Consider falling back to a generic value (e.g. "unknown") or omitting the pill when generatedAt is absent.
| '<span class="pill">Generated: ' + escapeHtml(payload.generatedAt || '2026-04-24T18:37:50.264Z') + '</span>', | |
| '<span class="pill">Generated: ' + escapeHtml(payload.generatedAt || 'unknown') + '</span>', |
|
@copilot resolve the merge conflicts in this pull request |
…dation-issues # Conflicts: # data/albertalert.sqlite # data/quarantined-sources.json # data/source-remediation-sweep.json # data/top-20-source-remediation.json # live-alerts.json # source-quarantine.html Co-authored-by: potemkin666 <183807833+potemkin666@users.noreply.github.com>
CI was configured to attempt Playwright fallback without ensuring Chromium was present, so fallback could fail with “please run
npx playwright install”. The scheduler was also still giving HTML scrapers too much airtime relative to RSS/Atom/JSON sources.CI browser provisioning
npx playwright install --with-deps chromiumScheduler bias toward machine-readable sources
Test coverage