Skip to content

feat: make ADR topK limit configurable via analysis.max_relevant_adrs - #193

Merged
Tgenz1213 merged 1 commit into
mainfrom
worktree-issue-189-topk-config
Sep 17, 2026
Merged

Tgenz1213 merged 1 commit into
mainfrom
worktree-issue-189-topk-config

Conversation

@Tgenz1213

Copy link
Copy Markdown
Owner

Summary

  • Replaces the hardcoded const topKADRs = 3 in Engine.Run with a new analysis.max_relevant_adrs config field (config.Analysis.MaxRelevantADRs, plain int), following the same <= 0-falls-back-to-default pattern already used by analysis.max_concurrency — a topK of 0 or negative has no legitimate meaning, so the *int nil-default pattern used elsewhere in VectorStore isn't needed here.
  • Engine.Run now reads this value once per run and passes it to both Store.Search (non-debug path) and Store.SearchWithDebugInfo (--debug path). No interface changes were needed in internal/index since both already took topK as a parameter.
  • Adds docs/arch/0020-configurable-topk-adr-limit.md documenting the decision, including confirming (not changing) that PgStore.Search's MaxSearchCandidates (1000) internal cap still comfortably exceeds any reasonable configured topK.
  • Updates CLAUDE.md's internal/analysis section to describe the config field instead of citing "top-3" as a fixed fact.

Deviations from the issue

None — implemented exactly per the acceptance criteria in #189.

Test plan

  • internal/analysis/analysis_test.go: TestRun_MaxRelevantADRs_RaisesLimitAboveDefault — a 4-ADR fixture (all passing scope + threshold via identical embeddings) confirms all 4 reach the LLM-judgment step when max_relevant_adrs: 4.
  • TestRun_MaxRelevantADRs_DefaultsToThreeWhenUnsetOrNonPositive — confirms both 0 and -1 fall back to the default of 3 (regression test pinning old behavior).
  • go build ./..., go test ./... (full suite, including internal/index's Docker-backed pgvector integration tests) — all pass.
  • golangci-lint run --timeout=5m — clean. Run via Docker (golangci/golangci-lint:v2.12.2) since the local Go toolchain (1.27.0) is newer than the installed golangci-lint's build (1.26.5), causing a panic locally — unrelated to this change, a pre-existing environment mismatch.
  • Independent subagent review of the full branch diff against main — no correctness issues found; confirmed the test fixtures genuinely make 4 ADRs qualify as candidates rather than trivially passing.

Closes #189

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings September 17, 2026 03:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Newly added documentation/comments inaccurately describe Engine.Run as passing topK to SearchRejected even though it uses SearchWithDebugInfo in debug mode.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR makes the per-file “relevant ADRs” top‑K limit configurable via analysis.max_relevant_adrs, replacing the previously hardcoded topKADRs = 3 in Engine.Run and documenting the decision for future maintenance.

Changes:

  • Add config.Analysis.MaxRelevantADRs (yaml:"max_relevant_adrs") and apply it in Engine.Run with a <= 0 fallback to the existing default of 3.
  • Update the analysis pipeline to pass the configured topK to VectorStore.Search (non-debug) and VectorStore.SearchWithDebugInfo (debug).
  • Add an ADR documenting the change and add regression tests covering both “raised limit” and “defaulting to 3”.
File summaries
File Description
internal/config/config.go Adds MaxRelevantADRs to config schema (new YAML key).
internal/analysis/engine.go Reads configured topK once per run and uses it for search calls.
internal/analysis/analysis_test.go Adds tests proving topK can be raised and that <= 0 defaults to 3.
docs/arch/0020-configurable-topk-adr-limit.md New ADR documenting rationale/behavior of configurable topK.
CLAUDE.md Updates repository guidance to describe configurable topK rather than fixed “top‑3”.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/arch/0020-configurable-topk-adr-limit.md Outdated
Comment thread internal/config/config.go
@Tgenz1213
Tgenz1213 force-pushed the worktree-issue-189-topk-config branch from 46a340e to f8e67ea Compare September 17, 2026 03:34
Replaces the hardcoded topKADRs=3 constant in Engine.Run with a config
field following the same <= 0 fallback pattern as max_concurrency, so
a corpus with legitimately more than 3 relevant ADRs per file can
raise the limit without a custom build.

Closes #189

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Tgenz1213
Tgenz1213 force-pushed the worktree-issue-189-topk-config branch from f8e67ea to d65cd2f Compare September 17, 2026 03:35
@Tgenz1213
Tgenz1213 merged commit f8d5a0d into main Sep 17, 2026
7 checks passed
@Tgenz1213
Tgenz1213 deleted the worktree-issue-189-topk-config branch September 17, 2026 03:41
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: ADR top-K result limit is hardcoded to 3, not configurable

2 participants