Skip to content

Commit 4368ea6

Browse files
authored
Orphan-chunk fixes (cancel + startup GC), nested .gitignore, human time bounds, and docs reconciliation (#124)
* engine: purge orphan chunks when a pipeline object is cancelled mid-embed The shared EmbedConsumer upserts an object's chunks to Milvus before its finalize hook writes the objects row. If the task was cancelled out from under it (an external job cancel, or a connector removal racing the consumer), the hook lost only the object_tasks 'succeeded' claim while still committing the chunks and the objects row, leaving chunks that no objects row points at. Those orphans are un-inspectable and un-cat-able (404) yet still match search, which queries Milvus directly. Claim completion first (guarded UPDATE ... WHERE status='running', via execute_rowcount); when the claim is lost, delete the just-upserted chunks and skip the objects row / cursor advance so a later sync re-does the object cleanly. Carry connector_uri in the finalize context for the delete. * mfs-ingest skill: drop jq from example snippets The job-poll loop and connector-list/ls examples piped through jq, which is not a project dependency and is absent on many machines, so the snippets failed silently. Use grep on the JSON status field, awk on the connector-list column, and plain --json output the agent reads directly. * engine: reconcile orphan chunks at startup Adds a cost-guarded startup GC that deletes Milvus chunks no committed objects row points at — the residual window the per-object self-heal can't cover (a consumer crash between upsert and finalize, or pre-existing orphans). It first compares the Milvus row total against the sum of committed chunk_counts and returns immediately when they match, so a healthy index pays only a couple of count() aggregates; the distinct-object scan + per-object delete run solely when there is genuine excess. Both the count and the scan are scoped to non-summary chunks via MilvusStore.distinct_objects/GC_SCOPE_EXPR, since directory summaries legitimately have no objects row and must never be collected. * api: clarify the ls 'indexable' field is eligibility, not searchability indexable=True only means an object is not binary and not opted out; data/config kinds (yaml, jsonl, csv) are eligible but never embedded. Agents and users were reading it as 'searchable'. The real signal is search_status — say so on the field. * docs: rewrite CONTRIBUTING with project background, architecture, and a connector guide Adds a 'what MFS is' + architecture-at-a-glance section and a substantial 'Contributing a connector' guide that carries the design intent — thin connector on a fat framework, required vs optional methods, honest Capabilities, object naming conventions, boundary rules, the four-sources consistency rule, and a pre-work design checklist. Reconciled against the current code rather than the old design notes: connectors are flat (__init__.py + plugin.py), registration is register() + the load_builtin() import list (no REGISTRY dict), the dataclass is PathStat (not FileStat), there is no _contract.py harness (fakes under tests/), and PROMPT is a short inline string rather than a PROMPT.md auto-collected into the skill. Also fixed stale bits in the existing file (skills/mfs path, $MFS_HOME default, v0.4.0 wording, mfs job list/show command names). * docs: add CLAUDE.md for agent onboarding, symlinked as AGENTS.md Captures the big-picture architecture (thin Rust CLI over a Python server that owns connectors / ingest / index / storage), the non-obvious build/test commands, the object-row/Milvus invariant, the four-sources connector-config rule, and lockstep release mechanics. AGENTS.md is a symlink so both the CLAUDE.md and AGENTS.md agent-file conventions resolve to one file. * docs: fix chunk_kind names (body, not chunk_body/file/code/image) The text chunk kind is 'body'; file/code/image are object kinds, not chunk kinds. mfs-find's --kind list and connector references said 'chunk_body' (so '--kind chunk_body' matched nothing), and ingest-pipeline.md listed file/code/ image as chunk_kinds. Align all of them with connectors/base.py ChunkKind. * connectors: reconcile config fields with the plugins across the four sources - jira: Server/DC PAT goes in 'token' (the plugin's PAT auth path), not 'api_token' (which is the Cloud email+token path). Fix the skill + add a 'token' field to the wizard. - slack: 'oldest' is passed straight to Slack and must be a unix timestamp; 'now-30d' is never parsed. Fix skill/docs/wizard, and drop the false claim that slack honors --since (it has no since_pushdown). - web: 'max_pages' default is 50, not 100 (skill table + wizard default). - file: drop 'max_file_bytes' from the file connector docs — FileConfig has no such key and it is read nowhere. * docs: correct inaccurate command and behavior descriptions - providers: image-description/summary setup is 'setup --section description' (and --section summary); there is no 'vlm' section. - search-and-browse: 'cat --range' requires an explicit colon — a bare '120' is rejected, not treated as '120:'. - ingest-pipeline: select_producer maps 8 object kinds onto 5 producer classes (not one per kind). - schema: the cache_key also folds in the call kind and config. - cli: match the real em-dash in the queued-job message. * deployments: compose uses MILVUS_URI/MILVUS_TOKEN, not the unread MFS_MILVUS_* names The server config reads MILVUS_URI / MILVUS_TOKEN (and ZILLIZ_*); MFS_MILVUS_* is read nowhere, so a Compose-supplied Zilliz endpoint was silently ignored and the server fell back to Lite. Map the host ZILLIZ_URI/ZILLIZ_TOKEN onto the real MILVUS_URI/MILVUS_TOKEN the server actually reads. * docs: minor accuracy fixes (cargo-fmt scope, s3 region default, artifact-kind docstring) - CLAUDE.md / CONTRIBUTING.md: CI cargo-fmt checks both cli and server-rs. - s3 skill: the connector sets no region default (boto3/AWS env decides), not us-east-1. - ArtifactStore docstring: drop the stale 'vlm_text' artifact kind — VLM output lives in the transformation cache, not the artifact cache. * file connector: honor nested .gitignore/.mfsignore, not just the root's The walk fed a single flat pattern list (DEFAULT_IGNORE + only the root .gitignore/.mfsignore) to walk_tree, so a subproject that ignores its build dir via its own nested .gitignore (e.g. server-rs/.gitignore '/target') had that dir walked and indexed — inflating object counts and pegging CPU on dev trees. Collect every .gitignore/.mfsignore in the tree and translate each line to a root-relative pattern (anchored lines scoped to their dir; floating lines become '<dir>/**/<line>'), folding git's per-directory semantics into the one flat list walk_tree consumes. This fixes both the pure-Python and the native walkers with no Rust change, and prunes ignored dirs as it descends (so .venv / a nested build dir is never entered). list() uses the cheaper root..dir ancestor chain. * connectors: accept human time bounds (ISO date / now-30d / unix ts) slack's 'oldest' previously required a raw unix timestamp and was passed straight to Slack — out of step with feishu's --since, which already took an ISO date, and a poor UX (nobody hand-computes an epoch). Add a shared common.timeparse parser (ISO date / datetime, relative now-Nd|w|h|m, or a unix ts) and use it for slack 'oldest' and feishu '--since'. Docs/skill/wizard describe the friendly forms again — now backed by the code. Slack still has no --since (no since_pushdown); 'oldest' is its bound.
1 parent ce1e2c6 commit 4368ea6

