ALL-205: Make reranker health, startup, and installer config provider-aware

[christabone]

TL;DR

• Makes reranker configuration, startup health, and installer behavior provider-aware while keeping bedrock_cohere as the standalone default.

Summary

• Adds provider-aware reranker requirement resolution in config/connections.yaml and aligns backend startup health handling with RERANK_PROVIDER.
• Extends standalone installer env output with RERANK_PROVIDER, BEDROCK_RERANK_MODEL_ARN, and RERANKER_URL, and moves provider-specific warnings to stages that already know the selected provider.
• Updates backend, installer shell, and related regression tests for the provider-aware paths.

Test Plan

• docker compose -f docker-compose.test.yml run --rm backend-unit-tests bash -lc "python -m pytest tests/unit/api/test_auth_api.py tests/unit/api/test_auth_api_endpoints.py tests/unit/api/test_chat_execute_flow_endpoint.py tests/unit/api/test_flow_evidence_export.py tests/unit/api/test_flows_api.py tests/unit/api/test_flows_endpoints.py tests/unit/lib/pdf_viewer/test_rapidfuzz_matcher.py -v --tb=short"
• docker compose -f docker-compose.test.yml run --rm backend-unit-tests bash -lc "python -m pytest tests/unit/test_config_loaders.py tests/unit/test_main_startup.py -v --tb=short"
• cd frontend && npm run test -- --run src/features/curation/evidence/useEvidenceNavigation.test.ts
• cd frontend && npm run test -- --run

Manual validation checklist

• Bedrock mode starts cleanly with HEALTH_CHECK_STRICT_MODE=true and no local reranker container present.
• local_transformers mode starts cleanly with HEALTH_CHECK_STRICT_MODE=true and the local reranker container present.
• none mode starts cleanly with HEALTH_CHECK_STRICT_MODE=true and no local reranker container present.

[claude]

PR Review: ALL-205 — Make reranker health, startup, and installer config provider-aware

Summary

This PR introduces a required_when conditional mechanism for the connections config so that the reranker service health check is only enforced at startup when RERANK_PROVIDER=local_transformers. The installer scripts are updated to prompt for rerank provider selection and persist the related env vars (RERANK_PROVIDER, BEDROCK_RERANK_MODEL_ARN, RERANKER_URL). Preflight memory warnings are now provider-aware. Tests are comprehensive and cover the new paths well.

---

BLOCKING

1. Comment uses prohibited word "fallback"
config/connections.yaml:21

#       - default: Fallback when the env var is unset

Per project rules, comments containing the word "fallback" are flagged as blocking regardless of intent. Reword the comment.

Suggested fix:

#       - default: Value assumed when the env var is unset

---

Warnings

1. Default value for RERANK_PROVIDER is duplicated across files
backend/main.py:378:

rerank_provider = os.environ.get("RERANK_PROVIDER", "bedrock_cohere").strip().lower()

config/connections.yaml:124:

default: bedrock_cohere

scripts/install/02_core_config.sh:16:

DEFAULT_RERANK_PROVIDER="bedrock_cohere"

scripts/install/lib/templates/env.standalone:54:

RERANK_PROVIDER=bedrock_cohere

The string "bedrock_cohere" is the implicit default in four separate locations. If this default ever changes, all four must be updated in lockstep. Consider defining the canonical default in one place (e.g., the connections YAML required_when.default) and having main.py rely on the connections loader's resolution rather than reading RERANK_PROVIDER independently.

2. Duplicated shell functions across installer scripts
normalize_rerank_provider is defined identically in both scripts/install/01_preflight.sh:71 and scripts/install/02_core_config.sh:73. Same for read_existing_env_value (01_preflight.sh:56 and 02_core_config.sh:38). The shared library scripts/install/lib/common.sh already exists and hosts similar utilities — consider moving these there.

---

Suggestions

1. Logging in main.py is informational-only — consider tightening
The new log block at main.py:381-390 logs whether the reranker health check is "enforced" or "advisory" but doesn't influence control flow — the actual enforcement is already handled by the strict_mode and required_services branch below. The logging is fine for observability, but the rerank_provider variable read from the env is only used for the log message and nowhere else in this function. If it's purely diagnostic, that's fine, but note it's redundant with what connections_loader already resolves.

---

What looks good

