PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The diff is a two-file config-leaf addition with its contract spec pinned in the same PR, and every load-bearing claim in the body is verifiable at source — I verified them rather than accepting them. Nothing here is debt: the header comment is updated so the file's own documentation stays true, and the four-entry reality is pinned in KbTenantBootstrapContract.spec.mjs rather than left to drift. Approve+Follow-Up would be inventing a residual where the PR already names its own (AC-1..AC-5 → #17621, correctly, because they are live-deployment receipts unreachable from this head).

Peer-Review Opening: Careful work, and the part I want to name is the precondition discipline — you did not just add the entry, you verified at the remote that the hazard which blocked this lane is actually gone before claiming it. I re-ran that check independently and it holds. This is the shape a config-leaf PR should have.

Note on form: this is a Cycle-1 full review rather than a micro-review despite the 20/5 diff. ai/deploy/kb-config.yaml is ai/ config and a consumed contract, which guide §6.4 excludes from the light path "regardless of size". Recording that so the form does not read as inflation.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Ticket #17377 (body + Contract Ledger + labels), the changed-file list, ai/services/knowledge-base/source/RawRepoSource.mjs on current dev, the three existing tenant entries as sibling precedent, and the neomjs/devindex remote itself as the source of authority for every precondition the entry depends on.
• Expected Solution Shape: One tenantRepos[] entry matching the shape of the three existing ones, plus whatever tracked contract pins the entry count. It must NOT hardcode a repo-specific path, credential, or branch outside the entry, and the bootstrap contract spec must move with it in the same PR — a config change that leaves its own contract spec red is the failure mode this file's spec exists to catch.
• Patch Verdict: Matches. The entry is field-for-field parallel to devindex-opt-out (kb-config.yaml:63-67), and KbTenantBootstrapContract.spec.mjs is updated in the same diff across all four assertions that encode the count — length, key list, and bySlug branch pinning. No fourth assertion was missed: I grepped ai, test and learn for any other pin on a three-repo count and found only generic tenantRepos[] references, none count-bearing.
• Premise Coherence: Coheres with verify-before-assert, and unusually literally. The body's claims are not framing — they are checkable, and each one resolved to the remote rather than to the PR's own description. The tenant's stated purpose ("content a query can actually be WRONG about") is itself a verify-before-assert argument: the three existing entries cannot fail a retrieval assertion, so they cannot falsify the ingestion path.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17377
• Related Graph Nodes: #17621 (residual owner, live receipts) · #17375 (blocker, discharged at source) · #16278
• Origin Session ID: eb671e6e-ca17-4a53-8069-64fd5885ce84

🔬 Depth Floor

• Documented search: I actively looked for (1) any other surface pinning the tenant count at three that this PR would leave stale — ai, test, learn across .mjs/.md/.yaml/.yml, all hits generic tenantRepos[], none count-bearing; (2) whether the "whole-tree ingestion became safe by construction" precondition actually holds at the remote rather than only in the comment — apps/devindex/resources/data returns zero entries in the recursive tree at main, and .gitignore excludes /apps/devindex/resources/data/ with the whole working set, so it is gitignored at source, not merely deleted once; (3) whether whole-tree ingestion would pull bulk non-prose into a tenant whose entire purpose is retrieval quality.

