Fix empty analysis fallback with lookback retry in pre-article-analysis pipeline#1562
Conversation
🏷️ Automatic Labeling SummaryThis PR has been automatically labeled based on the files changed and PR metadata. Applied Labels: size-xs Label Categories
For more information, see |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
…cle-analysis pipeline - Add subtractBusinessDays() and MAX_LOOKBACK_BUSINESS_DAYS to data-downloader.ts - Add lookback retry logic in pre-article-analysis.ts when date-filtered docs are 0 - Add dataFreshness field to SynthesisSummary and serialize it in synthesis output - Add isNonEmptyAnalysis() and readLatestNonEmptyAnalysis() to analysis-reader.ts - Parse dataFreshness from synthesis markdown in parseSynthesisSummary() - Add 19 new tests covering all new functionality - All 4020 tests pass, build:lib succeeds Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/7890551d-fe7d-453b-bc6a-2b1cc092100d Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
github-actions
Bot
added
testing
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
…dd comment - Simplify dataFreshness type from optional+null to required string|null - Fix isNonEmptyAnalysis JSDoc to accurately describe theme/narrative check - Add comment explaining requestedIdSet guard prevents lookback in deep-inspection mode Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/7890551d-fe7d-453b-bc6a-2b1cc092100d Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
|
@copilot fix failing checks |
|
@copilot fix failing checks |
…rals The dataFreshness field was made required (not optional) in the refactor commit, but three test synthesis objects in isNonEmptyAnalysis tests were missing the property, causing tsc --noEmit to fail in CI. Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/6a93378c-5a83-4668-8928-58f0ccc8b8f3 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
Fixed in
Already fixed in |
There was a problem hiding this comment.
Pull request overview
This PR improves resilience of the pre-article-analysis pipeline and downstream consumers by avoiding “empty analysis” outputs when a target date has no matching documents, and by annotating synthesis with data freshness when lookback is used.
Changes:
- Add business-day lookback support (
subtractBusinessDays,MAX_LOOKBACK_BUSINESS_DAYS) and apply it when date-filtered documents are zero. - Add
dataFreshnessmetadata to synthesis output and parsing to record when the analysis data date differs from the article date. - Add
isNonEmptyAnalysis+readLatestNonEmptyAnalysisto support fallback to the most recent non-empty daily analysis, with tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/analysis-reader.test.ts | Adds test coverage for business-day subtraction, dataFreshness parsing, non-empty analysis detection, and non-empty lookback reading. |
| scripts/pre-article-analysis/markdown-serializer.ts | Extends synthesis summary schema/serialization to include dataFreshness in “Data Quality Notes”. |
| scripts/pre-article-analysis/data-downloader.ts | Introduces business-day date arithmetic utility and lookback constant used by the pipeline. |
| scripts/pre-article-analysis.ts | Implements lookback retry when no documents match the requested date and propagates dataFreshness into synthesis. |
| scripts/analysis-reader.ts | Parses dataFreshness, adds “non-empty analysis” detection, and adds a lookback reader for non-empty daily analysis. |
| * | ||
| * @param dateStr - ISO date string (YYYY-MM-DD) | ||
| * @param days - Number of business days to subtract (must be >= 0) |
There was a problem hiding this comment.
The JSDoc says days must be >= 0, but the implementation clamps negative values to 0 (and tests rely on that). Update the JSDoc to reflect the actual behavior, or enforce the contract by throwing on negative input.
| * | |
| * @param dateStr - ISO date string (YYYY-MM-DD) | |
| * @param days - Number of business days to subtract (must be >= 0) | |
| * Fractional values are rounded down, and negative values are treated as 0. | |
| * | |
| * @param dateStr - ISO date string (YYYY-MM-DD) | |
| * @param days - Number of business days to subtract; fractional values are floored and values < 0 are clamped to 0 |
| export function subtractBusinessDays(dateStr: string, days: number): string { | ||
| const d = new Date(`${dateStr}T00:00:00Z`); | ||
| let remaining = Math.max(0, Math.floor(days)); |
There was a problem hiding this comment.
subtractBusinessDays() will throw a RangeError on malformed dateStr because new Date(...).toISOString() fails for Invalid Date. Since this is an exported utility, consider validating dateStr (e.g., strict YYYY-MM-DD) and either throw a clearer error or return a safe fallback.
| @@ -634,6 +674,9 @@ async function runPreArticleAnalysis(opts: { | |||
| console.log( | |||
| ` Selected ${allDocs.length} documents for analysis for ${date} (${excludedDocsCount} with missing or non-matching dates excluded)`, | |||
There was a problem hiding this comment.
excludedDocsCount can become negative when --document-ids causes additional documents to be fetched and appended to allDocs (making allDocs.length > flattenedDocs.length). This will produce misleading logs like "(-2 excluded)". Consider computing excluded counts before adding fetched-by-id docs, or clamp at 0 and log fetched-by-id count separately.
| for (let i = 1; i <= maxDaysBack; i++) { | ||
| const d = new Date(`${date}T00:00:00Z`); | ||
| d.setUTCDate(d.getUTCDate() - i); | ||
| const prevDate = d.toISOString().slice(0, 10); | ||
| const prev = await readDailyAnalysis(prevDate, basePath); |
There was a problem hiding this comment.
readLatestNonEmptyAnalysis() constructs a Date from the date string without validating format. If date is not a valid YYYY-MM-DD, toISOString() will throw (whereas readDailyAnalysis safely returns an empty result due to DATE_FORMAT_RE). Add the same DATE_FORMAT_RE guard here before doing Date arithmetic.
|
@copilot apply changes based on the comments in this thread |
…ount guard, date format guard Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/1197f077-0135-43c5-b5ab-8c1e899a0a77 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
… for granularity Agent-Logs-Url: https://github.com/Hack23/riksdagsmonitor/sessions/1197f077-0135-43c5-b5ab-8c1e899a0a77 Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Applied all 4 review comments in
All 91 analysis-reader tests pass, 4023+ total tests pass, 0 CodeQL alerts. |
🔍 Lighthouse Performance Audit
📥 Download full Lighthouse report Budget Compliance: Performance budgets enforced via |
Improves resilience of the pre-article-analysis pipeline and downstream consumers by avoiding "empty analysis" outputs when a target date has no matching documents, and by annotating synthesis with data freshness when lookback is used.
Changes Made
subtractBusinessDays()utility todata-downloader.tsfor lookback date calculation, with strict YYYY-MM-DD input validation (RangeErroron malformed dates)pre-article-analysis.tswhen date-filtered documents are 0, expanding to previous business days up toMAX_LOOKBACK_BUSINESS_DAYS(5)dataFreshnessmetadata to synthesis output indicating actual data date vs article datereadLatestNonEmptyAnalysis()inanalysis-reader.tswithDATE_FORMAT_REguard and fallback to previous day's analysisexcludedDocsCountwithMath.max(0, …)to prevent negative log output when by-ID fetches growallDocsbeyondflattenedDocsTesting
subtractBusinessDays()utility (including input validation and edge cases)isNonEmptyAnalysis(), andreadLatestNonEmptyAnalysis()(including date format guard)