Skip to content

feat: add canonical search targets without replacing existing results - #146

Merged
ayhammouda merged 1 commit into
mainfrom
agent/63-canonical-spare-slot
Oct 5, 2026
Merged

ayhammouda merged 1 commit into
mainfrom
agent/63-canonical-spare-slot

Conversation

@ayhammouda

@ayhammouda ayhammouda commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Vision — automated project maintainer

The additive spare-slot implementation is published at e8dd02f2b167c8df09c623bc588a21aaa9f6b644, based on verified main 49d307b814d254961ee7a7392e4620aebd87d2e8 after security PR #145. Its tree 77a3dd299cbb4d50e8e4a2e402161911e0fd0751 is byte-identical to the owner-reviewed implementation. Only the search service, new deterministic tests and README below the hero change.

Fresh developer evidence: 596 tests, canonical locked gates, doctor, frozen corpus, build and 4 installed-wheel stdio tests passed. Same retained 1,603-document index: 34→35/65 hits, gain only EX-015/3.13 (tempfile.TemporaryDirectory), zero losses, 69 citations unchanged. Original result prefixes/notes and 11 semantic controls are preserved. Six-tool schemas match base/source/installed wheel. Owner static review and raw JSON/source/wheel hashes confirm scope and evidence; no contributed code ran in the owner account.

Append-only admission adds at most one exact canonical target to explicit-version auto results with spare slots, if the complete proposed response fits 8,000 UTF-8 bytes. Oversized originals stay unchanged; this is not a global cap. Guards cover ambiguity, unknown identifiers, other kinds, versionless/full results, aliases, Unicode/escaped exact-fit and one-byte-over budgets. Empty-only fallback is preserved.

Published for review; not merged or released. Retained-index/Linux3.12/SDK-stdio evidence is not a fresh independent rebuild, named-client qualification or answer-quality claim. Initial failed lint/probe outputs remain recorded. The broker completed mandatory independent prepublication review successfully on 2026-10-05 after the decision was repaired to embed the owner review and all 11 raw semantic controls. Exact-head PR verification, hosted CI/review gates and SHA-matched merge remain pending. Detailed independent replay evidence will be attached by the exact-head verification step; the developer measurements above are not being relabeled as independently verified results. Installed owner timing requires a fresh full-cycle window before starting verification. Outcome review 2026-10-17; reject if the independent rebuild fails to reproduce the gain or loses semantic/version/citation evidence.

Refs #63.

  • Scoped implementation, developer gates and owner static review.
  • Broker independent prepublication review approved; detailed exact-head verification evidence pending.
  • Exact published-head verification, required CI/review triage.
  • Gated merge and post-merge checks.

No forbidden paths or public schema changed. No release claim.

Summary by CodeRabbit

  • Search
    • Automatic searches for a specific, recognized dotted symbol can now include its exact match alongside other results when space allows.
    • Existing results remain in their original order, and duplicate matches are avoided. Results that would exceed the response size limit are left unchanged.

@coderabbiteu

coderabbiteu Bot commented Oct 5, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Repository: ayhammouda/python-docs-mcp-server/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d28e0762-3271-4b11-b9d5-1e714df48729
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: ayhammouda/python-docs-mcp-server/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4b9e9e17-0343-4218-a39e-3376cc372c25
📥 Commits

Reviewing files that changed from the base of the PR and between 49d307b and e8dd02f.

⛔ Files ignored due to path filters (1)
  • README.md is excluded by none and included by none
📒 Files selected for processing (2)
  • src/mcp_server_python_docs/services/search.py
  • tests/test_search_spare_slot.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

SearchService can append one exact-title canonical symbol hit to eligible auto search results. Admission requires a single version-known dotted identifier, available result capacity, no duplicate location, and a serialized response within 8,000 bytes.

Changes

Canonical symbol search

Layer / File(s) Summary
Identifier checks and canonical admission
src/mcp_server_python_docs/services/search.py
SearchService identifies dotted identifiers known to the requested version. It appends a matching exact-title symbol hit only when eligibility, duplicate-location, and serialized-size checks pass. The prompt-only fallback reuses the identifier lookup.
Search result paths and validation
src/mcp_server_python_docs/services/search.py, tests/test_search_spare_slot.py
Exact symbol and FTS results now pass through canonical admission. Tests cover eligible and ineligible queries, result preservation, deduplication, requested versions, and the 8,000-byte limit.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant SearchService
  participant FTSIndex
  participant SymbolLookup
  Caller->>SearchService: auto search query and version
  SearchService->>FTSIndex: retrieve FTS results
  FTSIndex-->>SearchService: result hits
  SearchService->>SymbolLookup: find known identifier and exact symbol
  SymbolLookup-->>SearchService: candidate symbol
  SearchService-->>Caller: original results with candidate, or unchanged results
Loading

Merge Risk: ⚪ Minimal · up to e8dd0

Eligible automatic searches may now append one exact canonical symbol hit, leaving existing results unchanged. No concrete merge-blocking risk was identified. Exact-head CI and independent rebuild verification remain pending per the PR description.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to e8dd0

The additional target remains within the requested documentation version and existing input limits. Admission preserves existing results and adds no writes, external requests, or privileges. No material security risk was identified in the compared change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A caller can influence canonical admission through the existing query, version, kind, and result-limit inputs. The incremental exposure is at most one documentation symbol in an eligible response, within the requested version. The changed path introduces no credential access, persistent writes, remote requests, or executable tool authority.

Trust Boundaries and Controls

  • observed — Caller-controlled identifiers remain lookup inputs rather than SQL syntax or executable instructions. Explicit versions are validated before search, and both recognition and candidate lookup constrain results to that version. The existing cross-version classifier does not relax the admission filter.

