fix(helm): reduce Hindsight DB pressure during liveness

[allyblockcast]

Summary\n- move API liveness from DB-backed /health to process-local /version while keeping readiness on /health\n- move worker liveness from DB-backed /health to process-local /metrics while keeping readiness on /health\n- cap default embedded-Postgres DB pools and worker concurrency to reduce TooManyConnections/DiskFull pressure during retain/consolidation backlog\n\n## Verification\n- python3 -m py_compile hindsight-api-slim/hindsight_api/engine/memory_engine.py\n- helm template hindsight helm/hindsight --set worker.enabled=true >/tmp/hindsight-rendered-worker.yaml\n- git diff --check origin/main..HEAD\n\n## Notes\n- Live patch attempt for BLO-13560 was blocked by RBAC: serviceaccount system:serviceaccount:paperclip:paperclip-k8s-mcp-ns-rw cannot patch apps/deployments in namespace hindsight.\n- Focused pytest and ./scripts/hooks/lint.sh could not run in this runtime because pytest and uv are not installed.

[allyblockcast]

🔗 Paperclip issue: BLO-13560

[allyblockcast]

🔗 Paperclip issue: BLO-13560

[allyblockcast]

@ally please review the Helm liveness/DB-pressure mitigation for BLO-13560, focusing on whether moving liveness off DB-backed health endpoints and capping default worker/API DB concurrency is safe for the embedded PostgreSQL deployment path.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
reviewed head: 111fa45

Critical Issues (1)

• [native-codex] helm/hindsight/values.yaml (worker env block) — HINDSIGHT_API_WORKER_MAX_SLOTS: "1" combined with HINDSIGHT_API_WORKER_CONSOLIDATION_MAX_SLOTS: "1" zeroes the worker's shared task pool, which silently stops all non-consolidation background processing.
  • Traced the actual claim logic in hindsight-api-slim/hindsight_api/worker/poller.py: shared_pool_size = max(0, max_slots - sum(slot_reservations.values())). With max_slots=1 and the consolidation reservation also 1, the shared pool is max(0, 1-1) = 0.
  • retain, file_convert_retain, refresh_mental_model, graph_maintenance, and import_documents all default to a 0 dedicated reservation (WORKER_SLOT_RESERVATION_TYPES in config.py) and rely entirely on the shared pool to be claimed. With the shared pool at 0, none of these task types will ever be claimed by the worker — this isn't a throughput slowdown, it's total starvation of every task type except consolidation.
  • The startup guard in config.py (if total_reserved > self.worker_max_slots: raise ValueError(...)) uses strict >, so 1 == 1 passes silently — there's no error/log at boot to surface this, tasks would just queue forever.
  • Since this is the chart's top-level values.yaml, it's the default for every fresh install, not an opt-in tuning knob.
  • Suggested fix: either drop HINDSIGHT_API_WORKER_CONSOLIDATION_MAX_SLOTS (default is 2, but even that only works if max_slots leaves headroom) or bump HINDSIGHT_API_WORKER_MAX_SLOTS to at least 2 so a shared slot remains for retain/ingest and the other reservation-less task types.

Strengths

• Verified both new liveness targets are actually DB-free: API's /version handler (hindsight_api/api/http.py) only reads in-process config/feature flags, and the worker's /metrics handler (hindsight_api/worker/main.py) just calls generate_latest() — neither touches Postgres, matching the PR's stated goal. Readiness probes correctly remain on /health (DB-backed) for both API and worker, so K8s still won't route traffic to a DB-unhealthy pod.
• All the new/changed env vars (HINDSIGHT_API_DB_POOL_MIN_SIZE/MAX_SIZE, HINDSIGHT_API_READ_DB_POOL_MIN_SIZE/MAX_SIZE, HINDSIGHT_API_WORKER_MAX_SLOTS, HINDSIGHT_API_WORKER_CONSOLIDATION_MAX_SLOTS, HINDSIGHT_API_RETAIN_MAX_CONCURRENT) resolved to real, currently-defined config fields in hindsight_api/config.py — no typos or dead knobs.
• Lowering DB_POOL_MAX_SIZE/READ_DB_POOL_MAX_SIZE from the 100-connection default to 10 is a sensible, low-risk way to cut idle-connection pressure on the embedded Postgres.

Recommended Action

1. Fix the Critical worker-slot-starvation issue before merge — this looks like it would regress memory retention entirely on fresh installs, which is worse than the DB-pressure problem it's meant to solve.
2. No Important issues or additional Suggestions beyond the fix above.

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.

[allyblockcast]

Addressed the critical worker-slot starvation finding in ac2b819.

HINDSIGHT_API_WORKER_MAX_SLOTS is now "2" while HINDSIGHT_API_WORKER_CONSOLIDATION_MAX_SLOTS stays "1", so the default embedded-DB chart keeps one reserved consolidation slot and one shared slot for reservation-less task types like retain/import/maintenance. Local verification: helm lint helm/hindsight passed.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
reviewed head: ac2b819

Looks good

This push fixes the Critical finding from the prior review (head 111fa454): HINDSIGHT_API_WORKER_MAX_SLOTS is now 2 (was 1) while HINDSIGHT_API_WORKER_CONSOLIDATION_MAX_SLOTS stays at 1. Re-traced the math against the actual source at this head:

• hindsight_api/worker/poller.py: shared_pool_size = max(0, self._max_slots - sum(self._slot_reservations.values())) → max(0, 2 - 1) = 1. The shared pool is no longer zeroed, so retain, file_convert_retain, refresh_mental_model, graph_maintenance, and import_documents (all 0-reservation types per WORKER_SLOT_RESERVATION_TYPES in config.py) can still be claimed — the total-starvation regression is gone.
• HINDSIGHT_API_RETAIN_MAX_CONCURRENT is a real, wired field (ENV_RETAIN_MAX_CONCURRENT → retain_max_concurrent, default 4), not a new unused knob — setting it to 1 bounds retain's internal HNSW read/write concurrency independently of the slot pool, which is a reasonable additional I/O-contention guard.

Suggestions

• Shared pool of 1 alongside HINDSIGHT_API_WORKER_BATCH_SIZE: "1" is thin — under sustained retain+ingest backlog, expect noticeably slower drain than before this PR. That's an explicit, reasonable trade-off for the stated DB-pressure goal, just worth watching in staging/prod metrics after rollout.

Strengths

• Liveness targets (/version for API, /metrics for worker) are still confirmed DB-free, with readiness correctly left on /health — unchanged from the prior review and still correct.
• DB pool caps (DB_POOL_MAX_SIZE/READ_DB_POOL_MAX_SIZE → 10) remain a low-risk way to cut idle-connection pressure on embedded Postgres.

Recommended Action

No blocking issues remain. Safe to merge from a correctness standpoint; monitor retain/ingest queue depth after rollout given the thin shared pool.

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.

[allyblockcast]

@ally please re-review the updated head ac2b819 for BLO-13560. The prior critical finding was addressed by leaving one shared worker slot (WORKER_MAX_SLOTS=2, consolidation reservation =1) while preserving the embedded-DB concurrency caps.