Skip to content

Phase 2 — Reliability, observability & evaluation: crash recovery, backpressure, metrics, evals, adversarial suite#4

Merged
Aakash1337 merged 4 commits into
mainfrom
claude/relay-phase-2
Jul 5, 2026
Merged

Phase 2 — Reliability, observability & evaluation: crash recovery, backpressure, metrics, evals, adversarial suite#4
Aakash1337 merged 4 commits into
mainfrom
claude/relay-phase-2

Conversation

@Aakash1337

@Aakash1337 Aakash1337 commented Jul 4, 2026

Copy link
Copy Markdown
Owner

What this delivers

Phase 2 of the roadmap, complete on one branch: the proven pipeline becomes trustworthy unattended, and measurable enough to change safely.

Resumability & crash recovery

  • Transient compute failures park leads in error_retryable (the failing step's transaction has already rolled back — no partial work exists to duplicate); a later run resumes to the recorded return state, with the DB trigger counting retries and enforcing the cap. Model refusals park terminally for a human.
  • A recovery pass runs at the start of every worker tick: orphaned running runs get closed; jobs orphaned mid-sending fail safe — outcome unknown means never retried and never assumed sent, lead parked for human attention. Idempotent by construction.

Rate limiting, backpressure, bounded retries

  • Token buckets per external target (compute tiers, CRM). Waits beyond the cap raise Backpressure visibly — work parks instead of queueing unboundedly.
  • Bounded exponential retry for transient failures only: one blip is absorbed in-call; a refusal or invalid output is never re-rolled; a failing provider is never silently swapped.

Observability

  • /metrics (JSON) + /metrics/prometheus — every number derived on read from rows the pipeline already writes; no second metering pipeline to drift.
  • /ops — self-contained dashboard page (cost, error rate, reply rate, queue depths, active alerts).
  • Alert rules as dumb thresholds (they must survive the intelligent parts breaking): spend spike/hour, failure streak, stuck queue — structured log always, webhook sink optional.

Evaluation harness

  • Golden-set invariants through the real prompt scaffolding: opt-outs must triage unsubscribed; declines never become interest; injections cannot raise scores or flip triage toward more contact; copy respects length/leak bounds; JSON contracts hold.
  • just evals scores the configured backends (Gemini/Gemma today) with non-zero exit on threshold failure — a pre-deploy gate for model/prompt changes.
  • Exit gate proven: three injected regressions (sycophantic scoring, manufactured intent, broken contract) are each caught and localized by category.

Adversarial/correctness suite (the roadmap's full list)

  • Duplicate-send chaos: 4 workers race 3 queued jobs → exactly 3 sends, one sent transition per lead.
  • Transactional-outbox atomicity under an idempotency race.
  • Suppression bypass closed on every path — worker re-check, code gate, raw-SQL re-queue (and suppression rows are UPDATE-proof for the app role).
  • Replayed reply webhooks cannot double-transition or double-suppress.
  • CRM conflict: the canonical datastore wins deterministically.
  • Cross-tenant erasure attempt touches nothing, leaks nothing.
  • PII sweep: the raw address never appears in logs across a full journey + erasure.
  • Backup/restore: pg_dump → fresh DB → the DSR erasure survives the restore, and the hashed suppression entry survives with it.

Unattended-run proof + §8 decision

  • A simulated spine schedule converges a full synthetic cohort with exactly one human act (the gate); two further complete ticks change nothing (idempotent schedule).
  • docs/decisions/local-tool-calling.md: the local tier stays structurally tool-free (router rule + backends that never send tool schemas), with concrete Phase 3 revisit criteria. Pinned by test across every task type.

Verification

  • 242 tests passing against real Postgres (37 new across resumability, ratelimit, observability, evals, adversarial, unattended).
  • ruff check + ruff format --check clean.
  • Also fixed en route: a developer .env tuned for a real provider's free tier was throttling the hermetic suite through the new rate limiter — conftest now pins limits off for tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PeKoLBxk4FvswdScqS2upd


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added an operations dashboard with live metrics, alerts, and a Prometheus-formatted view.
    • Added evaluation commands and reporting for reasoning-quality checks.
    • Introduced crash recovery and unattended-run support to keep jobs moving.
  • Bug Fixes

    • Improved handling of transient failures with retry and backpressure.
    • Added alerting for spend spikes, stuck queues, and repeated failures.
  • Documentation

    • Updated the README and environment example with Phase 2 settings and guidance.

claude added 4 commits July 4, 2026 21:50
…ery on every tick

- Runner: ComputeUnavailable parks the lead in error_retryable (return
  state recorded; failed step's transaction already rolled back, so a
  crashed step leaves no partial work); ComputeRefused parks terminally
  for human attention. New resume handler walks error_retryable back to
  its recorded state; the DB trigger counts retries and enforces the cap,
  the handler parks terminally when the cap is spent.
- Recovery pass (pipeline/recovery.py): closes pipeline_runs orphaned in
  'running' and fails send jobs orphaned in 'sending' — outcome unknown,
  so never retried and never assumed sent; lead parked error_terminal.
  Tenant discovery via fn_tenants_with_stale_work (SECURITY DEFINER),
  processing under each tenant's RLS context. Idempotent; runs at the
  start of every worker tick, so the system self-heals on its normal
  schedule.
- RELAY_RECOVERY_STALE_AFTER_SECONDS (default 300).
- 8 exit-gate tests: park→resume with exactly one draft (no lost or
  duplicated work), retry-cap exhaustion, refusal is terminal, orphaned
  run closed (idempotent), fresh runs untouched, orphaned mid-send fails
  safe, recovery wired into the tick.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PeKoLBxk4FvswdScqS2upd
… calls

- ratelimit.py: thread-safe token buckets (injectable clock/sleeper) with
  visible backpressure — a wait beyond RELAY_RATE_LIMIT_MAX_WAIT_SECONDS
  raises instead of queueing unboundedly, and the runner's park path
  turns that into error_retryable
- with_backoff: bounded exponential retry for TRANSIENT failures only;
  refusals and invalid outputs are never re-rolled, and there is no
  tier/provider fallback anywhere — a failing provider fails loudly
- executors.execute(): bucket per compute tier + bounded backoff around
  backend.complete; billing still happens before the call
- CRM mirror sync draws from its own bucket
- Per-target RPS config (0 = off, the default); buckets are process-local
  (documented; distributed limiting is a Phase 3 concern)
- Retry semantics proven in the resumability suite: a single blip is
  absorbed in-call (no state-machine involvement), a sustained outage
  still parks and resumes with zero duplicated work
- 8 limiter/backoff tests with fake clocks (214 total)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PeKoLBxk4FvswdScqS2upd
Observability:
- tenant metrics derived on read from rows the pipeline already writes
  (lead states, run outcomes + cost in a 24h window, queue depths, sent/
  reply rates, suppression count); JSON at /metrics, Prometheus text at
  /metrics/prometheus
- alert evaluator with dumb-threshold rules (they must work when the
  intelligent parts are broken): spend spike per hour, failure streak,
  stuck queue; structured-log always + best-effort webhook sink
- /ops: self-contained dashboard page (no external assets), cards for
  cost/error-rate/reply-rate/queues + active alerts, optional 30s auto-
  refresh

Eval harness (relay.evals):
- golden-set invariants over any compute backend through the REAL prompt
  scaffolding: opt-outs must triage unsubscribed, declines never become
  interest, injections cannot raise scores or flip triage toward more
  contact, copy respects length/leak bounds, JSON contracts hold
- thresholded EvalReport with per-category localization
- exit gate proven: three injected regressions (sycophantic scoring,
  manufactured intent, broken contract) are each caught

Adversarial/correctness suite (roadmap list):
- duplicate-send chaos: 4 workers race 3 queued jobs → exactly 3 sends
- transactional-outbox atomicity under an idempotency race
- suppression bypass: worker re-check, code gate, and raw-SQL re-queue
  all closed (and suppression rows are UPDATE-proof for the app role)
- replayed reply webhooks cannot double-transition or double-suppress
- CRM conflict: canonical datastore wins deterministically
- cross-tenant erasure attempt touches nothing and leaks nothing
- PII sweep: raw address absent from all logs across a full journey +
  erasure
- backup/restore (pg_dump→fresh DB): erasure survives restore, the
  hashed suppression entry survives with it

Also: conftest now pins rate limits OFF for the suite — a developer .env
tuned for a real provider's free tier was throttling hermetic tests with
real wall-clock waits (25s → 2s for test_dry_run).

240 tests passing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PeKoLBxk4FvswdScqS2upd
…al CLI, docs

- tests/test_unattended.py: a simulated spine schedule converges a
  12-prospect cohort (all personas) to terminal states with exactly one
  human act (the gate), and two further full ticks change nothing —
  the idempotent-schedule exit gate. Plus the routing pin: tasks that
  require tools route hosted for every task type.
- docs/decisions/local-tool-calling.md: §8 open item resolved — the
  local tier stays structurally tool-free (router rule + backends that
  never send tool schemas), with concrete Phase 3 revisit criteria.
- scripts/run_evals.py + 'just evals': score the CONFIGURED backends
  against the golden set (real quota; non-zero exit on threshold
  failure — wire into pre-deploy when models/prompts change).
- README Phase 2 section; alert thresholds documented in .env.example.

242 tests passing; ruff clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PeKoLBxk4FvswdScqS2upd
@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds Phase 2 reliability, observability, and evaluation capabilities: process-local rate limiting with bounded retries, crash recovery for orphaned pipeline runs/send jobs, tenant metrics/alerting with an ops dashboard and API endpoints, a golden-set reasoning evaluation harness, and extensive adversarial/resumability/unattended-run test suites plus documentation.

Changes

Phase 2 Reliability, Observability & Evaluation

Layer / File(s) Summary
Configuration and env docs
src/relay/config.py, .env.example
New Settings fields for rate limits, retries, alert thresholds, and crash-recovery staleness are added and documented.
Rate limiting and backoff
src/relay/ratelimit.py
TokenBucket, Backpressure, bucket registry, limit_compute/limit_crm, and with_backoff retry helper are introduced.
Wiring into executors and CRM sync
src/relay/routing/executors.py, src/relay/crm/sync.py, tests/conftest.py
Backend calls now apply rate limiting and bounded retries; CRM sync applies backpressure; test defaults disable throttling.
Stale-work SQL and permissions
src/relay/db/sql/002_functions.sql, src/relay/db/sql/004_rls.sql
Adds fn_tenants_with_stale_work function with restricted execute grant.
Crash recovery
src/relay/pipeline/recovery.py, src/relay/workers/send_worker.py
recover_orphans closes stale pipeline runs and mid-send jobs; wired into worker tick.
Runner park/resume on failures
src/relay/pipeline/runner.py
Runner parks leads into retryable/terminal error states on compute failures and resumes via a new step handler.
Observability metrics/alerts core
src/relay/observability/*.py
TenantMetrics, Prometheus rendering, and evaluate_alerts with webhook delivery are added.
Observability API and ops dashboard
src/relay/api/schemas.py, src/relay/api/routes.py, src/relay/api/ops_ui.py
New /metrics, /metrics/prometheus, /alerts, /ops endpoints and self-contained dashboard page.
Observability tests
tests/test_observability.py
Validates metrics, Prometheus text, alert scenarios, webhook payloads, and endpoint auth.
Rate limiting tests
tests/test_ratelimit.py
Covers bucket burst/refill/backpressure and backoff retry/bound/refusal behavior.
Resumability tests
tests/test_resumability.py
Covers retry, parking, terminal refusal, and orphan recovery idempotency.
Eval harness
src/relay/evals/*.py, scripts/run_evals.py, justfile
Golden-case reasoning eval harness with CLI entrypoint and just recipe.
Eval regression tests
tests/test_evals.py
Verifies injected scoring/triage/contract regressions are caught and localized.
Adversarial tests
tests/test_adversarial.py
Covers concurrency, idempotency, suppression, replay, CRM conflicts, tenant isolation, and backup/restore.
Unattended-run tests and docs
tests/test_unattended.py, docs/decisions/local-tool-calling.md, README.md
Verifies convergence/idempotency and tool-routing, with supporting decision record and README updates.

Estimated code review effort: 4 (Complex) | ~75 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Executor
  participant RateLimiter
  participant Backend
  participant Recovery
  Executor->>RateLimiter: limit_compute(tier)
  RateLimiter-->>Executor: token acquired
  Executor->>Backend: with_backoff(backend.complete)
  Backend-->>Executor: ComputeUnavailable
  Executor->>Backend: retry with exponential delay
  Note over Recovery: on worker tick
  Recovery->>Backend: recover_orphans() closes stale runs/jobs
Loading
sequenceDiagram
  participant Browser
  participant OpsUI
  participant API
  participant Observability
  Browser->>OpsUI: load /ops
  OpsUI->>API: GET /metrics, GET /alerts
  API->>Observability: tenant_metrics(), evaluate_alerts()
  Observability-->>API: MetricsResponse, AlertsResponse
  API-->>OpsUI: JSON payloads
  OpsUI-->>Browser: render dashboard cards
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the Phase 2 changes: crash recovery, backpressure, observability, evals, and adversarial testing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/relay-phase-2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Aakash1337 Aakash1337 marked this pull request as ready for review July 5, 2026 00:21

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91e8c8ee32

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

Comment thread src/relay/routing/executors.py
Comment thread src/relay/routing/executors.py
@Aakash1337 Aakash1337 merged commit 2fc522b into main Jul 5, 2026
2 of 3 checks passed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (6)
src/relay/config.py (1)

103-103: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider validating/normalizing alert_webhook_url.

A bare str field silently accepts whitespace-only or malformed values. Adding a field_validator that strips the value (treating whitespace-only as empty) would provide defense-in-depth against exactly the kind of dotenv inline-comment parsing pitfall flagged in .env.example (see comment there) — the "empty = log only" contract would hold even if the underlying env value ends up containing stray whitespace/comment text.

♻️ Example validator
+    `@field_validator`("alert_webhook_url")
+    `@classmethod`
+    def _strip_webhook_url(cls, v: str) -> str:
+        return v.strip()
     alert_webhook_url: str = ""
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/config.py` at line 103, The alert_webhook_url config field
currently accepts raw strings without normalization, so whitespace-only or
malformed env values can slip through. Add a field_validator on
alert_webhook_url in the config model to strip incoming values and convert
whitespace-only input to an empty string, keeping the empty means log-only
behavior intact. Use the existing config class and the alert_webhook_url field
as the entry points so the validation is applied whenever settings are loaded.
scripts/run_evals.py (1)

24-29: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Validate the CLI argument before the dict lookup.

An invalid which value (typo, wrong case) raises an unhandled KeyError traceback instead of a usage message.

🛠️ Proposed fix
     which = sys.argv[1] if len(sys.argv) > 1 else "both"
-    tiers = {
+    valid = {
         "local": [ComputeTier.LOCAL],
         "hosted": [ComputeTier.HOSTED],
         "both": [ComputeTier.LOCAL, ComputeTier.HOSTED],
-    }[which]
+    }
+    if which not in valid:
+        print(f"Usage: run_evals.py [local|hosted|both]", file=sys.stderr)
+        sys.exit(2)
+    tiers = valid[which]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/run_evals.py` around lines 24 - 29, The CLI parsing in run_evals.py
looks up tiers by the raw which argument, which can throw an unhandled KeyError
for invalid values. Validate which before indexing the dict in the
argument-handling logic, and if it is not one of the supported options in tiers,
emit a clear usage/error message and exit cleanly instead of crashing.
src/relay/evals/harness.py (1)

232-257: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoff

Consider retry/backoff for real-provider eval runs.

run_evals calls backend.complete() directly with no with_backoff/rate-limit wrapper, unlike executors which wrap the same call (per the "Wire rate limiting into executors" layer). Since scripts/run_evals.py is documented to hit real hosted providers and is meant to gate CI/pre-deploy checks, a single transient ComputeUnavailable here becomes a hard case failure and can fail the gate for reasons unrelated to reasoning quality.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/evals/harness.py` around lines 232 - 257, run_evals currently calls
backend.complete() directly, so transient provider/rate-limit failures can fail
evals as hard case errors. Update run_evals in harness.py to invoke the
completion path through the same retry/backoff wrapper used by executors (the
with_backoff/rate-limit flow) before require_fields and case.check. Keep the
existing ComputeError handling, but ensure backend.complete is retried for
real-provider runs so one ComputeUnavailable does not immediately fail the eval
gate.
src/relay/api/routes.py (1)

529-559: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Local imports inside route handlers.

PlainTextResponse and OPS_PAGE are imported inline inside metrics_prometheus/ops_page, while HTMLResponse is presumably imported at module level elsewhere. Since ops_ui.py has no dependency back on routes.py, there's no circular-import concern — consider hoisting these to top-level imports for consistency.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/api/routes.py` around lines 529 - 559, The route handlers
metrics_prometheus and ops_page use local imports for PlainTextResponse and
OPS_PAGE even though there is no circular dependency here. Hoist those imports
to the module level alongside the other route dependencies in routes.py, and
keep the route functions focused on building and returning the response objects
for consistency with HTMLResponse and the rest of the module.
src/relay/observability/alerts.py (1)

105-115: 🧹 Nitpick | 🔵 Trivial

No de-duplication/debounce for repeated alert firings.

evaluate_alerts fires and re-posts the webhook every time thresholds remain breached (e.g. a persistent queue_stuck condition across successive worker-tick evaluations), with no cooldown/dedup state. This is likely fine for an initial threshold-alerting pass, but worth calling out for anyone wiring this into a real schedule, since it can spam the configured webhook.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/observability/alerts.py` around lines 105 - 115, The
evaluate_alerts flow currently reposts the webhook on every tick while alerts
stay active, so add a simple dedup/cooldown mechanism around the alert dispatch
in evaluate_alerts before calling _post_webhook. Use the existing alert
identities (for example alert.rule, alert.severity, and tenant_id) to track
recently sent alerts, and skip or debounce repeated sends within a configurable
window while still logging alerts normally.
.env.example (1)

81-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reorder keys per dotenv-linter.

Static analysis flags RELAY_RATE_LIMIT_HOSTED_RPS, RELAY_RATE_LIMIT_CRM_RPS, RELAY_COMPUTE_RETRY_ATTEMPTS/_BASE_SECONDS, and the alert keys as out of alphabetical order. Low effort, keeps lint clean if this runs in CI.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.env.example around lines 81 - 95, Reorder the environment keys in the
.env.example block to satisfy dotenv-linter’s alphabetical ordering. Keep the
existing symbols grouped logically, but adjust the sequence of
RELAY_RATE_LIMIT_LOCAL_RPS, RELAY_RATE_LIMIT_HOSTED_RPS,
RELAY_RATE_LIMIT_CRM_RPS, RELAY_COMPUTE_RETRY_ATTEMPTS,
RELAY_COMPUTE_RETRY_BASE_SECONDS, and the RELAY_ALERT_* entries so they appear
in strict lexicographic order within this section.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.env.example:
- Line 96: The RELAY_ALERT_WEBHOOK_URL entry in the example env file is being
parsed incorrectly when left unquoted with an inline comment, so update the
.env.example value to use an explicit quoted empty string for the webhook
setting. Keep the intent of the RELAY_ALERT_WEBHOOK_URL config clear so
pydantic-settings/python-dotenv treats it as truly empty and the “empty = log
only” behavior remains intact.

In `@src/relay/api/routes.py`:
- Around line 529-535: The metrics_prometheus route is returning a
PlainTextResponse, which leaves the Prometheus exposition Content-Type too
generic. Update metrics_prometheus to set the explicit Prometheus content type
for the text_format output (including the version=0.0.4 parameter) instead of
relying on PlainTextResponse defaults, so scrapers can identify the format
correctly.

In `@src/relay/pipeline/recovery.py`:
- Around line 87-125: The stale recovery queries in _recover_tenant can be
picked up by overlapping process_pending() executions and double-record
audit/stats for the same rows. Update the PipelineRun and SendJob lookups inside
tenant_session to lock eligible rows with FOR UPDATE SKIP LOCKED (or the
SQLAlchemy equivalent on the select used with session.execute), so parallel
recovery ticks only process each stale record once.

In `@src/relay/routing/executors.py`:
- Around line 67-78: Move the rate-limit and retry logic out of the tenant
transaction in PipelineRunner._advance_once so tenant_session() is not held open
during limit_compute() and with_backoff() sleeps. Split the DB-read/decision
work from the backend.complete(request) call, or perform the backoff and compute
wait path before opening tenant_session(), keeping the transaction limited to
the minimal database work.
- Around line 67-78: `_advance_once()` is not handling
`relay.ratelimit.Backpressure`, so rate-limit waits escape the retryable compute
path and can also hold `tenant_session()` open while sleeping. Update the flow
around `limit_compute()` and `with_backoff()` in `_advance_once()` to treat
`Backpressure` like the other retryable compute failures, convert it into the
retryable/error-retryable path instead of letting it reach `run()`’s generic
handler, and ensure any backoff sleep happens outside the open database
transaction.

---

Nitpick comments:
In @.env.example:
- Around line 81-95: Reorder the environment keys in the .env.example block to
satisfy dotenv-linter’s alphabetical ordering. Keep the existing symbols grouped
logically, but adjust the sequence of RELAY_RATE_LIMIT_LOCAL_RPS,
RELAY_RATE_LIMIT_HOSTED_RPS, RELAY_RATE_LIMIT_CRM_RPS,
RELAY_COMPUTE_RETRY_ATTEMPTS, RELAY_COMPUTE_RETRY_BASE_SECONDS, and the
RELAY_ALERT_* entries so they appear in strict lexicographic order within this
section.

In `@scripts/run_evals.py`:
- Around line 24-29: The CLI parsing in run_evals.py looks up tiers by the raw
which argument, which can throw an unhandled KeyError for invalid values.
Validate which before indexing the dict in the argument-handling logic, and if
it is not one of the supported options in tiers, emit a clear usage/error
message and exit cleanly instead of crashing.

In `@src/relay/api/routes.py`:
- Around line 529-559: The route handlers metrics_prometheus and ops_page use
local imports for PlainTextResponse and OPS_PAGE even though there is no
circular dependency here. Hoist those imports to the module level alongside the
other route dependencies in routes.py, and keep the route functions focused on
building and returning the response objects for consistency with HTMLResponse
and the rest of the module.

In `@src/relay/config.py`:
- Line 103: The alert_webhook_url config field currently accepts raw strings
without normalization, so whitespace-only or malformed env values can slip
through. Add a field_validator on alert_webhook_url in the config model to strip
incoming values and convert whitespace-only input to an empty string, keeping
the empty means log-only behavior intact. Use the existing config class and the
alert_webhook_url field as the entry points so the validation is applied
whenever settings are loaded.

In `@src/relay/evals/harness.py`:
- Around line 232-257: run_evals currently calls backend.complete() directly, so
transient provider/rate-limit failures can fail evals as hard case errors.
Update run_evals in harness.py to invoke the completion path through the same
retry/backoff wrapper used by executors (the with_backoff/rate-limit flow)
before require_fields and case.check. Keep the existing ComputeError handling,
but ensure backend.complete is retried for real-provider runs so one
ComputeUnavailable does not immediately fail the eval gate.

In `@src/relay/observability/alerts.py`:
- Around line 105-115: The evaluate_alerts flow currently reposts the webhook on
every tick while alerts stay active, so add a simple dedup/cooldown mechanism
around the alert dispatch in evaluate_alerts before calling _post_webhook. Use
the existing alert identities (for example alert.rule, alert.severity, and
tenant_id) to track recently sent alerts, and skip or debounce repeated sends
within a configurable window while still logging alerts normally.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 310727e1-8b4d-433c-a6f0-526a655aa499

📥 Commits

Reviewing files that changed from the base of the PR and between 3c1fda8 and 91e8c8e.

📒 Files selected for processing (29)
  • .env.example
  • README.md
  • docs/decisions/local-tool-calling.md
  • justfile
  • scripts/run_evals.py
  • src/relay/api/ops_ui.py
  • src/relay/api/routes.py
  • src/relay/api/schemas.py
  • src/relay/config.py
  • src/relay/crm/sync.py
  • src/relay/db/sql/002_functions.sql
  • src/relay/db/sql/004_rls.sql
  • src/relay/evals/__init__.py
  • src/relay/evals/harness.py
  • src/relay/observability/__init__.py
  • src/relay/observability/alerts.py
  • src/relay/observability/metrics.py
  • src/relay/pipeline/recovery.py
  • src/relay/pipeline/runner.py
  • src/relay/ratelimit.py
  • src/relay/routing/executors.py
  • src/relay/workers/send_worker.py
  • tests/conftest.py
  • tests/test_adversarial.py
  • tests/test_evals.py
  • tests/test_observability.py
  • tests/test_ratelimit.py
  • tests/test_resumability.py
  • tests/test_unattended.py

Comment thread .env.example
RELAY_ALERT_SPEND_UNITS_PER_HOUR=100
RELAY_ALERT_FAILURE_STREAK=3
RELAY_ALERT_QUEUE_STALE_SECONDS=600
RELAY_ALERT_WEBHOOK_URL= # Slack/n8n hook; empty = log only

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm which dotenv loader is used and its version
rg -n 'python-dotenv|env_file|load_dotenv' pyproject.toml src/relay/config.py 2>/dev/null

Repository: Aakash1337/Relay

Length of output: 260


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== pyproject.toml =='
rg -n 'pydantic-settings|python-dotenv|dotenv|env_file|load_dotenv' pyproject.toml src/relay/config.py

echo
echo '== relevant dependency block =='
python3 - <<'PY'
from pathlib import Path
text = Path('pyproject.toml').read_text()
for key in ['pydantic-settings', 'python-dotenv', 'dotenv']:
    if key in text:
        print(f'FOUND {key}')
PY

echo
echo '== src/relay/config.py excerpt =='
sed -n '1,120p' src/relay/config.py

Repository: Aakash1337/Relay

Length of output: 6245


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== dependency files mentioning python-dotenv / dotenv =='
rg -n 'python-dotenv|dotenv' pyproject.toml poetry.lock requirements*.txt uv.lock pdm.lock poetry.lock 2>/dev/null || true

echo
echo '== .env.example surrounding lines =='
nl -ba .env.example | sed -n '88,102p'

echo
echo '== pydantic-settings dot-env behavior references in repo =='
rg -n 'env_file|dotenv|python-dotenv' src . 2>/dev/null || true

Repository: Aakash1337/Relay

Length of output: 1026


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== .env.example lines 92-100 =='
sed -n '92,100p' .env.example | cat -n

echo
echo '== python-dotenv version pins in uv.lock =='
python3 - <<'PY'
from pathlib import Path
text = Path('uv.lock').read_text().splitlines()
for i, line in enumerate(text, 1):
    if 'name = "python-dotenv"' in line or 'python_dotenv' in line:
        start = max(1, i-2)
        end = min(len(text), i+8)
        for j in range(start, end+1):
            print(f"{j}:{text[j-1]}")
        print('---')
PY

Repository: Aakash1337/Relay

Length of output: 4365


🌐 Web query:

python-dotenv empty value inline comment parses comment as value bug 1.2.2

💡 Result:

In version 1.2.2 of python-dotenv, there is a known parsing behavior where an empty unquoted value followed by an inline comment is incorrectly parsed as including the comment text as part of the value [1]. For example, the line KEY= # comment is parsed as {'KEY': '# comment'} rather than {'KEY': ''} [2][1]. This issue arises because the parser's equal sign regex (=[^\S\r\n]*) consumes the whitespace immediately following the equals sign [3][2]. Consequently, the subsequent comment parsing logic—which relies on finding whitespace before the # character—fails to detect the comment, causing it to be treated as part of the unquoted value instead [3][2][1]. This behavior is tracked as an issue (Issue #600) and was addressed in subsequent development through pull requests that adjust the parser logic to correctly identify and strip inline comments even when the preceding value is empty [4][3][2]. Users encountering this can either avoid inline comments on lines with empty values or use quoted empty values (e.g., KEY="") as a workaround, which are parsed correctly [3].

Citations:


Quote the empty webhook value

RELAY_ALERT_WEBHOOK_URL= is loaded through pydantic-settings with python-dotenv 1.2.2, where an unquoted empty value plus an inline comment can be parsed as the comment text. That can make alert_webhook_url non-empty and break the “empty = log only” fallback.

🛠️ Proposed fix
-RELAY_ALERT_WEBHOOK_URL=                     # Slack/n8n hook; empty = log only
+RELAY_ALERT_WEBHOOK_URL=""                    # Slack/n8n hook; empty = log only
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RELAY_ALERT_WEBHOOK_URL= # Slack/n8n hook; empty = log only
RELAY_ALERT_WEBHOOK_URL="" # Slack/n8n hook; empty = log only
🧰 Tools
🪛 dotenv-linter (4.0.0)

[warning] 96-96: [ValueWithoutQuotes] This value needs to be surrounded in quotes

(ValueWithoutQuotes)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.env.example at line 96, The RELAY_ALERT_WEBHOOK_URL entry in the example
env file is being parsed incorrectly when left unquoted with an inline comment,
so update the .env.example value to use an explicit quoted empty string for the
webhook setting. Keep the intent of the RELAY_ALERT_WEBHOOK_URL config clear so
pydantic-settings/python-dotenv treats it as truly empty and the “empty = log
only” behavior remains intact.

Source: Linters/SAST tools

Comment thread src/relay/api/routes.py
Comment on lines +529 to +535
@router.get("/metrics/prometheus", include_in_schema=False)
def metrics_prometheus(
tenant_id: uuid.UUID = Depends(require_tenant),
):
from fastapi.responses import PlainTextResponse

return PlainTextResponse(prometheus_text(tenant_metrics(tenant_id)))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the route and nearby tests/usages
git ls-files src/relay/api/routes.py
wc -l src/relay/api/routes.py
sed -n '500,560p' src/relay/api/routes.py

printf '\n--- search for metrics route tests/usages ---\n'
rg -n "metrics/prometheus|prometheus_text|tenant_metrics|PlainTextResponse|content-type|Content-Type" src tests . -g '!**/.git/**'

Repository: Aakash1337/Relay

Length of output: 5317


🌐 Web query:

Prometheus 3.0 scrape target Content-Type text/plain version=0.0.4 charset=utf-8 required documentation

💡 Result:

In Prometheus 3.0, providing an accurate Content-Type header for scrape targets is mandatory; if the header is missing, unparsable, or unrecognized, the scrape will fail [1][2]. For the Prometheus text format version 0.0.4, the official MIME type and parameters are defined as: Content-Type: text/plain; version=0.0.4 [3][4] While some internal implementations or client libraries may include charset=utf-8 (e.g., text/plain; version=0.0.4; charset=utf-8) [5], Prometheus is strict regarding the Content-Type parsing in version 3.0 [6]. If your scrape target is currently returning a header that causes a failure—such as text/plain; version=0.0.4; charset=utf-8—and you cannot change the target's output, you can configure a fallback scrape protocol on the Prometheus side [6]. Key requirements for Prometheus 3.0 scrape targets: 1. Strict Content-Type: You must return a valid Content-Type header that includes the appropriate MIME type and version parameter [3][1]. 2. Fallback Configuration: If your target returns a non-standard or unsupported Content-Type string (such as one with additional charset parameters that the parser does not expect), you can define a fallback_scrape_protocol in your Prometheus configuration for that specific target to ensure the scrape succeeds [6]. 3. Recommended Format: The preferred text format for modern Prometheus usage is version 1.0.0 (text/plain; version=1.0.0; escaping=allow-utf-8) or the OpenMetrics format, as version 0.0.4 is considered a legacy fallback [3][4]. Targets should expose metrics using only one specific content type [1][2].

Citations:


🌐 Web query:

Prometheus plain text exposition content-type charset utf-8 accepted 0.0.4 parser

💡 Result:

The Prometheus text exposition format version 0.0.4 is designed to use the HTTP Content-Type header: text/plain; version=0.0.4; charset=utf-8 [1][2][3]. Key details regarding this specification and its parsing include: 1. Requirement for Semicolons: According to standard MIME media type definitions (as implemented by libraries like Go's mime package), media parameters must be separated by semicolons (;) [4]. A common error occurs when implementations incorrectly use commas (,) instead of semicolons to separate the charset parameter (e.g., text/plain; version=0.0.4, charset=utf-8), which causes parsing failures in strict environments such as Prometheus v3 [4]. 2. Charset Standard: The charset is universally expected to be UTF-8 [1][2]. While earlier discussions considered the explicit inclusion of the charset parameter [5], it has become standard practice in Prometheus client libraries to include; charset=utf-8 as part of the Content-Type string to ensure clarity and interoperability [2][6][3]. 3. Versioning: The version=0.0.4 parameter explicitly identifies the Prometheus text exposition format [1]. If the version parameter is missing, Prometheus servers may fall back to the most recent text format version [1]. 4. Troubleshooting Parsing Errors: If a scrape target provides an invalid Content-Type string (such as one using commas instead of semicolons), Prometheus v3 will fail to parse it [4]. In such cases, one must either correct the exporter's header generation or configure the Prometheus scrape job with the fallback_scrape_protocol=PrometheusText0.0.4 setting to bypass the strict header parsing [4].

Citations:


🌐 Web query:

Prometheus 3.0 scrape parser accepts Content-Type text/plain; charset=utf-8 text format version 0.0.4

💡 Result:

In Prometheus 3.0, the scrape parser requires that targets return a valid and supported Content-Type header [1][2]. While Prometheus text format version 0.0.4 is a supported format (identified by the MIME type text/plain with the parameter version=0.0.4) [3][4], the parser expects strict adherence to the defined Content-Type specifications [1][5]. Some users have reported that sending a Content-Type header formatted as text/plain; charset=utf-8 (without the required version parameter) fails in Prometheus 3.0 because it does not match the expected format string [6]. The parser in Prometheus 3.0 is more strict than in version 2.0; it no longer silently falls back to the classic text format if the header is missing, unparsable, or unrecognized [1][2][7]. If a scrape target provides an invalid or missing Content-Type, the scrape will fail unless a fallback_scrape_protocol is explicitly configured in the Prometheus scrape configuration [1][2][5]. To ensure compatibility with Prometheus 3.0, endpoints should return the correct Content-Type header, such as: text/plain;version=0.0.4 [3][1] If a target cannot be updated to provide the exact header, you can configure the scrape job in Prometheus to use the appropriate fallback: scrape_configs: - job_name: 'example' fallback_scrape_protocol: PrometheusText0.0.4 scrape_configs:... [1][2][8]

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the observability text format and the current test expectations.
sed -n '1,220p' src/relay/observability/metrics.py
printf '\n--- tests ---\n'
sed -n '1,220p' tests/test_observability.py

Repository: Aakash1337/Relay

Length of output: 9389


🌐 Web query:

Starlette PlainTextResponse default media_type text/plain charset utf-8 documentation

💡 Result:

In Starlette, the PlainTextResponse class inherits from the base Response class [1]. Its default media_type is set to "text/plain" [1]. When a response has a media type starting with "text/" (which includes "text/plain"), Starlette automatically appends a charset parameter to the Content-Type header if one is not already present [2][3][4]. By default, this charset is "utf-8" [1]. Therefore, a standard PlainTextResponse will typically result in a Content-Type header value of "text/plain; charset=utf-8". Starlette is designed to avoid appending a duplicate charset if you have already explicitly included one in the media_type parameter [3][5].

Citations:


Set the Prometheus exposition Content-Type explicitly.

PlainTextResponse will emit text/plain; charset=utf-8, which omits the version=0.0.4 parameter Prometheus expects for this exposition format. That can break scrapes for /metrics/prometheus unless the job is configured with a fallback.

🔧 Proposed fix
     from fastapi.responses import PlainTextResponse

-    return PlainTextResponse(prometheus_text(tenant_metrics(tenant_id)))
+    return PlainTextResponse(
+        prometheus_text(tenant_metrics(tenant_id)),
+        media_type="text/plain; version=0.0.4; charset=utf-8",
+    )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@router.get("/metrics/prometheus", include_in_schema=False)
def metrics_prometheus(
tenant_id: uuid.UUID = Depends(require_tenant),
):
from fastapi.responses import PlainTextResponse
return PlainTextResponse(prometheus_text(tenant_metrics(tenant_id)))
`@router.get`("/metrics/prometheus", include_in_schema=False)
def metrics_prometheus(
tenant_id: uuid.UUID = Depends(require_tenant),
):
from fastapi.responses import PlainTextResponse
return PlainTextResponse(
prometheus_text(tenant_metrics(tenant_id)),
media_type="text/plain; version=0.0.4; charset=utf-8",
)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/api/routes.py` around lines 529 - 535, The metrics_prometheus route
is returning a PlainTextResponse, which leaves the Prometheus exposition
Content-Type too generic. Update metrics_prometheus to set the explicit
Prometheus content type for the text_format output (including the version=0.0.4
parameter) instead of relying on PlainTextResponse defaults, so scrapers can
identify the format correctly.

Comment on lines +87 to +125
with tenant_session(tenant_id) as session:
stale_runs = (
session.execute(
select(PipelineRun).where(
PipelineRun.status == "running",
PipelineRun.started_at < cutoff,
)
)
.scalars()
.all()
)
for run in stale_runs:
run.status = "failed"
run.detail = "orphaned by crash; closed by recovery"
run.finished_at = datetime.now(tz=UTC)
audit.record(
session,
tenant_id=tenant_id,
actor_type="worker",
actor_id=ACTOR,
action="run.recovered",
entity_type="pipeline_run",
entity_id=str(run.id),
payload={"lead_id": str(run.lead_id) if run.lead_id else None},
)
stats.runs_closed += 1

# 2. Fail orphaned mid-send jobs — never retry, never assume sent.
with tenant_session(tenant_id) as session:
stale_jobs = (
session.execute(
select(SendJob).where(
SendJob.status == "sending",
SendJob.started_at < cutoff,
)
)
.scalars()
.all()
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP '\b(recover_orphans|process_pending)\s*\(' --type=py -C2

Repository: Aakash1337/Relay

Length of output: 154


🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files 'src/relay/pipeline/*' 'src/relay/**' | sed -n '1,200p'
echo '--- recovery.py outline ---'
ast-grep outline src/relay/pipeline/recovery.py --view expanded
echo '--- references to recover_orphans/process_pending/tenant_session ---'
rg -n --hidden --glob '!**/.git/**' '\brecover_orphans\b|\bprocess_pending\b|\btenant_session\b' src tests -C 3

Repository: Aakash1337/Relay

Length of output: 50373


🏁 Script executed:

#!/bin/bash
set -euo pipefail

nl -ba src/relay/pipeline/recovery.py | sed -n '1,240p'

Repository: Aakash1337/Relay

Length of output: 194


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- recovery.py ---'
sed -n '1,240p' src/relay/pipeline/recovery.py | cat -n

echo '--- process_pending usage ---'
rg -n '\bprocess_pending\b' src tests -C 2

echo '--- recover_orphans usage ---'
rg -n '\brecover_orphans\b' src tests -C 2

Repository: Aakash1337/Relay

Length of output: 16565


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- send_worker.py ---'
sed -n '1,240p' src/relay/workers/send_worker.py | cat -n

echo '--- api routes / ops ui mentions ---'
rg -n 'process_pending|recover_orphans|send_worker|worker' src/relay/api src/relay/workers -C 2

Repository: Aakash1337/Relay

Length of output: 18071


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- state_machine.py outline ---'
ast-grep outline src/relay/domain/state_machine.py --view expanded

echo '--- transition implementation ---'
sed -n '1,260p' src/relay/domain/state_machine.py | cat -n

echo '--- model constraints for PipelineRun, SendJob, AuditLog ---'
rg -n 'class PipelineRun|class SendJob|class AuditLog|UniqueConstraint|CheckConstraint|status|transition' src/relay/db/models.py -C 3

Repository: Aakash1337/Relay

Length of output: 18823


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- state definitions around SEND_QUEUED / ERROR_TERMINAL ---'
rg -n 'SEND_QUEUED|ERROR_TERMINAL|send_queued|error_terminal' src/relay/domain src/relay/db -C 3

echo '--- transition rules table / seeds ---'
rg -n 'lead_transition_rules|send_queued|error_terminal' src/relay/db src/relay/domain tests -C 2

Repository: Aakash1337/Relay

Length of output: 20664


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- trigger/function references for leads and send_jobs ---'
rg -n 'lead.*trigger|transition|illegal lead state|send_jobs.*trigger|status.*sending|error_terminal|send_queued' src/relay/db/sql src/relay/db/models.py -C 4

echo '--- relevant SQL functions ---'
sed -n '1,260p' src/relay/db/sql/002_functions.sql | cat -n
echo '--- triggers ---'
sed -n '1,240p' src/relay/db/sql/003_triggers.sql | cat -n

Repository: Aakash1337/Relay

Length of output: 39388


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- 002_functions.sql ---'
sed -n '1,260p' src/relay/db/sql/002_functions.sql | cat -n

echo '--- 003_triggers.sql ---'
sed -n '1,260p' src/relay/db/sql/003_triggers.sql | cat -n

Repository: Aakash1337/Relay

Length of output: 16970


Recovery scans can double-record stale rows under overlapping ticks

process_pending() is reachable from both the admin tick route and the worker/CLI, and _recover_tenant() reads stale runs/jobs without row locks. Overlapping passes can select the same rows and append duplicate recovery audit entries / stats. Add FOR UPDATE SKIP LOCKED here if parallel recovery ticks are possible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/pipeline/recovery.py` around lines 87 - 125, The stale recovery
queries in _recover_tenant can be picked up by overlapping process_pending()
executions and double-record audit/stats for the same rows. Update the
PipelineRun and SendJob lookups inside tenant_session to lock eligible rows with
FOR UPDATE SKIP LOCKED (or the SQLAlchemy equivalent on the select used with
session.execute), so parallel recovery ticks only process each stale record
once.

Comment on lines +67 to +78
settings = get_settings()
# Backpressure before the call; bounded backoff around it — transient
# failures only. A refusal or bad output is never re-rolled, and a
# failing tier is never silently swapped for another.
limit_compute(str(decision.tier))
response = with_backoff(
lambda: backend.complete(request),
attempts=settings.compute_retry_attempts,
base_seconds=settings.compute_retry_base_seconds,
retry_on=(ComputeUnavailable,),
what=f"{backend.name}:{task_type}",
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'statement_timeout|pool_timeout|pool_size' src/relay/db 2>/dev/null
rg -n 'def tenant_session' -A15 src/relay/db 2>/dev/null

Repository: Aakash1337/Relay

Length of output: 1077


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== executors outline ==\n'
ast-grep outline src/relay/routing/executors.py --view expanded || true

printf '\n== engine tenant_session + session listeners ==\n'
ast-grep outline src/relay/db/engine.py --view expanded || true

printf '\n== limit_compute search ==\n'
rg -n "def limit_compute|limit_compute\(" src/relay -A20 -B10 || true

printf '\n== with_backoff search ==\n'
rg -n "def with_backoff|with_backoff\(" src/relay -A30 -B10 || true

printf '\n== PipelineRunner execution path search ==\n'
rg -n "class PipelineRunner|_advance_once|_step_check_source|execute\(" src/relay -A30 -B10 || true

Repository: Aakash1337/Relay

Length of output: 50373


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== PipelineRunner / step functions ==\n'
rg -n "class PipelineRunner|def _advance_once|def _step_check_source|def execute\(" src/relay/pipeline src/relay -A40 -B20 || true

printf '\n== tenant_session call sites with nontrivial work ==\n'
rg -n "with tenant_session\(" src/relay -A25 -B10 || true

printf '\n== settings for rate limit and compute retries ==\n'
rg -n "rate_limit_max_wait_seconds|compute_retry_attempts|compute_retry_base_seconds" src/relay -A8 -B8 || true

Repository: Aakash1337/Relay

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== engine.py relevant section ==\n'
sed -n '1,140p' src/relay/db/engine.py

printf '\n== ratelimit.py relevant section ==\n'
sed -n '1,220p' src/relay/ratelimit.py

printf '\n== config settings section ==\n'
sed -n '1,260p' src/relay/config.py

Repository: Aakash1337/Relay

Length of output: 15473


Move rate-limit/backoff out of the tenant transaction PipelineRunner._advance_once() keeps tenant_session() open while execute() runs, so limit_compute() and the with_backoff() sleeps hold the tenant connection/transaction. With the default 30s max wait plus retries, that can tie up pool slots under load. Split the DB work from the compute call, or move the wait/retry path before opening the tenant session.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/routing/executors.py` around lines 67 - 78, Move the rate-limit and
retry logic out of the tenant transaction in PipelineRunner._advance_once so
tenant_session() is not held open during limit_compute() and with_backoff()
sleeps. Split the DB-read/decision work from the backend.complete(request) call,
or perform the backoff and compute wait path before opening tenant_session(),
keeping the transaction limited to the minimal database work.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'Backpressure' -g '*.py'
rg -n -B3 -A15 'except ComputeUnavailable' src/relay/pipeline/runner.py

Repository: Aakash1337/Relay

Length of output: 154


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files:\n'
git ls-files 'src/relay/**/*.py' | sed -n '1,120p'

printf '\nBackpressure/ComputeUnavailable references:\n'
rg -n 'Backpressure|ComputeUnavailable|limit_compute|with_backoff|tenant_session|error_retryable' src/relay -g '*.py'

printf '\nrunner.py outline:\n'
ast-grep outline src/relay/pipeline/runner.py --view expanded

printf '\nexecutors.py outline:\n'
ast-grep outline src/relay/routing/executors.py --view expanded

printf '\nratelimit.py outline:\n'
ast-grep outline src/relay/ratelimit.py --view expanded

Repository: Aakash1337/Relay

Length of output: 12089


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' src/relay/ratelimit.py
printf '\n--- runner.py ---\n'
sed -n '1,240p' src/relay/pipeline/runner.py
printf '\n--- executors.py ---\n'
sed -n '1,180p' src/relay/routing/executors.py

Repository: Aakash1337/Relay

Length of output: 17728


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the relevant files precisely and print compact context around the cited code.
fd -a 'runner.py|executors.py|ratelimit.py' src/relay
printf '\n--- src/relay/routing/executors.py ---\n'
nl -ba src/relay/routing/executors.py | sed -n '1,140p'
printf '\n--- src/relay/pipeline/runner.py ---\n'
nl -ba src/relay/pipeline/runner.py | sed -n '1,220p'
printf '\n--- src/relay/ratelimit.py ---\n'
nl -ba src/relay/ratelimit.py | sed -n '1,220p'

Repository: Aakash1337/Relay

Length of output: 374


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search the entire repository for the exception and the helper wiring.
rg -n --hidden --glob '!**/.git/**' 'class Backpressure|Backpressure\(|raise Backpressure|limit_compute\(|with_backoff\(' .

# Show the exact implementation of any matching symbols if found.
python3 - <<'PY'
from pathlib import Path
for path in [Path('src/relay/ratelimit.py'), Path('src/relay/routing/executors.py'), Path('src/relay/pipeline/runner.py')]:
    if path.exists():
        print(f"\n### {path}")
        for i, line in enumerate(path.read_text().splitlines(), 1):
            if 'Backpressure' in line or 'limit_compute' in line or 'with_backoff' in line or 'ComputeUnavailable' in line or 'ComputeRefused' in line or 'tenant_session' in line:
                print(f"{i}: {line}")
PY

Repository: Aakash1337/Relay

Length of output: 2031


Route Backpressure through the retryable compute path

  • limit_compute() raises relay.ratelimit.Backpressure, but _advance_once() only catches ComputeUnavailable / ComputeRefused, so this escapes to run()’s generic exception handler and fails the whole run instead of parking error_retryable.
  • limit_compute() also sleeps while tenant_session() is open, so a rate-limit wait holds the DB transaction open.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/relay/routing/executors.py` around lines 67 - 78, `_advance_once()` is
not handling `relay.ratelimit.Backpressure`, so rate-limit waits escape the
retryable compute path and can also hold `tenant_session()` open while sleeping.
Update the flow around `limit_compute()` and `with_backoff()` in
`_advance_once()` to treat `Backpressure` like the other retryable compute
failures, convert it into the retryable/error-retryable path instead of letting
it reach `run()`’s generic handler, and ensure any backoff sleep happens outside
the open database transaction.

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.

2 participants