[laboef1900]

Fixes #1798

Part of #1827.

Change

• Resolve duplicate sources by standalone internal ID or Confluence content ID, under the same caller-visible page predicate used by the full scan and candidates. Historical standalone Confluence IDs are not source identifiers.
• Missing, deleted, inaccessible and visibly ambiguous sources return the same empty result; invisible collisions cannot change the answer. No UI or full-scan algorithm changes.

Evidence

• Reproduced before fix with real PostgreSQL: standalone source returned [] while the scan found neighbors (baseline-reproduced.log).
• Focused backend regressions: 4 files, 50 tests passed, including source/candidate visibility and identifier collisions.
• Actual authenticated HTTP smoke against isolated real PostgreSQL and Redis under env -i, with provider database rows restricted to loopback: standalone internal IDs (including historical metadata) and Confluence content ID matched full-scan neighbors; inaccessible private and space rows were excluded. Synthetic vectors exercise selection/access, not retrieval quality. No production model traffic required.
• Node 24.22.1. Parent serialized global gates on stable source: lint, typecheck, build, full npm test and high audit passed.

Risk / rollback

• Numeric IDs can collide across readable source types; the endpoint deliberately returns no results rather than choosing arbitrarily. Per-page best-chunk scoring and full-scan average scoring/caps remain different, so arbitrary corpora need not produce identical rankings or sets.
• Roll back the commit to restore previous lookup behavior (including the standalone defect). No migration or configuration change.

[laboef1900]

Independent Astra-high source/evidence review of published 41baf94: correct, no actionable findings (confidence0.97). Source-aware IDs and full-scan-equivalent visibility are preserved; readable collisions fail closed while hidden collisions cannot suppress visible results. Historical standalone Confluence IDs are not aliases. Reviewed executed baseline failure, focused50, all global gates, sanitized authenticated actual HTTP smoke with real PostgreSQL/Redis and controlled vectors, and published-source attestation. Earlier inherited-environment smoke does not establish absence of egress; corrected env-i proof supplies isolation evidence. Vectors prove source selection/access behavior, not retrieval quality. Audit high-severity gate passed with two moderate advisories retained. Read-only reviewer inspected evidence and did not execute checks. This is not GitHub approval; CI completion tracked separately.

[laboef1900]

Independent review: PR #1841

Issue: #1798 (bug; no NO AI label). Target: dev.

Head: 41baf947f0ca0b80c6ff157f1b7169087c041dd5
Base: 5edff9e08dad25e9db1c5668ff211c794fc9c2fd

Verdict

Correct; no confirmed patch-introduced actionable findings. Confidence: 0.97. Review disposition: COMMENT (read-only source/evidence assessment, not newly executed verification or GitHub approval).

Scope and boundary review

Full five-file diff; full detector and integration suite; HTTP route and shared visibility SQL. Checked source-to-candidate dispatch, numeric/string identifiers, restriction predicates, deletion and collision behavior. No algorithm or frontend change.

Acceptance criteria

• Source-aware standalone internal IDs and Confluence content IDs — FULLY ADDRESSED. duplicate-detector.ts:76-96 resolves canonical identifiers inside visiblePagesPredicate; historical standalone Confluence IDs are not aliases.

• Same source visibility as full scan; inaccessible candidates never returned — FULLY ADDRESSED. Source, candidate query and scanAllDuplicates use visiblePagesPredicate with caller spaces/user. Readable ambiguity returns []; hidden collisions cannot suppress visible rows.

Findings

None.

Evidence inspected (reused, not executed here)

• pr-1841-metadata.json: exact-head Typecheck & Lint, Tests, Playwright E2E and Semgrep SUCCESS; non-applicable jobs SKIPPED
• epic-1827-evidence/1798/sanitized-smoke.log: authenticated API internal/historical/Confluence IDs matched full-scan neighbors, inaccessible rows excluded
• epic-1827-evidence/1798/published-source-attestation.json: published SHA matches retained tested-source attestation

Evidence limits

Read-only source/evidence review; no new checks or services. Synthetic vectors prove selection/access, not retrieval quality or arbitrary ranking equivalence. Prior unsanitized environment run does not establish no egress.

Suggested targeted verification

Parent only, isolated real PostgreSQL/Redis: npx vitest run src/domains/knowledge/services/duplicate-detector.integration.test.ts from backend; include page-restriction duplicate regressions. No new suspected bug reproduction needed.

Cross-PR interaction

No conflicting behavior identified among this assigned group by source inspection.

No source edits, branch changes, services, GitHub reviews, pushes or merges performed.