Resilience and Maintainability Implications

  • observed — Admission constructs a separate hit list and does not modify the original response or shared corpus. Rejected admission returns the original object. Unexpected failures propagate to the existing handler, which returns a tool error rather than a partially emitted result; there is no new persisted partial state to recover.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding canonical search targets without replacing existing results.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@ayhammouda

Copy link
Copy Markdown
Owner Author

Vision — automated project maintainer

Independent verification (temporary credential): success

Head: e8dd02f2b167c8df09c623bc588a21aaa9f6b644; main: 49d307b814d254961ee7a7392e4620aebd87d2e8.

{
  "head_sha": "e8dd02f2b167c8df09c623bc588a21aaa9f6b644",
  "base_sha": "49d307b814d254961ee7a7392e4620aebd87d2e8",
  "approved": true,
  "summary": "Independent prepublication review passed. The exact head has the stated parent and tree; the full diff changes only search behavior, its tests, and README. A fresh pinned-source build produced a 1,603-document index. On that same index, frozen retrieval improved from 34/65 to 35/65 solely through EX-015, with no lost hits; 69 citations resolved. Original hits and notes across all 65 cases and all 11 semantic controls were preserved. Locked checks, targeted tests, index validation, wheel build, and isolated installed-wheel stdio smoke passed. One pytest test was skipped. GitHub reports no hosted checks for this unpublished SHA; those remain pending for publication and merge.",
  "commands": [
    {
      "command": "git clone --depth=1 https://github.com/ayhammouda/python-docs-mcp-server.git repo",
      "exit_code": 0
    },
    {
      "command": "git fetch --depth=1 origin e8dd02f2b167c8df09c623bc588a21aaa9f6b644 49d307b814d254961ee7a7392e4620aebd87d2e8",
      "exit_code": 0
    },
    {
      "command": "git diff --check 49d307b814d254961ee7a7392e4620aebd87d2e8 HEAD",
      "exit_code": 0
    },
    {
      "command": "uv sync --locked --dev",
      "exit_code": 0
    },
    {
      "command": "uv run --locked ruff check src/ tests/ benchmarks/ ops/ .github/scripts/",
      "exit_code": 0
    },
    {
      "command": "uv run --locked pyright src/ benchmarks/",
      "exit_code": 0
    },
    {
      "command": "uv run --locked pytest --tb=short -q",
      "exit_code": 0
    },
    {
      "command": "uv run --locked python -m benchmarks validate-corpus --corpus docs/benchmarks/corpus.yml --schema docs/benchmarks/corpus.schema.json",
      "exit_code": 0
    },
    {
      "command": "uv build",
      "exit_code": 0
    },
    {
      "command": "uv run --locked pytest tests/test_stdio_smoke.py -q",
      "exit_code": 0
    },
    {
      "command": "isolated installed-wheel pytest --import-mode=importlib pd-verifier-installed-smoke.py -q",
      "exit_code": 0
    },
    {
      "command": "uv run --locked python-docs-mcp-server build-index --versions 3.11,3.12,3.13",
      "exit_code": 0
    },
    {
      "command": "uv run --locked python-docs-mcp-server doctor",
      "exit_code": 0
    },
    {
      "command": "uv run --locked python-docs-mcp-server validate-corpus",
      "exit_code": 0
    },
    {
      "command": "exact-base raw replay of 65 frozen cases and 11 controls",
      "exit_code": 0
    },
    {
      "command": "exact-head raw replay of 65 frozen cases and 11 controls",
      "exit_code": 0
    },
    {
      "command": "uv run --locked python -m benchmarks.regression --output /tmp/pd-regression-head.json",
      "exit_code": 0
    },
    {
      "command": "independent raw-result and frozen-hit comparison",
      "exit_code": 0
    },
    {
      "command": "uv run --locked pytest tests/test_search_spare_slot.py -q",
      "exit_code": 0
    },
    {
      "command": "GitHub check-runs API query for exact head",
      "exit_code": 0
    }
  ],
  "blockers": []
}

@ayhammouda

Copy link
Copy Markdown
Owner Author

Vision — automated project maintainer

Merge decision for exact head e8dd02f2b167c8df09c623bc588a21aaa9f6b644 against main 49d307b814d254961ee7a7392e4620aebd87d2e8: independent broker verification succeeded (receipt-backed; evidence in the preceding verification comment), all 11 exact-head hosted checks passed, and pdctl threads 146 returned no unresolved conversations.

Independent fresh pinned-source evidence reproduces 35/65 frozen retrieval hits versus 34/65, solely EX-015, with no losses, 69 resolved citations, all original response prefixes/notes and all 11 semantic controls preserved. Locked gates, wheel and installed-stdio checks passed; one full-suite test was skipped. The verification record reuses the exact prepublication head/base review, so its historical statement that hosted checks were pending is superseded by today's live 11/11 success check.

Review triage: CodeRabbit's substantive review reports no actionable findings. Its 26.67% docstring-coverage advisory is nonblocking: it is not a required repository gate, production helper contracts are documented, and most newly counted functions are descriptive tests. No gate or threshold is changed. README was excluded by CodeRabbit but covered by owner and independent review.

Proceeding only through SHA-matched pdctl merge; post-merge CI remains to be confirmed. This is narrow canonical-target retrieval evidence, not generated-answer accuracy or a release announcement. Outcome review: 2026-10-17; revisit/remove if real feedback identifies distractors or reproduced regressions.

@ayhammouda
ayhammouda merged commit 66bd44d into main Oct 5, 2026
12 checks passed
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.

1 participant