39 files changed

Lines changed: 1144 additions & 147 deletions

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
CLAUDE.md

CLAUDE.md

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
# CLAUDE.md
2+
3+
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4+
5+
## What this is
6+
7+
MFS exposes many sources (code, docs, messages, databases, object stores) as **one
8+
file-like, searchable namespace**. It is a monorepo of two cooperating processes:
9+
10+
- **`cli/`** — a thin **Rust** CLI (`mfs`). It is *only* an HTTP client of the
11+
server; it holds no business logic. Don't add behavior here that belongs on the
12+
server.
13+
- **`server/python/`** — the **Python** server (`mfs-server`, FastAPI). It owns
14+
connectors, the ingest pipeline, the vector index, and storage, exposed over an
15+
HTTP `/v1` control plane. This is where almost all work happens.
16+
17+
The SDKs (`sdks/python`, `sdks/typescript`) are generated from
18+
`protocol/openapi.yaml`. `server-rs/` is an optional PyO3 acceleration wheel with a
19+
transparent pure-Python fallback (`mfs_server.common.accel`) — never required.
20+
21+
`CONTRIBUTING.md` and `docs/architecture.md` are the long-form references; the
22+
points below are the ones that need several files to piece together.
23+
24+
## Commands
25+
26+
All Python commands run from `server/python/`. **`dev` is an extra, not a
27+
dependency-group**`uv run pytest` fails (no pytest); always use `--extra dev`:
28+
29+
```bash
30+
uv sync # default deps (local ONNX, Milvus Lite, SQLite)
31+
uv run --extra dev pytest # all unit/smoke tests
32+
uv run --extra dev pytest tests/test_engine_chunkable_e2e.py::test_chunkable_path_drains_through_pipeline # one test
33+
uv run --extra dev ruff format --check src/ tests/ # what CI enforces
34+
uv run --extra dev ruff check src/ tests/ # lint (loose; not gated in CI)
35+
uv run mfs-server run # run server from checkout, binds 127.0.0.1:13619
36+
```
37+
38+
- A few tests import optional connector SDKs (e.g. `lark_oapi`) and **fail to
39+
collect** without them; `--ignore` those files or `uv sync --extra all-connectors`.
40+
- CI (`.github/workflows/lint.yml`) enforces **formatting only** — `ruff format
41+
--check` and `cargo fmt --all -- --check`. Tests are not gated, so run them yourself.
42+
43+
Rust:
44+
45+
```bash
46+
cd cli && cargo build --release && cargo test
47+
cargo fmt --manifest-path cli/Cargo.toml --all -- --check # CI format-checks cli
48+
cargo fmt --manifest-path server-rs/Cargo.toml --all -- --check # ...and server-rs
49+
```
50+
51+
All server state (config, token, SQLite metadata, Milvus Lite vectors, caches, and
52+
the ONNX model cache) lives under one root, `$MFS_HOME` (default `~/.mfs`). The
53+
vector and metadata backends are selectable by environment/config (Milvus Lite vs a
54+
remote Milvus/Zilliz; SQLite vs Postgres) — see `docs/deployment.md`.
55+
56+
## Architecture that spans files
57+
58+
**Connectors → path tree.** Each connector (`server/python/src/mfs_server/connectors/<name>/plugin.py`)
59+
presents an external system as a path tree and classifies each object's
60+
`object_kind`; it never touches the index, embeddings, or cache directly. The
61+
contract is `connectors/base.py` (`ConnectorPlugin`): 5 abstract methods
62+
(`stat` / `list` / `fingerprint` / `sync` / `object_kind_of`) plus `read` **or**
63+
`read_records`, with optional overrides (`grep`, `search`, `preset_for`, …).
64+
Registration is `register(MyPlugin)` in the connector's `__init__.py` plus an entry
65+
in `registry.py:load_builtin()` — there is no central `REGISTRY` dict.
66+
67+
**`object_kind` and `chunk_kind` are framework-fixed `Literal`s** in `base.py`.
68+
Adding a value is a framework change (RFC), not a connector change.
69+
70+
**Ingest runs two lanes** (`engine/`, see `docs/ingest-pipeline.md`):
71+
- *Object Lane* — each object goes producer → in-memory chunk queue → a
72+
**process-shared `EmbedConsumer`** that embeds + upserts chunks to Milvus, then a
73+
finalize hook (`_on_pipeline_object_indexed`) writes the `objects` metadata row.
74+
- *Job Lane* — cross-object work (directory summaries) that can't be done per-object.
75+
76+
The consumer is decoupled and shared across jobs; this is the subtle part. The core
77+
invariant is **a chunk exists in Milvus iff a committed `objects` row points at it**
78+
(directory summaries are the deliberate exception — they have no `objects` row).
79+
Cancellation/removal and a startup GC reconcile against this invariant, so any new
80+
finalize/cleanup path must preserve it.
81+
82+
**Storage** (`storage/`): metadata in SQLite or Postgres (connectors, objects,
83+
jobs, and the queues — *a queue is just a table*), vectors in Milvus
84+
(`connector_uri` is the partition key), an artifact cache, and a separate
85+
transformation cache (model outputs). Shapes: `docs/schema.md`, `docs/caching.md`.
86+
87+
## Connector config: four sources, one source of truth
88+
89+
A connector's config/credentials are described in four places that must stay
90+
consistent; **the code is the source of truth**, the rest document it at increasing
91+
detail (skill < wizard < docs):
92+
93+
1. **Code**`connectors/<name>/plugin.py` (keys it reads) + the user's `connector.toml`.
94+
2. **Wizard**`server/connector_schemas.py` (an important-fields subset).
95+
3. **Skill**`skills/mfs-ingest/reference/connectors/<name>.md` (short, for agents).
96+
4. **Docs**`docs/connectors/<name>.md` (full human walkthrough).
97+
98+
When you touch one, grep the connector name across the other three and reconcile;
99+
field names and enum values must match the plugin exactly. Secrets are never
100+
plaintext in TOML — the TOML carries an `env:VAR` / `file:/abs/path` reference that
101+
the server resolves at plugin-build time; a plugin reads credentials only through
102+
the framework, never `os.environ[...]`.
103+
104+
## Releasing
105+
106+
Versions are **lockstep** across `mfs-cli`, `mfs-server`, the SDKs, and `server-rs`.
107+
A single `v*` tag triggers three publish workflows (`release.yml` → CLI binaries +
108+
GitHub Release, `publish-crates.yml` → crates.io, `publish-pypi.yml` → PyPI via
109+
Trusted Publishing). `server-rs` is deliberately **not** published to PyPI. Merging
110+
to `main` does not publish; pushing the tag does.
111+
112+
## Skills
113+
114+
`skills/` holds two agent skills — `mfs-ingest` (register/update sources) and
115+
`mfs-find` (search/browse). They are **agent-agnostic**: describe intent + the `mfs`
116+
CLI/API, never couple to a specific agent's tools or UI.

0 commit comments

Comments
 (0)