fix(helm): align Hindsight probes and worker caps

[allyblockcast]

Summary

• point API liveness/readiness at /version instead of /health
• point worker liveness/readiness at /metrics instead of /health
• keep worker DB-pressure defaults at batch size 1, max slots 2, consolidation slot 1, retain slot 1, and retain concurrency 1

Verification

• helm lint helm/hindsight
• helm template hindsight helm/hindsight --set worker.enabled=true --set worker.replicaCount=4

Live rollout note

• Live patch attempt for BLO-14004 was blocked by Kubernetes RBAC: system:serviceaccount:paperclip:paperclip-k8s-mcp-ns-rw cannot patch apps/deployments in namespace hindsight. BLO-14005 tracks the scoped rollout permission/operator path needed before live soak can complete.

[allyblockcast]

🔗 Paperclip issue: BLO-14004
🔗 Paperclip issue: BLO-14005

[allyblockcast]

🔗 Paperclip issue: BLO-14004
🔗 Paperclip issue: BLO-14005

[allyblockcast]

@ally Please review the Helm remediation for BLO-14004/BLO-13567. Focus on whether the API /version probes, worker /metrics probes, and worker retain/consolidation slot defaults match the intended live DB-pressure constraints without breaking the chart's StatefulSet worker identity contract.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Self-review comment mode: this PR was authored by the same bot identity; formal review/approval must come from a human or a distinct reviewer identity.
reviewed head: 9079cf5

Critical Issues (1)

• [gstack/review] helm/hindsight/values.yaml:166-169 — the new HINDSIGHT_API_WORKER_RETAIN_MAX_SLOTS: "1" zeroes the worker's shared task-slot pool, permanently starving four other task types.
  • worker_max_slots=2, consolidation reservation=1 (line 168), and now retain reservation=1 (line 169, new in this PR) — reservations sum to exactly worker_max_slots. hindsight-api-slim/hindsight_api/worker/poller.py computes shared_pool_size = max(0, max_slots - sum(reservations)), which is now max(0, 2-2) = 0.
  • Per hindsight-api-slim/hindsight_api/config.py:534-541, file_convert_retain, refresh_mental_model, graph_maintenance, and import_documents all default to a 0 reservation — they rely entirely on the shared pool to run. With the shared pool now permanently 0, none of those task types can ever be claimed by this worker, with no error or warning surfaced.
  • The config-level validator (config.py:1989-1997) only raises when total_reserved > worker_max_slots; it does not catch the == case, so this ships silently.
  • The retained comment directly above (line 166, unchanged by this PR) — "Keep one shared slot available for reservation-less task types like retain." — is now false: retain is no longer reservation-less, and it consumed the shared slot the comment describes.
  • Recommendation: either bump HINDSIGHT_API_WORKER_MAX_SLOTS to 3 to preserve a shared slot, or confirm graph_maintenance/import_documents/refresh_mental_model are genuinely unused in this deployment and update the stale comment to say so explicitly.

Important Issues (1)

• [native-codex] helm/hindsight/values.yaml:51,152 — readiness probes for both API and worker now point at endpoints that don't check DB connectivity, removing DB-outage detection from k8s orchestration.
  • hindsight-api-slim/hindsight_api/api/http.py:3391-3401 — /health calls app.state.memory.health_check() and returns 503 on DB failure. /version (the new readiness target) returns static version/feature-flag info with no DB touch.
  • hindsight-api-slim/hindsight_api/worker/main.py:90-107 — same pattern: /health calls memory.health_check() and returns 503 on failure; /metrics (the new readiness target) just serializes Prometheus counters regardless of DB state.
  • livenessProbe already made this same swap in an earlier commit (unchanged by this diff), so this PR completes the pattern rather than introducing it fresh — but the net effect after this PR is that no probe on either service checks DB connectivity anymore. If the DB goes down, pods stay Ready/Live and keep taking traffic instead of being pulled from rotation or restarted.
  • The PR's own "Live rollout note" says the live patch for BLO-14004 was blocked by RBAC and hasn't soaked in production yet — worth confirming there's compensating DB-health alerting (Prometheus/alertmanager on the DB pool metrics) before this rolls out, since the probe-based safety net is gone.

