feat(api): config-selectable default peer-similarity basis (#1469 verifier follow-up)

[stranske]

Bounded closer follow-up for the one substantive gap in the post-merge verifier comparison on #1497 (source issue #1469). Does not close #1469 — the closer closes that issue after this lands and verifies.

Why

The #1497 provider comparison split: anthropic/claude-opus-4-6 PASS (82%), openai/gpt-5.4 CONCERNS (86%, completeness 6/10). Auditing every openai concern against merged main (872569b):

Concern	Audit result
No config-driven basis selection; requirement said basis selectable via config with Jaccard default	Valid. rg -n basis etl/manager_similarity_flow.py returns nothing and there is no env/config knob anywhere; the only selector was basis: str = Query("jaccard", ...) on the endpoint. Fixed here.
Unclear that a crowding alert fires end-to-end	False positive (truncated diff). etl/conviction_flow.py:726-747 loads crowded_trades rows, derives similar_manager_count under SIMILARITY_CROWDING_MIN_SCORE, and emits AlertEvent(event_type="crowded_trade_change", ...); alerts/engine.py:86 evaluates the similar_manager_count_gte condition against that payload. It reuses alert_rules/crowded_trades exactly as the issue's Non-Goals require.
No off-by-one crowding-threshold regression test	False positive. tests/test_alert_engine.py:130-169 asserts the inclusive boundary fires (threshold 3, count 3 → "Crowding Rule") and that a fractional threshold does not fire spuriously (threshold 3.9, count 3 → still only "Crowding Rule"). An off-by-one in either direction breaks one of those two assertions.
Cosine-only migration may leave row/backfill ambiguity	Not a defect. The issue explicitly allowed "a basis column or a score pair"; the flow writes both scores on the same row (etl/manager_similarity_flow.py:90-96), cosine is nullable, and the endpoint skips rows whose selected score is NULL or non-finite.
Fetch-all-then-filter is less efficient than limiting in SQL	Real but a performance nit on a bounded per-manager peer set; the Python filter is what rejects non-finite scores, which was itself a CodeRabbit fix on #1497. Not tracked here.

Also cross-checked the first half of acceptance criterion 1: the hand-computed cosine assertion exists — tests/test_manager_similarity_flow.py:43 asserts cosine_similarity([1.0, 1.0], [1.0, 0.0]) == pytest.approx(1 / 2**0.5).

So one real gap remained: "basis is config-selectable (Jaccard remains default)."

Change

• MANAGER_SIMILARITY_DEFAULT_BASIS (documented in .env.example) now supplies the default basis for GET /managers/{id}/similar.
• The query parameter becomes optional (basis: str | None = Query(None, pattern="^(jaccard|cosine)$")); an explicit value still wins over configuration, and an invalid value is still rejected with 400 by the same pattern.
• The value is read per request (no import-time module state), lower-cased/trimmed, and an unrecognized setting logs a warning and falls back to jaccard — Jaccard remains the default in every unconfigured and misconfigured case.

Test gate

pytest -o addopts= tests/test_manager_similarity_flow.py tests/test_env_example_coverage.py = 7 passed, including the new test_similar_manager_default_basis_is_config_selectable, which covers: unset env → jaccard; MANAGER_SIMILARITY_DEFAULT_BASIS=cosine → cosine ordering (manager_id 2, score 0.8); explicit basis=jaccard overriding a cosine config; and =euclidean → warn + jaccard.

Deliberate-break demonstration: replacing basis = basis or _configured_similarity_basis() with basis = basis or "jaccard" fails the new gate —
FAILED tests/test_manager_similarity_flow.py::test_similar_manager_default_basis_is_config_selectable, AssertionError: assert 'jaccard' == 'cosine' (1 failed, 4 passed). The break was reverted and the suite re-run green.

Wider check: pytest -o addopts= tests/test_manager_similarity_flow.py tests/test_env_example_coverage.py tests/test_signals_api.py tests/test_alert_engine.py tests/test_crowded_contrarian.py tests/test_conviction_flow.py = 45 passed, 1 failed — the single failure is test_conviction_flow.py::test_conviction_deployment_nightly_utc_schedule, the pre-existing cross-file reload leak on main that PR #1499 fixes (it fails on main too and is unrelated to this change). ruff check clean; black --check clean; git diff --check clean.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 32 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 73a6fbf6-531f-4820-a2b5-0c95db2d6528

📥 Commits

Reviewing files that changed from the base of the PR and between 872569b and 8e05194.

📒 Files selected for processing (3)

• .env.example
• api/managers.py
• tests/test_manager_similarity_flow.py

---

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

[stranske]

Closer note: CodeRabbit posted "Review limit reached — next review available in 40 minutes" (plus an organization usage spending cap) at 14:51Z, so this PR has no substantive automated review yet. It is intentionally left open rather than merged on green gates alone, because it changes a production request path (GET /managers/{id}/similar default basis). Next closer pass: trigger @coderabbitai review, then merge on clean review + green checks, apply verify:compare, and close source issue #1469 once the verifier PASSes.

[coderabbitai]

You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 21 minutes.

[github-actions]

Provider Comparison Report

Provider Summary

Provider	Model	Verdict	Confidence	Summary
openai	gpt-5.6-terra	PASS	93%	The implementation fulfills the verifier follow-up acceptance criteria. .env.example documents MANAGER_SIMILARITY_DEFAULT_BASIS=jaccard; the managers similarity endpoint makes basis optional...
anthropic	claude-sonnet-5	PASS	60%	The PR adds MANAGER_SIMILARITY_DEFAULT_BASIS to .env.example and schema/test files consistent with the goal of making peer-similarity basis config-selectable while defaulting to Jaccard, closing th...

📋 Full Provider Details (click to expand)

openai

• Model: gpt-5.6-terra
• Verdict: PASS
• Confidence: 93%
• Scores:
  • Correctness: 9.0/10
  • Completeness: 10.0/10
  • Quality: 9.0/10
  • Testing: 9.0/10
  • Risks: 9.0/10
• Summary: The implementation fulfills the verifier follow-up acceptance criteria. .env.example documents MANAGER_SIMILARITY_DEFAULT_BASIS=jaccard; the managers similarity endpoint makes basis optional and resolves the effective basis per request from the environment when no query value is supplied. Explicit query values retain precedence and the existing constrained query pattern continues to reject unsupported values. Configuration normalization trims and lowercases the environment value, while unsupported configuration emits a warning and safely falls back to Jaccard. The implementation avoids import-time configuration state. Updated similarity/API tests cover configuration-driven selection, explicit override behavior, normalization/fallback behavior, and preserve the established cosine and crowding coverage. The changes are localized, readable, and introduce no meaningful compatibility, security, or performance concern.

anthropic

• Model: claude-sonnet-5
• Verdict: PASS
• Confidence: 60%
• Scores:
  • Correctness: 7.0/10
  • Completeness: 7.0/10
  • Quality: 6.0/10
  • Testing: 7.0/10
  • Risks: 6.0/10
• Summary: The PR adds MANAGER_SIMILARITY_DEFAULT_BASIS to .env.example and schema/test files consistent with the goal of making peer-similarity basis config-selectable while defaulting to Jaccard, closing the one real gap identified in the prior audit ([Audit] Peer similarity: embedding-cosine basis + crowding alerts #1469 follow-up). CI (ci.yml, pr-00-gate.yml) passed on the merge commit. However, the actual api/managers.py diff implementing the query-parameter/config precedence logic was truncated in the review context, limiting direct verification of some acceptance criteria nuances (per-request evaluation, lower-casing/trimming, warn-and-fallback on invalid config). The PR also includes substantial unrelated changes (short_interest adapter/ETL/migration, llm/client.py) that are out of scope for the stated feature, which is a code-hygiene/process concern but does not itself indicate the core feature is broken. Given CI success, the presence of expanded relevant tests, and the .env.example addition matching the required env var name, the implementation appears to satisfy the acceptance criteria at a functional level, with some documentation/verification caveats. Overall PASS with moderate confidence due to incomplete visibility into the core api/managers.py diff and scope-creep concerns unrelated to correctness of the target feature.
• Concerns:
  • The full diff for api/managers.py (where the MANAGER_SIMILARITY_DEFAULT_BASIS logic should live) was truncated in the provided context, so the exact implementation details (per-request read, lower-casing/trimming, warning-and-fallback behavior) could not be directly verified from the diff text — only inferred from the .env.example addition and the PR's stated intent.
  • The PR bundles unrelated, sizable changes (short_interest adapter/flow/migration, llm/client.py, config/llm_slots.json) alongside the stated 'config-selectable peer-similarity basis' scope. This is scope creep that complicates review and increases risk surface, even though CI passed.
  • Task/acceptance checkboxes in the PR body remain unchecked, which is either a documentation oversight or a signal that the author didn't mark completion — this is a minor process concern given CI succeeded and file changes align with the stated fix.
  • tests/test_manager_similarity_flow.py was expanded (+48/-4), which is consistent with testing the new config-driven default basis, but the specific assertions (invalid-value fallback logging, case-insensitivity, per-request evaluation) are not visible in the truncated diff to confirm all three acceptance criteria bullets are tested.

Agreement

• Verdict: PASS (all providers)

Disagreement

Dimension	openai	anthropic
Correctness	9.0/10	7.0/10
Completeness	10.0/10	7.0/10
Quality	9.0/10	6.0/10
Testing	9.0/10	7.0/10
Risks	9.0/10	6.0/10

Unique Insights

• openai: The implementation fulfills the verifier follow-up acceptance criteria. .env.example documents MANAGER_SIMILARITY_DEFAULT_BASIS=jaccard; the managers similarity endpoint makes basis optional and resolves the effective basis per request from the environment when no query value is supplied. E...
• anthropic: The full diff for api/managers.py (where the MANAGER_SIMILARITY_DEFAULT_BASIS logic should live) was truncated in the provided context, so the exact implementation details (per-request read, lower-casing/trimming, warning-and-fallback behavior) could not be directly verified from the diff text — only inferred from the .env.example addition and the PR's stated intent.; The PR bundles unrelated, sizable changes (short_interest adapter/flow/migration, llm/client.py, config/llm_slots.json) alongside the stated 'config-selectable peer-similarity basis' scope. This is scope creep that complicates review and increases risk surface, even though CI passed.; Task/acceptance checkboxes in the PR body remain unchecked, which is either a documentation oversight or a signal that the author didn't mark completion — this is a minor process concern given CI succeeded and file changes align with the stated fix.; tests/test_manager_similarity_flow.py was expanded (+48/-4), which is consistent with testing the new config-driven default basis, but the specific assertions (invalid-value fallback logging, case-insensitivity, per-request evaluation) are not visible in the truncated diff to confirm all three acceptance criteria bullets are tested.

🔍 LangSmith Traces

• openai
• anthropic

[github-actions]

Workflow state fingerprint for Agents Verifier. Do not edit.