feat: use Nextcloud filename for indexed file title + reconcile on rename

[cbcoutinho]

Summary

The vector-sync pipeline derived an indexed file's display title from the document's embedded metadata (e.g. a PDF's /Title), falling back to the Nextcloud filename only when absent. That embedded title frequently disagrees with how the user actually named the file, which is confusing in the astrolabe vector-viz UI. The UI is a passive consumer of the title payload field, so this is a backend change.

This PR:

• For doc_type == "file", always derives the display title from the Nextcloud filename (shared file_title_from_path helper). Notes / deck cards / news keep their existing metadata-derived titles.
• Adds rename/move reconciliation: a rename in Nextcloud keeps the fileid (our doc_id) and content (etag/mtime) but changes the path, so both the dedup claim and the scanner freshness gate skip re-embedding and the stored file_path/title go stale. New reconcile_document_path refreshes file_path + title on the existing real chunks via a metadata-only set_payload — no re-fetch, no re-embed.

Both rename skip-paths are covered, regardless of Nextcloud's etag-on-rename behavior:

• dedup hit (etag unchanged on rename) → claim_existing_index(current_path=...)
• scanner incremental skip (etag changed, mtime stable) → reconcile in the scanner loop

Both reuse payloads already fetched, so steady-state scans (no rename) add no extra Qdrant round-trip (reconcile no-ops when the path is unchanged).

Changes

• vector/sharing_state.py — file_title_from_path(), reconcile_document_path(), optional current_path on claim_existing_index().
• vector/processor.py — file title from filename; threads current_path into the dedup claim.
• vector/scanner.py — threads current_path into the dedup claim; reconciles in the incremental-skip branch.
• tests/unit/vector/test_sharing_state.py — +90 lines: title helper, reconcile (rename / no-op / legacy backfill), dedup-hit reconcile.

Testing

• ruff check, ruff format --check, ty check — clean.
• pytest tests/unit/ — 1360 passed, 1 skipped.

Known follow-ups (out of scope)

• The path-2 reconcile refreshes file_path/title but not the stored etag; an etag-regenerating rename leaves a mild perpetual-rescan inefficiency (correctness unaffected).
• No end-to-end integration test for rename → rescan → updated payload yet (the reconcile primitive is unit-tested).

Refs: Deck #204

---

This PR was generated with the help of AI, and reviewed by a Human

[claude]

Claude finished @cbcoutinho's task in 2m 32s —— View job

---

PR Review: feat/file-title-from-filename

• Load tools and set up
• Read changed files and git diff
• Analyze sharing_state.py changes
• Analyze processor.py changes
• Analyze scanner.py changes
• Review test coverage
• Post final review

---

Overall: ready to merge. The design is sound, the implementation is clean, and the two rename skip-paths are both covered. Findings below are all nit/observation level — nothing blocking.

---

vector/sharing_state.py

file_title_from_path — Clean and correct. The or file_path fallback for the degenerate "/" case is the right call; returning "" as a title would be confusing. Worth tracing for confidence:

"/".rstrip("/") → ""
"".rsplit("/", 1)[-1] → ""
"" or "/" → "/"  ✓

reconcile_document_path — The real_only=True filter is the right choice: placeholders already carry the current file_path from write_placeholder_point, and the reconcile is only meaningful for real embedded chunks. No extra Qdrant round-trip on unchanged paths since the early return False fires. The logger.info using %r (quoted output) makes None vs empty string visually distinct in logs — good defensive choice.

claim_existing_index ordering — Reconcile fires before add_principal. Both are independently non-fatal, so the order doesn't affect correctness. Worth noting that a reconcile failure doesn't suppress the principal grant (and vice versa), which is the right fail-open behaviour.

🟢 Nit — existing_metadata.get("file_path") passes None to stored_path when the key is absent. This triggers the "backfill" path in reconcile_document_path (None != current_path → write fires). That's the intended legacy-backfill behaviour, and it works, but it means every dedup hit for a legacy point without a file_path key will issue a set_payload even on repeat scans. Since reconcile returns True and logs at INFO in that case, it could be noisy until all legacy points are backfilled. Not a correctness issue — just worth noting if you see unexpected INFO spam post-deploy.

---

vector/processor.py

Title derivation change (line 626) — The old file_metadata.get("title") or file_path.split("/")[-1] used embedded document metadata as the primary source. The new file_title_from_path(file_path) always uses the Nextcloud filename, which is the stated goal. One minor downstream effect: file_metadata (line 622) is now only consumed for page_boundaries — it's still used, just no longer for the title. No dead-code issue, just a narrowed use.

🟢 Nit — The comment block added at lines 623–625 is well-written and explains the why clearly.

---

vector/scanner.py

New elif reconcile branch (lines 590–617) — The guard condition is correctly structured:

elif existing_metadata is not None and not existing_metadata.get("is_placeholder", False):

The existing_metadata is not None half is technically unreachable (if it were None, needs_indexing would be True and the if branch would have fired). Keeping it there is fine — it makes the condition self-documenting and safe against future refactors.

The comment above the elif clearly states the three conditions that must hold to reach this branch (fresh mtime → not needs_indexing; etag dedup hit → already continued; not placeholder). This is exactly the rename-with-stable-mtime case and the comment is accurate after the round-2 fix.

🟡 Observation — The scanner's reconcile elif has no direct unit test. The reconcile primitive is well tested in TestReconcileDocumentPath, and the dedup-hit reconcile path is tested in TestClaimExistingIndex, but the scanner-layer invocation (the incremental-skip path where etag changed but mtime is stable) isn't exercised at the unit level. The PR description acknowledges this as a known follow-up. Since the primitive is tested and the wiring is straightforward, this is acceptable short-term — but worth adding a scanner unit test before this code path gets more complex.

---

tests/unit/vector/test_sharing_state.py

The test additions are solid:

• TestFileTitleFromPath — 4 cases cover normal, no-directory, trailing-slash, and the root-only edge case.
• TestReconcileDocumentPath — 4 cases cover no-op (unchanged), no-op (empty current), rename, and legacy-backfill. The filter-key assertion (_must_keys(kwargs["points"]) == ["doc_id", "doc_type", "is_placeholder"]) confirms real_only=True is wired through.
• TestClaimExistingIndex additions — the dual-write case (test_dedup_hit_reconciles_and_grants_new_principal) is the most important new test; it verifies both set_payload calls fire independently, which was the round-1 finding. test_dedup_hit_without_current_path_skips_reconcile closes the regression case for non-file callers.

🟢 Nit — test_hit_for_already_listed_user_writes_nothing (the pre-existing test at line 255) was written without current_path. It now also implicitly tests that claim_existing_index without current_path skips reconcile even when file_path is present in the payload — this is covered by the new test_dedup_hit_without_current_path_skips_reconcile, which makes the intent explicit. Good.

---

Summary

	Finding
🔴 Blocking	none
🟡 Notable	Scanner reconcile elif has no unit test (acknowledged follow-up)
🟢 Nit	Legacy-point backfill will log at INFO on every dedup hit until backfilled
🟢 Nit	file_metadata now used only for page_boundaries (narrowed use, not a problem)

[cbcoutinho]

Round-1 findings addressed in a03bc0af:

• _shadow_classify "removal" (🟡): false alarm from a stale base — the branch was cut from dd335275 before feat: tier-0 document classifier (shadow mode) #855 landed the tier-0 classifier, so GitHub diffed against the newer master and showed those lines as removed. Rebased onto current origin/master (now 0.102.0); _shadow_classify and its imports are intact, my changes apply cleanly with no conflict.
• Placeholder round-trip (🟢, scanner.py): added the not existing_metadata.get("is_placeholder", False) guard so the scanner-skip reconcile no longer issues a 0-point set_payload for a not-yet-indexed file.
• Dual-write test gap (🟢): added test_dedup_hit_reconciles_and_grants_new_principal — renamed file + new ACL user, asserting both set_payload calls fire (file_path/title and acl_principals).

Left as-is:

• Merging the two set_payload calls (🟢): keeping them separate, per your own note that merging adds complexity for little gain — the dual-write case is now covered by a test.

ruff/ty green; tests/unit/vector/ + classifier tests pass (116).

[cbcoutinho]

Round-2 findings addressed in b7479b0d (both documentation-only, no behavior change):

• reconcile_document_path docstring (🟡): corrected — it returns True once the path differs and the set_payload is a Qdrant-side no-op when no real chunks exist; it does not return False in that case. Took the lighter docstring fix (no extra pre-check scroll), per your suggestion.
• Scanner comment imprecision (🟡): reworded to state the precise trigger — "modified_at stable so not re-queued; path may be stale from a rename" — instead of the loose "dedup miss / etag changed" phrasing.

ruff/ty green; tests/unit/vector/ pass (29).

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cbcoutinho]

Round 3: thanks — taking the "ready to merge" verdict. No code changes this round; the remaining items are nits and I want to avoid churning the review/CI cycle for them:

• Scanner elif has no unit test (🟡): acknowledged follow-up — adding it means standing up the first scanner-level unit harness (WebDAV tag-REPORT + Qdrant + send-stream mocks). The reconcile primitive and the dedup-hit path are unit-tested; deferring the scanner-wiring test as noted in the PR.
• Legacy-point backfill INFO noise (🟢): it self-heals — the first reconcile writes file_path onto the real chunks, so the next scan sees stored_path == current_path and no-ops. One INFO line per legacy doc on the first post-deploy scan, then quiet.
• file_metadata narrowed use (🟢): intentional — still consumed for page_boundaries.

Holding for the integration matrix to go green before this is merge-ready.