Skip to content

fix: remove RETRIEVAL_TIMEOUT_BUFFER_MS#266

Merged
SgtPooki merged 4 commits intomainfrom
remove-retrieval-deadlines-from-old-cron-style
Feb 16, 2026
Merged

fix: remove RETRIEVAL_TIMEOUT_BUFFER_MS#266
SgtPooki merged 4 commits intomainfrom
remove-retrieval-deadlines-from-old-cron-style

Conversation

@SgtPooki
Copy link
Copy Markdown
Collaborator

built on top of #263.

we no longer need the RETRIEVAL_TIMEOUT_BUFFER_MS with pgboss workers and retrieval's handling abort signals.

@FilOzzy FilOzzy added this to FOC Feb 11, 2026
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FOC Feb 11, 2026
@BigLep BigLep requested review from Copilot and silent-cipher and removed request for silent-cipher February 12, 2026 16:01
@BigLep BigLep moved this from 📌 Triage to 🔎 Awaiting review in FOC Feb 12, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes the now-unneeded RETRIEVAL_TIMEOUT_BUFFER_MS configuration path, relying on pg-boss job timeouts and end-to-end abort-signal propagation for retrieval cancellation.

Changes:

  • Removed RETRIEVAL_TIMEOUT_BUFFER_MS from config schema/loading and from documented environment variables.
  • Simplified retrieval scheduling/execution to stop computing “interval minus buffer” time budgets and removed the batch/deal-level timeout plumbing in retrieval flows.
  • Updated retrieval/job unit tests to reflect abort-signal-driven cancellation behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
docs/environment-variables.md Removes RETRIEVAL_TIMEOUT_BUFFER_MS docs and related constraints/notes.
apps/backend/src/config/app.config.ts Drops env var from Joi schema + config types/loading; adjusts retrieval interval validation.
apps/backend/.env.example Removes RETRIEVAL_TIMEOUT_BUFFER_MS from backend env template.
apps/backend/src/scheduler/scheduler.service.ts Removes per-run computed timeout and passes only AbortSignal into retrieval batch runs.
apps/backend/src/retrieval/retrieval.service.ts Removes timeout params/logic (including withTimeout) and relies on abort propagation.
apps/backend/src/retrieval/retrieval.service.spec.ts Updates tests away from timeout/deadline behavior toward abort-driven behavior.
apps/backend/src/jobs/jobs.service.ts Removes interval/buffer timeout calculation for retrieval jobs; relies on job-level abort timeout.
apps/backend/src/jobs/jobs.service.spec.ts Updates mocked retrieval API signature to match removal of timeout arg.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/backend/src/retrieval/retrieval.service.ts Outdated
Comment thread apps/backend/src/config/app.config.ts Outdated
Comment thread apps/backend/src/retrieval/retrieval.service.spec.ts
@BigLep BigLep requested a review from silent-cipher February 12, 2026 16:32
Copy link
Copy Markdown
Collaborator

@silent-cipher silent-cipher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@github-project-automation github-project-automation Bot moved this from 🔎 Awaiting review to ✔️ Approved by reviewer in FOC Feb 14, 2026
Base automatically changed from 258-we-need-to-set-dealretrieval-max-timeout to main February 16, 2026 12:58
@SgtPooki SgtPooki merged commit 4f650a8 into main Feb 16, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from ✔️ Approved by reviewer to 🎉 Done in FOC Feb 16, 2026
@SgtPooki SgtPooki deleted the remove-retrieval-deadlines-from-old-cron-style branch February 16, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

5 participants