Prevent concurrent version bumps and spurious releases in the release
pipeline:
- Add a workflow-level concurrency group (cancel-in-progress: false) so
only one bump-version run executes at a time. Concurrent runs have
previously raced to bump the version and push tags, causing release
failures. Subsequent pushes now queue instead of cancelling an
in-flight bump/release.
- Make commitizen the single source of truth for whether a release is
warranted. The previous grep heuristic counted commits matching
feat|fix|docs|refactor|perf|test|build|ci|chore, but commitizen only
bumps for feat/fix/breaking changes. A CI- or docs-only push therefore
set bumped=true and fired release+docker against the old, already
released tag. Now we compare the latest tag before/after running
bump-mcp.sh and only set bumped=true (and emit the new tag) when it
actually changes, so release/docker exit early on non-release pushes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remaining items from the PR #831 Claude review:
- processor span symmetry: add "vector_sync.total_chars" to the sparse
embedding span (already on the dense span) and drop the redundant
"embedding.batch_size" attribute from both spans — it always equalled
vector_sync.chunk_count and would mislead once batching is split.
- metrics: document the deliberate "throughput counts only on full success"
contract in record_document_parse (partial extractions flagged
success=False are counted as a parse-error but never inflate
pages/chars/bytes throughput).
- config: extract _detect_base_provider() -> (family, model) as the single
source of truth for the provider-detection priority chain, shared by
get_embedding_model_name() and get_embedding_provider_family(). Preserves
the intentional gateway asymmetry (only the family method short-circuits).
- base.py: Optional[...] -> PEP 604 `... | None`; drop now-unused import.
Behavior unchanged (get_embedding_* outputs covered by test_config.py).
Refs Deck #175, PR #831.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a path_prefix filter to semantic search, honoured on both the MCP tool and
the dense-only visualization/API paths through the shared filter contract.
- build_base_filter_conditions: append FieldCondition(file_path,
MatchText(path_prefix)) when set. file_path is only on doc_type == "file"
points, so a non-empty path_prefix implicitly restricts to files.
- Promote path_prefix to an explicit keyword param on the SearchAlgorithm ABC
and both algorithms; thread it through nc_semantic_search (blank ⇒ no filter),
the /api/v1 search endpoints, and the viz route.
- Add a file_path TEXT payload index to _PAYLOAD_INDEX_FIELDS (no content
re-index; idempotent startup migration). MatchText tokenizes on server Qdrant
and matches by substring on local/embedded qdrant-client — both serve folder
scoping.
- Update ADR-027 (Phase 2 implemented; readiness table; semantics note). Tests.
Refs ADR-027 Phase 2. Deck #177.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a modified_after/modified_before date-range filter to semantic search,
honoured on both the MCP tool path (BM25HybridSearchAlgorithm) and the
dense-only visualization/API path (SemanticSearchAlgorithm) through one shared
contract.
- Promote modified_after/modified_before to explicit keyword params on the
SearchAlgorithm ABC and both concrete algorithms; factor the shared
placeholder+ownership+doc_type+date filter into
access_filter.build_base_filter_conditions so new filters land in one place.
- nc_semantic_search: accept RFC 3339 / ISO 8601 (or Unix seconds) bounds via
utils.validation.parse_modified_timestamp; Annotated/Field constraints on the
numeric args; explicit McpError guard for after > before. Thread the parsed
bounds through the cross-app and per-doc_type dispatch.
- /api/v1 search endpoints + viz route parse the same formats and 400 on bad or
inverted ranges.
- Add a modified_at INTEGER payload index to _PAYLOAD_INDEX_FIELDS; the
idempotent _ensure_payload_indexes() startup path migrates existing
collections with no content re-index.
- Update ADR-027 to resolve the review feedback (validation placement, shared
algorithm contract, deferral of nc_semantic_search_answer, payload index,
RFC-3339-at-the-boundary rationale). Add unit tests.
Refs ADR-027. Deck #177.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the two important findings from the claude bot's latest re-review:
- _verify_files: skip get_excluded_file_paths entirely when the tag REPORT
returns no files. An empty `tagged` yields an empty `tagged_ids` regardless
of exclusions, so the lookup's 2xlen(EXCLUDED_TAGS) WebDAV fan-out is wasted
work in the common "this tag matched nothing" case. The per-result loop still
runs, so malformed doc_ids are still kept (fail-open) — pinned by a new test
(test_verify_files_empty_tag_set_skips_exclusion_lookup), which also asserts
the exclusion lookup is never awaited.
- Rewrite the semaphore comment: it claimed "the slot bounds them", but the slot
only caps concurrent *searches* — get_excluded_file_paths internally spawns a
task group issuing 2xlen(EXCLUDED_TAGS) concurrent WebDAV calls, so live
Nextcloud connections can exceed VERIFICATION_CONCURRENCY. Comment now says so
and points at configuration.md.
The third 🟡 (sequential dir expansion in find_files_by_tag) is pre-existing and
flagged by the reviewer as a follow-up, not part of this PR.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the blocking + important findings from the claude bot's re-review:
- test (blocking): pin the file verifier's fail-open contract for definitive
403/404 on the tag REPORT, not just transient 503/429. A disabled systemtags
endpoint commonly 403s; unlike the per-access verifiers (where 403/404 = drop),
the batch file verifier must keep all results since the whole set hinges on one
REPORT. Adds _http_error(403)/_http_error(404) to
test_verify_files_tag_fetch_failure_keeps_all and documents the asymmetry.
- docs (important): migration caveat — if vector-index was created as
user_visible=False (manual occ tag:add, or pre-release), an owner's tag won't
surface in a recipient's REPORT and shared-file results are silently dropped
after upgrade. Note that the MCP server's get_or_create_tag defaults to
user_visible=True, and how to verify/fix an existing tag.
- docs (important): note the file verifier's latency scales with both the
Depth:infinity folder expansion and the EXCLUDED_TAGS lookup (~2 WebDAV calls
per excluded tag, fanned out under one slot); suggest lowering
VERIFICATION_CONCURRENCY for large excluded-tag lists / deeply tagged trees.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the latest PR-review comment on the verify-on-read tag-gate work:
- tests: stringify note IDs in test_verify_on_read.py so SearchResult.id
matches production (scanner stringifies all IDs on write) — helper and the
keeps/deleted/mixed/dedupe assertions (blocking).
- tests: make the unshared-file negative control a PDF so the drop is
unambiguously "unshared", not a mime_type_filter mismatch.
- config: add Validator("VECTOR_SYNC_PDF_TAG", len_min=1) — an empty tag name
would make find_files_by_tag("") misbehave in the verifier and scanner.
- verification: correct the _verify_files comment — two batch fetches (tag
REPORT + EXCLUDED_TAGS lookup) are held under one semaphore slot; the
pure-Python intersection runs outside it.
- tests: de-duplicate the minimal-PDF constant into a shared PDF_BYTES in
tests/integration/conftest.py, imported by both integration modules.
Verified: ruff/format/ty/unit all green; the two integration modules
(10 tests) pass against a local Nextcloud (app-only, no MCP profile needed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verify-on-read only checked file *accessibility* (file_accessible_by_id),
never tag membership, so a file removed from the `vector-index` tag (but
still readable) kept surfacing in semantic search, and stale points only
got evicted when they happened to rank in a search's top-K.
Rework `_verify_files` to gate on current `vector-index` tag membership via
a single batch `find_files_by_tag(tag, mime_type_filter="application/pdf")`
REPORT per search (plus a one-shot EXCLUDED_TAGS lookup for exclusion-wins
parity) — exactly what the scanner indexes. A file is kept iff it is in that
set, so untagged / deleted / excluded files drop out immediately and the
existing eviction wiring reclaims their Qdrant points. The gate is strict
for all file results, own and shared. Mirrors the batch-fetch-and-intersect
shape of `_verify_news_items` (one semaphore slot, fail-open on fetch error,
malformed-id keep).
- Promote the tag name to a `vector_sync_pdf_tag` Settings field (dynaconf
env mapping VECTOR_SYNC_PDF_TAG) used by both scanner and verifier;
drop the scanner's direct os.getenv.
- Expose `find_files_by_tag` on NextcloudClientProtocol.
- Rewrite the file-verifier unit tests (tagged/untagged/deleted/excluded/
fail-open/non-numeric); update the ACL + verify-on-read integration tests
to seed tagged PDFs.
- Amend ADR-019 and the configuration.md verify-on-read latency budget.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Failed deletes no longer bump astrolabe_documents_indexed_total: the outer
except in process_document now gates doc_type on operation != "delete", so a
delete error is counted as processed-error but not as an indexing event.
Added test_failed_delete_is_processed_but_not_indexed.
- registry parse span: pass record_exception=True explicitly (matches
instrument_tool) and add a structured logger.warning on the parse-error path
(processor/tier/byte_size/duration_ms) for a Loki-aggregatable failed-parse
signal.
- test_error_does_not_increment_throughput: snapshot-before/delta pattern
instead of absolute 0.0 (counters are global singletons).
- config: document the deliberate gateway asymmetry between
get_embedding_model_name() (no gateway branch) and
get_embedding_provider_family() (short-circuits on gateway).
- Cleanup in touched scope: narrow `except (HTTPStatusError, Exception)` to
`except Exception` (drop now-unused import); convert registry signatures from
Optional[...] to `... | None`.
Refs Deck #175, PR #831.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer findings:
- Fix double-count of exhausted-retry failures: the inner final-retry branch
and the outer except both recorded a processing error. Consolidate to the
outer handler (single call site); inner branch keeps only the Qdrant-upsert
error metric. Regression test added.
- Deletes are no longer counted as indexing events: the delete success path
drops doc_type so astrolabe_documents_indexed_total is not inflated.
Regression test added.
- Reuse the already-resolved `settings` in _index_document instead of a second
get_settings() call.
- Use explicit `> 0` guards in record_document_parse / record_embedding instead
of truthiness checks.
SonarCloud:
- S1244 (BUG): replace float `==` equality in metric tests with pytest.approx.
- S5332 (hotspot): use https in the gateway-URL test fixture.
- S1192: extract the repeated "vector_sync.chunk_count" span-attribute literal
into a module constant.
Review nit: move the duplicated `_sample` test helper into a shared
`metric_sample` fixture in tests/unit/conftest.py.
Refs Deck #175, PR #831.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make per-tier bottlenecks in the document-processing pipeline
(scan -> fetch -> parse -> chunk -> embed -> Qdrant upsert) visible via
metrics, traces, and structured logs. Today the document_processors layer
emits only a logger.info line: no metric, no span, and page counts live only
inside a log string. The single processing-duration histogram is unlabeled and
whole-document, so it cannot isolate parse vs embed vs upsert.
New astrolabe_* metric family (distinct from the mcp_* protocol metrics):
- astrolabe_document_parse_{duration_seconds,total} + pages/chars/bytes counters
recorded at the ProcessorRegistry.process() boundary (covers all current and
future processors uniformly)
- astrolabe_document_escalation_total (dormant; tiered-pipeline readiness)
- astrolabe_embedding_{duration_seconds,requests_total,chunks_total,chars_total}
- astrolabe_document_chunks_total, astrolabe_documents_indexed_total{source,status}
Tracing: new document_processor.parse child span + enriched embed/chunk span
attributes (provider/model/batch_size/chunk_count). Structured logs gain a
consistent field vocabulary (doc_id, doc_type, processor, tier, pages, chars,
byte_size, chunks, duration_ms, status) so Loki can aggregate without regex.
Tier-readiness: processor/tier are labels from day one and a tier property is
added to DocumentProcessor, so adding docling/OCR/LLM tiers later is additive
(new label values, never new metrics). Tenant comes from the kube namespace
label; mime_type/model are span attributes only (cardinality). Existing
mcp_vector_sync_*/mcp_qdrant_* are left untouched.
Refs Deck #175 (superset of #173 Phase 2). Dashboard/recording-rules follow-up
tracked on #175 for homelab-argocd.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Modernize the new models (DeckCardSummary, DeckCommentSummary,
StackOverview, BoardOverviewResponse + the loosened unions) to PEP 604
syntax (list[...] / X | None), per CLAUDE.md.
- Make status="done" exclude archived cards so open/done/archived partition
the board with no overlap (a done+archived card is reported only as
"archived"); document the semantics in docstrings and docs/deck.md, add a
partition unit test.
- deck_get_archived_stacks: pass through label/assigned_to filters (status
stays archived-only by definition); note the limitation in the docstring.
- Rename _validate_description_max_length → _validate_positive_length (now a
generic positive-length guard).
- Soften deck_get_board_overview docstring: it views board state and omits
the ACL/user/label-management fields deck_get_board exposes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EMBEDDING_GATEWAY_URL is configured as a bare origin (scheme://host:port) —
the deployment's Service URL. GatewayProvider now appends the gateway's /v1
base path before handing the URL to the OpenAI SDK, so both embed posts
({base}/embeddings) and dimension discovery ({base}/models) land under /v1.
Idempotent: a URL already ending in /v1 is left unchanged.
This lets EMBEDDING_GATEWAY_URL stay a bare domain (matching the gitops
Service URLs) instead of requiring a hand-appended /v1.
Also align the `embedding_gateway_model` field default with _DEFAULTS
("mistral/mistral-embed"). The gateway catalog is provider-namespaced, and
_detect_dimension matches `entry.id == embedding_model`; the stale
un-namespaced default would silently miss the catalog entry and leave the
dimension unresolved (re-triggering the external-mode startup crash).
Tests: bare / trailing-slash / idempotent normalization + a bare-origin
discovery test asserting /v1/models. 16 gateway-provider tests pass;
providers + vector suites green (129 total); ruff clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deck read tools returned too many tokens to be usable as boards grow — even
deck_get_stacks(description_max_length=1) exceeded the MCP token limit because
every card was fully serialized in list views.
- Add compact projection models (DeckCardSummary, DeckCommentSummary,
StackOverview, BoardOverviewResponse) and a uniform detail="summary"|"full"
knob (summary default) on deck_get_cards / get_stacks / get_stack /
get_archived_stacks.
- Add pre-serialization filtering: status (open/done/archived/all), label,
assigned_to.
- Add deck_get_board_overview: board title + label legend + stacks with
compact card rows + counts in a single call.
- Compact comments: detail / message_max_length / newest-first order on
deck_get_card_comments.
- Docs + unit/integration tests.
BREAKING CHANGE: deck list tools now default to detail="summary" and
status="open". The include_archived_cards parameter is replaced by status
(use status="all" to include archived cards); pass detail="full" to restore
the previous per-card shape.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