Strengths

• Diff is minimal and scoped exactly to what the PR description promises (probe paths + one new env var).
• helm lint / helm template verification steps are a reasonable smoke check for a chart-only change.
• Live-rollout blocker (RBAC) is called out transparently in the PR body rather than silently left unverified.

Recommended Action

1. Fix the shared-slot starvation (Critical) before merge — either raise worker.env.HINDSIGHT_API_WORKER_MAX_SLOTS or confirm/document that the zero-reservation task types are unused here.
2. Confirm DB-health alerting exists independent of the probes (Important) before this reaches production, given the probes no longer catch DB outages.

[allyblockcast]

Addressed both findings in f205156:

Critical (shared-slot starvation) — confirmed: graph_maintenance is enqueued directly out of the core retain path (fact_storage.py:331 calls enqueue_relink_victims), so this wasn't a theoretical risk — zeroing the shared pool would have silently stalled graph maintenance (and file_convert_retain/refresh_mental_model/import_documents) on every worker. Bumped HINDSIGHT_API_WORKER_MAX_SLOTS from 2 to 3 to restore one shared slot alongside the two reservations, and reworded the stale comment to spell out the invariant (max_slots must exceed sum(reservations)).

Important (readiness probes lost DB-outage detection) — reverted readinessProbe.httpGet.path back to /health for both api and worker, keeping livenessProbe on /version//metrics. This matches standard k8s guidance: liveness shouldn't depend on downstream deps (avoids restart storms on a transient DB blip), but readiness should — so k8s pulls a pod from Service rotation when the DB is down instead of routing live traffic to a pod that can't serve it. Added comments explaining the asymmetry so it doesn't get "fixed" back to /version again.

Verified with helm lint helm/hindsight and helm template hindsight helm/hindsight --set worker.enabled=true --set worker.replicaCount=4 — probe paths and worker env render as expected.

Live-rollout blocker from the PR description is unchanged (RBAC — tracked in BLO-14005).

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Self-review comment mode: this PR was authored by the same bot identity; formal review/approval must come from a human or a distinct reviewer identity.
reviewed head: f205156

Looks good

This push resolves both issues flagged in the prior review of this PR (head 9079cf5):

• Shared-slot starvation (previously Critical) — fixed. HINDSIGHT_API_WORKER_MAX_SLOTS is now 3 (was 2), and the new HINDSIGHT_API_WORKER_RETAIN_MAX_SLOTS: "1" reservation no longer exhausts the pool: consolidation(1) + retain(1) = 2 reserved, leaving 1 shared slot for the reservation-less types (file_convert_retain, refresh_mental_model, graph_maintenance, import_documents). Verified directly against hindsight-api-slim/hindsight_api/config.py:534-541 — the env var name matches the retain reservation key exactly — and the validator at config.py:1989-1997 now passes with headroom (2 < 3) rather than silently landing on the unchecked == case.
• Readiness probes losing DB-connectivity detection (previously Important) — fixed. Both api.readinessProbe and worker.readinessProbe still target /health (unchanged from base, now with explanatory comments); only livenessProbe targets /version//metrics. That's the correct k8s split: liveness shouldn't restart pods for external dependency outages, readiness should pull them from rotation. Verified the probe paths in the fetched blob at this head — no probe regression.

Strengths

• The new comments on MAX_SLOTS and the readiness probes explicitly encode the invariant that broke before ("keep MAX_SLOTS one greater than the sum of reservations below") — good defense against this exact regression recurring.
• Diff stays minimal and scoped to the two previously-identified defects; no unrelated churn.

Note (non-blocking)

The PR description text ("point API liveness/readiness at /version instead of /health", "point worker liveness/readiness at /metrics instead of /health") describes an earlier state of this branch and is now stale relative to the current diff, where readiness stays on /health. Worth a quick edit to the PR body before merge so the description matches what's shipping, but this doesn't block — the code itself is correct.

Recommended Action

1. Optionally update the PR description to match the current (correct) diff.
2. Ready to merge once CI (helm lint / helm template) is green.