Skip to content

feat(api): opt-in per-bank attribution for remote reranker requests#2781

Open
chethanuk wants to merge 1 commit into
vectorize-io:mainfrom
chethanuk:fix/issue-2729-featapi-optin-perbank-attribution-for-remote
Open

feat(api): opt-in per-bank attribution for remote reranker requests#2781
chethanuk wants to merge 1 commit into
vectorize-io:mainfrom
chethanuk:fix/issue-2729-featapi-optin-perbank-attribution-for-remote

Conversation

@chethanuk

Copy link
Copy Markdown

Summary

Opt-in per-bank cost attribution for remote rerank requests. When HINDSIGHT_API_RERANKER_SEND_BANK_AS_HEADER=true, outbound rerank POSTs carry X-Hindsight-Bank-Id: <bank_id>. Default is false — a byte-identical no-op.

Covers: the Cohere-compatible HTTP client (Cohere via base URL, OpenRouter, ZeroEntropy, SiliconFlow, Alibaba) and the LiteLLM proxy reranker (LiteLLMCrossEncoder, same per-request pattern).

Fixes #2729

Root cause

_CohereCompatibleRerankClient.predict posted with no per-request headers. The process-wide client only carries Authorization/Content-Type and is shared across banks, so a client-default header could pin bank A's id onto bank B's requests. The existing apply_bank_attribution() body mutator can't help — the Cohere rerank wire format has no OpenAI-style user field.

sequenceDiagram
    participant R as recall / reflect
    participant CV as _current_bank_id (contextvar)
    participant C as shared rerank client<br/>(one per process, all banks)
    participant P as remote provider
    R->>CV: bind bank_id
    R->>C: predict(query, docs)
    C->>C: bank_attribution_headers()<br/>flag on + bank bound → {X-Hindsight-Bank-Id}
    C->>P: POST /rerank (per-request headers,<br/>never client defaults)
    Note over C,P: bank A and bank B requests on the same client<br/>each carry their own id — no bleed
Loading

Design notes

  • Per-request headers, not client defaults — the client is shared across banks; defaults would misattribute. A dedicated two-bank isolation test proves requests interleaved on the same client carry distinct ids.
  • Flag is static (server-level), not bank-configurable — it's an infrastructure/billing concern.
  • Header and flag names match the contract proposed in the issue exactly. The issue's open question about an X-Bank-Id spelling for MCP surfaces is left to maintainers — trivial rename if preferred.
  • Reranks issued during reflect currently run with no bank bound; upstream fix(reflect): preserve bank attribution in provider calls #2764 fixes that binding orthogonally, and this feature picks it up automatically once merged.
  • Non-ASCII bank ids are skipped (no header) rather than percent-encoded: httpx requires ASCII header values, and an encoded id wouldn't match billing dashboards. Covered by a dedicated test — a unicode bank id can never 500 a recall.

How this differs from #2740

Strictly additive and scoped to the issue's stated minimum plus the safe LiteLLM proxy path: no existing attribution/LiteLLM tests are deleted, the LiteLLM legacy response fallback is untouched, and the unrelated reflect-binding change stays in #2764. The LiteLLM SDK path is deliberately left out: passing headers= into litellm.arerank unconditionally is unverified against litellm's rerank signature (it may break SDK reranks even with the flag off). Tests here exercise the real env flag + contextvar end-to-end through httpx.MockTransport — including absent-by-default, absent-when-no-bank, auth-header survival, and two-bank per-request isolation — rather than patching the header helper.

Out of scope (follow-ups)

Native Cohere SDK, LiteLLM SDK (pending kwarg verification), Google, TEI (self-hosted; attribution value low), local/FlashRank/JinaMLX/RRF (no remote spend to attribute).

How to test

cd hindsight-api-slim
uv run pytest tests/test_reranker_bank_attribution.py tests/test_bank_attribution_config.py -v
uv run pytest tests/test_cohere_cross_encoder.py tests/test_bank_attribution.py \
              tests/test_tei_cross_encoder.py tests/test_reranker_timeouts.py -q
uv run ty check hindsight_api/engine/bank_attribution.py hindsight_api/engine/cross_encoder.py
cd ../hindsight-embed && uv run pytest tests/test_env_template.py -q

Test evidence

Run on the de-stacked, squashed branch (single commit on upstream main):

Check Result
attribution + cohere + tei + timeouts suites (6 files) 72 passed, 3 skipped
hindsight-embed/tests/test_env_template.py 7 passed
ruff check / ruff format --check on touched files clean
ty check on bank_attribution.py + cross_encoder.py clean
generate-docs-skill.sh idempotency clean (no drift)

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.

feat(api): opt-in per-bank attribution for remote reranker requests

1 participant