feat(discovery): GitHub search backstop (only search if local was idle)#214
Merged
Conversation
UNIFY-PR-05 — the second half of the GH/local partition. Brave/Exa are paid (~1,000 free queries/month each), so the day's search budget should be spent once. GitHub should search only as a backstop: when a local run did not already search that calendar day. - `SearchBudgetLedger.searched_on(day=..., engine=...)`: True if a search (queries > 0) was recorded on the given UTC calendar day. Bucketed in UTC; a budget-skipped run that logged 0 queries does not count. - `DiscoveryConfig.search_backstop_only` (default False), set `true` in `agents/news/ci.overlay.yaml`. When True, `run_news_discover_job` checks the ledger before issuing engine queries and skips Brave/Exa/Google CSE for the day if a search was already recorded (by any run, local or CI), recording a `search_backstop_skipped=true` warning. Source-native discovery and the deterministic phases are unaffected; local runs search on their own cadence and take priority. Also fixes the post-run check `failed_runs == len(persisted_runs)` to require `persisted_runs` first, so a zero-run day (the backstop skipping all engines) finishes non-fatal instead of tripping the all-failed branch on `0 == 0`. This completes the code side of the GH/local partition. Re-enabling the workflows + seeding the state repo is the operational follow-up (UNIFY-PR-06); the parked scrape->classify decouple is tracked as issue #213. Tests: searched_on (same-day detection, zero-query exclusion, UTC bucketing); the CI-overlay config flip; and the discover job skipping search when already searched today vs running it when the ledger is empty. ruff/mypy clean; 1376 unit tests pass. Co-Authored-By: Claude Opus 4.8 <[email protected]>
shaypal5
added
configuration
discovery
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
==========================================
+ Coverage 92.81% 92.82% +0.01%
==========================================
Files 83 83
Lines 12283 12302 +19
==========================================
+ Hits 11400 11419 +19
Misses 883 883
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a “search backstop” mode so GitHub Actions only spends paid open-web search budget (Brave/Exa/Google CSE) on days when no other run has already searched (per the search-budget ledger), preserving budget for local runs and preventing duplicate daily spend.
Changes:
- Added
SearchBudgetLedger.searched_on(day=..., engine=...)to detect “real” searches (queries > 0) on a given UTC calendar day. - Introduced
DiscoveryConfig.search_backstop_only(defaultFalse) and enabled it in the CI overlay to skip engine discovery when the ledger shows an earlier search that day. - Adjusted
run_news_discover_jobfatality logic so “zero attempted runs” (e.g., all engines backstop-skipped) is non-fatal, and added unit tests/docs updates for the behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/denbust/discovery/search_budget.py |
Adds searched_on() using UTC-day bucketing and queries>0 semantics. |
src/denbust/config.py |
Adds discovery.search_backstop_only configuration flag. |
src/denbust/pipeline.py |
Implements backstop gating and fixes the 0 == 0 fatal condition for zero attempted runs. |
agents/news/ci.overlay.yaml |
Enables the backstop for CI runs. |
docs/operational_reference.md |
Documents CI overlay behavior and the backstop warning flag. |
tests/unit/test_search_budget.py |
Adds unit coverage for same-day detection, zero-query exclusion, and UTC bucketing. |
tests/unit/test_pipeline_core.py |
Adds behavioral tests for backstop skip vs. run behavior. |
tests/unit/test_config.py |
Asserts local vs CI overlay differences include the backstop flag. |
.agent-plan.md |
Updates planning/mainline status to reflect UNIFY-PR-05 being merged and sets next steps. |
Comment on lines
+2820
to
+2836
| from datetime import UTC, datetime | ||
|
|
||
| from denbust.discovery.search_budget import SearchBudgetLedger | ||
|
|
||
| config = Config( | ||
| store={"state_root": tmp_path}, | ||
| source_discovery={"enabled": False}, | ||
| discovery={ | ||
| "enabled": True, | ||
| "engines": {"brave": {"enabled": True}}, | ||
| "search_backstop_only": True, | ||
| }, | ||
| ) | ||
| # A local run already searched earlier today. | ||
| SearchBudgetLedger(config.discovery_state_paths.search_budget_path).record( | ||
| engine="brave", queries=20, run_id="local-run", now=datetime.now(UTC) | ||
| ) |
This comment has been minimized.
This comment has been minimized.
Addresses review of #214: 1. Rolling 24h window instead of a UTC calendar day. The calendar-day check only deferred to local when local happened to run first that day; if GH's schedule fired before local, GH searched every day and the backstop saved nothing (local has no backstop and searches anyway -> two searches/day). It also double-searched across the UTC-midnight boundary. The ledger method is now `searched_since(since=...)`; the discover job gates on `run_timestamp - 24h`. With a rolling window GH defers to a recent local search regardless of clock ordering: as long as local runs at least daily, GH always skips and only searches once local has been idle longer than a day. Docs + overlay comment corrected to this honest framing, with a note to schedule GH discover at least daily. 2. Discover test now uses the real CI triple (source_discovery on + scraping_enabled off + search_backstop_only on): the backstop skips the engines while source-native discovery runs empty-but-SUCCEEDED, so the zero-engine day stays non-fatal — locking in the `persisted_runs and failed_runs == len(...)` guard against future source-native run-status changes. (The previous test used source_discovery off, which the real CI config never does.) 3. `searched_since` treats a tz-naive `recorded_at` as UTC rather than silently converting from the local machine tz. ruff/mypy clean; 1376 unit tests pass. Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
pr-agent-context report: This run includes an unresolved review comment on PR #214.
For each unresolved review comment, recommend one of: resolve as irrelevant, accept and implement
the recommended solution, open a separate issue and resolve as out-of-scope for this PR, accept and
implement a different solution, or resolve as already treated by the code.
After I reply with my decision per item, implement the accepted actions, resolve the corresponding
PR comments, and push all of these changes in a single commit.
# Copilot Comments
## COPILOT-1
Location: tests/unit/test_pipeline_core.py
URL: https://github.com/DataHackIL/tfht_enforce_idx/pull/214#discussion_r3407594076
Status: outdated
Root author: copilot-pull-request-reviewer
Comment:
This test can be flaky around UTC midnight: the ledger record uses `datetime.now(UTC)` while `run_news_discover_job()` independently sets `result.run_timestamp` via `datetime.now(UTC)`. If the day flips between the two calls, `searched_on(day=run_day)` won't see the record and the test may fail intermittently. Freeze the pipeline clock (or patch `_build_run_snapshot`) so both use the same fixed timestamp.Run metadata: |
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
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
UNIFY-PR-05— the second half of the GH/local partition, completing its code side. Brave/Exa are paid (~1,000 free queries/month each), so the day's search budget should be spent once. GitHub should search only as a backstop: when a local run did not already search that calendar day.Change
SearchBudgetLedger.searched_on(day=..., engine=...)— True if a search (queries > 0) was recorded on the given UTC calendar day. A budget-skipped run that logged 0 queries doesn't count.DiscoveryConfig.search_backstop_only(defaultFalse), settrueinagents/news/ci.overlay.yaml. When True,run_news_discover_jobchecks the ledger before issuing engine queries and skips Brave/Exa/Google CSE for the day if a search was already recorded (by any run, local or CI), recording asearch_backstop_skipped=truewarning. Source-native discovery and the deterministic phases are unaffected.Local runs search on their own cadence and take priority; GH searches only on a day local was idle, then records its spend so a later GH run the same day also skips.
A fix the backstop surfaced
The post-run check
failed_runs == len(persisted_runs)fataled on0 == 0when the backstop skipped all engines (a zero-run day). Guarded it withpersisted_runs and …, so "nothing ran" is non-fatal while "all attempted runs failed" still is.Tests
searched_on: same-day detection, zero-query exclusion, UTC bucketing (a23:30-05:00record counts as the next UTC day).localFalse →ciTrue).ruff/mypy clean; 1376 unit tests pass.
Scope / sequencing
UNIFY-PR-04= no-scrape guardrail; this = search-backstop).UNIFY-PR-06); confirmSTATE_REPO_PATpush/force-push rights first.backfill_discover(a manual operator recovery job) is intentionally not backstop-gated.🤖 Generated with Claude Code