• The _resolve_required_flag / _parse_boolean_value design is clean and validates inputs strictly (raises ValueError on bad types), which is the right approach.
• required_when in the YAML schema is a good general-purpose mechanism that can be reused for other conditional services.
• Test coverage is thorough: unit tests for required_when parsing, integration test for get_required_connections with different provider values, startup tests for strict-mode behavior, and comprehensive shell test updates for the installer.
• The preflight memory_warning_suffix is a nice touch — only warns about memory-intensive local models when relevant.

Overall Assessment

This is a well-structured PR. One blocking comment wording issue to fix, and some duplication to consider consolidating. The core logic is sound and well-tested.

---

Need help? Comment @claude followed by your request.

[christabone]

@claude Please review these recent changes.

• Updated PR head: 95338f3
• Context: addressed prior PR feedback

[claude]

Re-review: ALL-205 — Make reranker health, startup, and installer config provider-aware

Triggered by commit 95338f3 ("ALL-205: Fix CI follow-ups for reranker provider PR")

Status of previously flagged issues

1. BLOCKING (now resolved): "fallback" comment wording
config/connections.yaml — The comment now reads Value assumed when the env var is unset instead of Fallback when the env var is unset. Addressed in 95338f3.

2. Warning (still open): Default value "bedrock_cohere" duplicated across four locations

• backend/main.py:378 — os.environ.get("RERANK_PROVIDER", "bedrock_cohere")
• config/connections.yaml:123 — default: bedrock_cohere
• scripts/install/02_core_config.sh:16 — DEFAULT_RERANK_PROVIDER="bedrock_cohere"
• scripts/install/lib/templates/env.standalone:54 — RERANK_PROVIDER=bedrock_cohere

Not blocking, but a future source of drift.

3. Warning (still open): Duplicated shell functions
normalize_rerank_provider is defined identically in both scripts/install/01_preflight.sh and scripts/install/02_core_config.sh. read_existing_env_value is defined in both scripts as well (with the same body but reading from different path variables). Consider extracting at least normalize_rerank_provider into scripts/install/lib/common.sh.

---

New changes in 95338f3

1. Warning: try/except rapidfuzz pattern in 6 test files

Files affected:

• backend/tests/unit/api/test_auth_api.py
• backend/tests/unit/api/test_auth_api_endpoints.py
• backend/tests/unit/api/test_chat_execute_flow_endpoint.py
• backend/tests/unit/api/test_flow_evidence_export.py
• backend/tests/unit/api/test_flows_api.py
• backend/tests/unit/api/test_flows_endpoints.py

The original code unconditionally stubbed rapidfuzz via sys.modules.setdefault(...). The new pattern is:

try:
    import rapidfuzz  # noqa: F401
except ModuleNotFoundError:
    sys.modules.setdefault("rapidfuzz", SimpleNamespace(...))

This is a try/except that catches an error and provides a default value, which matches the letter of the project's no-fallback rule. However, this is test scaffolding managing an optional test dependency — not production code masking missing data. The new pattern is also strictly better than what it replaces (preferring the real module when available). Flagging as a warning rather than blocking given the test-only context; team can decide if this needs a different approach (e.g., marking rapidfuzz as a required test dependency so the except path is never hit).

2. Frontend test loosened assertion
frontend/src/features/curation/evidence/useEvidenceNavigation.test.ts

-      anchor: evidence.anchor,
+      anchor: expect.objectContaining({
+        ...evidence.anchor,
+        subsection_title: null,
+      }),

This looks like a fix for a new subsection_title field being added to anchors. The expect.objectContaining makes this tolerant of additional future fields. This is fine — it's a test assertion adjustment, not a production fallback.

---

Overall Assessment

The blocking issue from the first review has been addressed. The core logic (_resolve_required_flag, required_when config mechanism, installer prompts, preflight provider-awareness) remains well-structured and well-tested. The remaining warnings are about code hygiene (duplication) rather than correctness. The rapidfuzz try/except is worth a team discussion on whether the dependency should just be required in test environments, but it's not blocking.

This PR looks good to merge.

---

Need help? Comment @claude followed by your request.

[christabone]

@claude Please review these recent changes.

• Updated PR head: 77f7c45
• Context: addressed prior PR feedback, fixed the Agent PR Gate lint issue, and all PR checks are now passing