• Challenge (non-blocking, worth watching at AC-3 rather than here): (3) came back cleaner than I expected — package-lock.json is 354KB and the single largest blob in the repo, and I went looking for it as noise. It is excluded, but by DEFAULT_EXCLUDE_PATHS, not by the DEFAULT_EXCLUDE_EXTENSIONS your body cites at RawRepoSource.mjs:6. Your claim about that constant is exactly right — it is binary/media-only — the protection just comes from the sibling constant below it. Worth knowing which one is load-bearing if this ever needs tuning.

  What does survive ingestion is a small set of navigation manifests: learn/tree.json (3.5KB) and docs/examples.json (9KB). These list guide titles and topics as data. AC-3 asserts that "a devindex-guide-only semantic query returns a chunk sourced from this tenant's learn/**" — and learn/tree.json is under learn/** while containing no prose. A query for a guide topic could match the manifest that names the topic instead of the guide that explains it, and AC-3 as worded would still read as satisfied. Suggest #17621's step-4 receipt records which chunk matched, not only that one did. Not a change to this PR.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates (no overshoot) — "first real-content tenant" is supported: 26 learn/** markdown files at the remote, which I counted, matching the body exactly
• Anchor & Echo summaries: precise codebase terminology, no metaphor or source-code snapshot anchor that overshoots durable intent — the new header block explains why this tenant differs in retrieval terms and carries no ticket refs, correctly
• [RETROSPECTIVE] tag: N/A — none claimed
• Linked anchors: cited tickets/PRs actually establish the claimed pattern — #17375 is cited as discharged-at-source and the remote 404 confirms it; #17621 is a real open ticket and a valid residual owner, not the close target

Findings: Pass. The body under-claims if anything — see the exclusion-constant note above.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The header comment carries the reasoning for why the Neo repo is deliberately not registered here and why this fourth entry is different in kind from the first three. That is the property that makes a config file survive its author: the next agent to touch kb-config.yaml inherits the argument, not just the rows. Worth keeping as the pattern for tenant additions.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI/MCP tool surface is touched, and the PR introduces no cross-substrate convention, skill change, or new pattern requiring predecessor-step updates — it adds one row to an existing, already-documented tenant list.

🎯 Close-Target Audit

• Close-targets identified: #17377
• For each #N: confirmed not epic-labeled — #17377 carries enhancement, ai, architecture, agent-os

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket (or parent epic) contains a Contract Ledger matrix — #17377 §"Contract Ledger Matrix"
• Implemented PR diff matches the Contract Ledger exactly (no drift) — the entry ships credentialRef: none and branchRef: main as prescribed; I confirmed main is genuinely the remote's default_branch rather than an assumed default

Findings: Pass.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line
• Achieved evidence ≥ close-target required evidence, OR residuals are explicitly listed — AC-1..AC-5 are listed with a ## Post-Merge Validation section
• If residuals exist: residual owner named — #17621, an existing open ticket that is not the close target
• Two-ceiling distinction: the body distinguishes shipped-at-L2-because-ceiling explicitly — "the running orchestrator reads the deploy home's mounted copy, not this tree" is a real ceiling statement, not an unprobed one
• Evidence-class collapse check: this review does not promote the L2 config-parse receipt to L3 — the tenant is registered here and ingests only after the deploy-home update
• Deployment causality: correctly handled — no runtime receipt is used as a merge gate; all five are Post-Merge Validation on #17621

Findings: Pass. The L2/L3 split is drawn in the right place: nothing in this diff can produce a sync-cycle receipt from an unmerged head, and the PR does not pretend otherwise.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 8723efb692c118d98024d5f05e8c5dad07d309ca (gh pr checks exit 0), plus the author's js-yaml parse receipt through the production normalizeTenantRepoEntry
• Reviewer falsifier: ran one — the body's whole-tree-safety precondition. repos/neomjs/devindex/git/trees/main?recursive=1 filtered on resources/data returns 0 entries, and .gitignore excludes the directory. The concern was that a purge can scrub history while leaving the working tree populated, in which case a main clone would still carry the pipeline state; it does not, and the gitignore makes it safe going forward rather than only at this instant.
• Test location: pass — the contract pin lands in the existing KbTenantBootstrapContract.spec.mjs alongside the assertions it extends, not in a new file

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 96 - Config leaf lands in the file that owns tenant bootstrap, with its tracked contract spec updated in the same change. No boundary crossed, no repo-specific logic leaked into the resolver.
• [CONTENT_COMPLETENESS]: 95 - Header comment updated so the file documents its own new reality; the count is pinned in all four places the spec encodes it. Nothing left stale — I searched for a fifth pin and there is none.
• [EXECUTION_QUALITY]: 95 - Every precondition was verified at the remote before the claim was written. The one imprecision is which constant excludes package-lock.json, and it is in the safe direction.
• [PRODUCTIVITY]: 92 - Two files, 20 additions, contract pinned, residuals correctly routed to an existing owner.
• [IMPACT]: 84 - First tenant whose content a retrieval assertion can actually fail against; the three existing entries could not falsify the ingestion path, so this is where the lane starts producing real signal.
• [COMPLEXITY]: 22 - Mechanically trivial; the work was in the verification, not the diff.
• [EFFORT_PROFILE]: Quick Win - Small diff, real unblock, evidence correctly bounded.

The thing I would keep from this one: you treated the ticket's precondition as a claim to re-check at the source rather than as a fact you inherited. #17375 was recorded as blocking, and instead of waiting on it or asserting it discharged, you went and looked at the remote. That is the whole discipline, on a PR small enough that skipping it would have gone unnoticed.

🖖 Grace (Claude Opus 5, Claude Code)