Parent validation and publication

No additional actionable findings from parent triage.

Current final-head GitHub CI is passing. This is an independent source/evidence review, not a newly rerun full Docker/browser matrix or a GitHub approval. Source-only findings are identified as such. No code modifications or merges performed.

Reviewed by PR Reviewer Agent (eight independent review contexts; parent-validated publication).

[laboef1900]

Automated PR Review

PR: #1841 -- Fix duplicate detection for standalone sources
Branch: feature/1798-standalone-duplicates -> dev (reviewed at 41baf947)
Diff size: 140 lines (+63/-10)
Reviewed at: 2026-11-01 10:43 UTC

---

Static Checks

Check	Status	Details
TypeScript	PASS	npm run typecheck
Lint	PASS	npm run lint
Backend Tests	PASS*	1 failed
Frontend Tests	PASS	5799 passed (5799)
Production Build	PASS	npm run build
npm audit	FAIL	pre-existing: 11 advisories (10 moderate, 1 high systeminformation) in the shared lockfile; package-lock.json not changed by this PR
Branch Target	PASS	feature/* → dev

Docker App Test

Built from source (docker-compose.yml + docker-compose.build.yml, native arch) on an isolated project pr-review-1841; API reached through the nginx proxy (the backend publishes no host port by design).

Check	Status
Containers Healthy (postgres, redis, backend, frontend)	PASS
Migrations (fresh DB → Migrations complete, through 129)	PASS
Backend /api/health + /api/health/ready	PASS
Fresh-instance setup (POST /api/setup/admin → 201) + Login	PASS
API /api/pages, /api/search?q=…, /api/analytics/trending, /api/settings	PASS (200)
API /api/admin/settings, /api/admin/llm-providers, /api/notifications/count	PASS (200)
Frontend loads (GET / → 200)	PASS
No error/fatal logs (level ≥ 50)	PASS

PR-specific: created a standalone page (POST /api/pages → id 1, source: standalone) and called GET /api/pages/1/duplicates → 200 {"duplicates":[],"pageId":"1"}; unknown id → 200 [] (no embeddings in the review stack, so neighbours are empty by construction).

E2E Browser Tests

CI recipe (E2E_CI=1 npx playwright test: built backend + vite preview, fresh kb_e2e database, provisioning via e2e/helpers/ci-setup.ts).

Project	Status
e2e helper unit tests (vitest --config e2e/vitest.config.ts)	PASS
chromium + collab	PASS (68 passed (3.5m))

Code Review Findings

#	Severity	File:Line	Finding
1	WARNING	backend/src/domains/knowledge/services/duplicate-detector.ts:77-82	The new standalone arm compares cp.id::text = $1 against the raw request string, while every other per-page resolver in the repo (pages-crud.ts:1280-1286 GET /pages/:id, pages-embeddings.ts:288-293 local graph, knowledge-admin.ts:86-87, bulk-page-selection.ts:359) normalises all-digit identifiers with toPageIdText (core/utils/page-id-text.ts, #1167) before the id::text comparison. A zero-padded id such as GET /api/pages/007/duplicates therefore resolves page 7 on GET /api/pages/007 but returns duplicates: [] here, and because missing/inaccessible/ambiguous sources are deliberately indistinguishable the caller cannot tell the lookup silently failed. Fix: pass /^\d+$/.test(pageId) ? toPageIdText(pageId) : pageId to the id::text arm (keep the raw value for the confluence_id arm, as the sibling routes do).

Acceptance Criteria (Issue #1798)

AC	Criterion	Status	Evidence
AC-1	Resolve source identity with source-aware rules: a standalone article addressed by its internal ID is found by the per-page detector	PASS	duplicate-detector.ts:77-82 resolves (source='confluence' AND confluence_id=$1) OR (source='standalone' AND id::text=$1), matching the repo's canonical key (pages-crud.ts canonical_key CASE, page-relocate-service.ts:220 parentKeyFor, migration 111). Integration test duplicate-detector.integration.test.ts:88-108 seeds a shared standalone page (USER_B) and a private own standalone page and asserts findDuplicates(USER_A, String(id)) returns the same neighbours as scanAllDuplicates. Caveat: zero-padded ids are not normalised (Finding 1).
AC-2	Source scope uses the same visibility rules as the full scan (standalone shared / own-private pages participate; restricted pages excluded)	PASS	Old space_key = ANY($2) arm removed; source lookup now gated solely by visiblePagesPredicate(2, 3) (duplicate-detector.ts:81), the same fragment scanAllDuplicates uses at :245. Test :88-108 asserts per-page neighbours equal full-scan neighbours for all three visible sources; duplicate-detector.restriction.integration.test.ts:42-49 still covers restricted sources.
AC-3	Confluence IDs must continue to work	PASS	Confluence arm cp.source='confluence' AND cp.confluence_id=$1; test :97-102 iterates 'conf-source' and existing tests :128-240 ('src-1', 'src-trashed', 'secret-src') remain green on the confluence path (source column defaults to 'confluence' in fixtures).
AC-4	No leak of inaccessible candidate pages (or inaccessible sources)	PASS	Candidate kNN unchanged (visiblePagesPredicate(3, 4, 'cp2') at :168). Source: hidden private standalone (String(hidden)), hidden-space Confluence ('secret-source') and historical standalone confluence_id ('old-standalone-id') all return [] (:103-105). Collision test :110-118 proves a hidden colliding row neither changes the answer nor acts as an existence oracle, and a visible collision fails closed with [] (rows.length !== 1, :85-87).

Story Validation (Issue #1798)

Issue intent: GET /api/pages/:id/duplicates must find a standalone article by its internal ID (it previously required confluence_id plus an accessible Confluence space), resolving the source under the same source-aware visibility rules the full scan already uses, without leaking inaccessible pages.
Story completion: FULLY ADDRESSED
User journey: Before: calling the per-page duplicates endpoint with a standalone article's internal ID returned [] even though GET /api/admin/duplicates paired that article with neighbours (source lookup was confluence_id = $1 AND space_key = ANY(accessible)). -> After: standalone pages resolve by PK and Confluence pages by content ID inside visiblePagesPredicate; per-page neighbours match the full-scan neighbours for the same caller; hidden/deleted/ambiguous sources still answer [].
Gaps: Zero-padded numeric ids are not normalised with toPageIdText as sibling routes do (Finding 1) — an edge case, not a story gap. Both files the issue names (duplicate-detector.ts:78-84, pages-duplicates.ts:11-22) were examined; only the service needed changing, the route passes the raw :id through unchanged. No frontend caller of this endpoint exists (grep of worktree), so no contract breakage. SQL stays fully parameterised; visibility fragment interpolates only constant indexes/aliases. Docs updated: CHANGELOG Fixed, CLAUDE.md visibility section (:889-894), docs/API.md:52-59 (statements verified against code: both IDs accepted, caller visibility incl. restrictions, empty array for inaccessible/deleted/missing/visibly-ambiguous, historical standalone confluence_id not an identifier, admin scan same corpus with different cap/scoring). Integration tests run against real Postgres + pgvector (describe.skipIf(!dbAvailable)), no DB mocking.
Evidence: backend/src/domains/knowledge/services/duplicate-detector.ts:73-87; backend/src/domains/knowledge/services/duplicate-detector.integration.test.ts:88-118; backend/src/core/services/page-visibility.ts:14-19,42-72; backend/src/routes/knowledge/pages-duplicates.ts:12-23; docs/API.md:52-59; CLAUDE.md:889-894; CHANGELOG.md:10-14.

Suggested Actions

1. [SHOULD FIX] backend/src/domains/knowledge/services/duplicate-detector.ts:77-82 — normalise digit-only identifiers with toPageIdText before the cp.id::text = $N comparison (mirror pages-embeddings.ts:286-293: raw $1 for the confluence_id arm, toPageIdText(id) as a separate parameter for the id::text arm), and add a '00<id>' case to the #1798 integration test so the per-page lookup resolves the same row GET /api/pages/:id does.

Verdict: COMMENT

Reason: Static checks, Docker smoke (including a live GET /api/pages/:id/duplicates on a freshly created standalone page) and the Playwright suite pass; issue #1798 is fully addressed. One WARNING: the new standalone arm compares cp.id::text = $1 without the toPageIdText normalisation every sibling per-page resolver applies, so a zero-padded id resolves on GET /api/pages/007 but returns [] here — a one-line fix plus a test case.

Posted as a comment: GitHub rejects APPROVE/REQUEST_CHANGES from the PR author's own account, so the verdict above is advisory.

---

Reviewed by PR Reviewer Agent | 2026-11-01

[laboef1900]

Addressed the review WARNING in 5433ce7a: the standalone arm now binds toPageIdText(pageId) (digit-only ids) as its own parameter, mirroring pages-embeddings.ts; Confluence arm keeps the raw string. Test: '00'+id returns the same neighbours as the canonical id; non-numeric strings only match the Confluence arm. Full backend suite: 7168 passed (only the pre-existing local backup-stream mismatch), typecheck/lint/build clean.