diff --git a/CHANGELOG.md b/CHANGELOG.md index e157d83..6a30342 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,35 @@ All notable changes to this project are documented here. The format is based on ### Added +- **Bounded parallel ingest — `citadel ingest --jobs N`** (the 2026-07 audit's backlog #11, and + with it its finding 1.2.5). Ingest was strictly serial because the per-source staging redirect + *assigned* `config.WIKI_DIR` and `os.environ` in place: one process could only ever be inside one + redirect. That redirect is now context-local (a `ContextVar` behind `config.wiki_dir()` and + friends; child processes get their wiki through an explicit per-spawn env), which is what lets N + sources stage at once — the isolation primitive, a per-source staging copy, was already there. + `--jobs N` (or `CITADEL_JOBS`) folds in that many sources concurrently. **Every guarantee is + unmoved**: one promote per source, all-or-nothing, nothing partial on the live wiki, the manifest + still saved per completed source. What is shared is serialized rather than raced — one lock guards + the two moments that touch the live wiki (cloning it, promoting onto it), while the minutes-long + sessions run fully in parallel; the promote is *base-aware*, pruning only what its own clone had + and its staging lacks, so a concurrent source's new page is never deleted as "the agent removed + it"; and a promote whose pages have moved since the clone is refused **before** it writes + anything, with that source re-run serially at the end of the run, where it sees the winner's page + and merges into it (the report lists these under *Re-run serially* — the one place parallelism + costs a session a serial run would not have). Report, manifest, and failures bookkeeping stay on + the main thread, so those writes are as single-threaded as they ever were, and an interrupt + cancels queued sources while still recording work already promoted. The default stays **1** — + serial, line for line as before — because the real cost is not safety but **cross-linking**: + concurrent sessions cannot see each other's new pages. Best on a large backlog of unrelated + sources (see docs/recipes.md), and available on `citadel refresh --jobs N` too — a refresh slice is ordered by last-checked time rather than by topic, so its sources are usually unrelated; `curate` remains serial by design. Chunked sources needed one more + rule, found by an adversarial cross-review and reproduced offline before it was fixed: a resume + checkpoint's delta is measured against the wiki its source was **cloned from**, never against a + live wiki that has since moved on. Measured against live, a page a concurrent source *created* + read as a deletion this source had made, and one it *rewrote* read as this source's change + carrying stale bytes — and because a checkpoint is durable, that delta outlived the parallel run + and could prune a fully-ingested source's page in a later, even strictly serial, run. The base + state a replay is guarded against now comes from that same snapshot, so a page another source + touched fails the guard instead of passing it. - **`citadel serve --http` — the opt-in Streamable HTTP transport** (the 2026-07 audit's backlog #12, closing the last open item of its § 3.1 MCP-surface gap). Stdio remains the default and is unchanged; `--http` serves the SAME thirteen tools, four prompts, and `wiki://` resources over diff --git a/CLAUDE.md b/CLAUDE.md index d6e787e..1611069 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,9 +28,10 @@ uv run python -m citadel Subcommands: `init [DIR]` (scaffold a workspace: `citadel.toml` marker, `.env`, `raw/`, `wiki/`; idempotent), `ingest [paths…]` (fold raw/ into the wiki; `--verbose`/`-v` streams the agent session, `--log-dir DIR` writes a transcript per source, `--quiet` drops the progress spinner, +`--jobs N`/`-j` folds N sources in CONCURRENTLY (default 1 = serial; `CITADEL_JOBS`), `--full-rescan` distrusts the manifest's stat cache and re-hashes every tracked source, `--force ` deliberately re-reads already-ingested sources as a reconcile — it requires -explicit paths and is refused without them), `refresh [--limit N] [--min-age-days D] [--dry-run]` +explicit paths and is refused without them), `refresh [--limit N] [--min-age-days D] [--dry-run] [--jobs N]` (the THIRD lifecycle: re-verify the least-recently-checked sources — ordered by the manifest's `ingested_at` stamp, oldest/stampless first — through forced reconcile sessions on an explicit per-run budget of N sources; the sustainable alternative to regenerating the wiki after a model @@ -229,7 +230,20 @@ it is itself a workspace. vanished source's stale provenance before any pending session touches a page that still cites it (else that pre-existing bad citation would fail the pending session's validation and roll it back). This all-or-nothing + network-share-hardened machinery (`_robust_*`, `robust_mkdir`) is - load-bearing — don't simplify it away. **One mutating run per workspace**: ingest and curate take + load-bearing — don't simplify it away. **Bounded parallelism** (`--jobs N` / `CITADEL_JOBS`, default 1 = serial): the per-source staging + copy IS the isolation primitive, so N sources run at once through the same `_SourceJob` loop — + worker threads only plan/stage/run/promote, while every SHARED write (report, manifest, failures) + stays on the main thread. One lock (`_LIVE_WIKI_LOCK`) guards the only two moments that touch the + live wiki: the clone (taken together with a hash snapshot of what was cloned) and the promote. + Under it the promote is **base-aware** — it prunes only what its own clone had and its staging + lacks (else it would delete a concurrent source's new pages), and it REFUSES, before writing a + byte, if a path it would touch has moved since the clone; that source is then re-run SERIALLY at + the end of its group, where it merges into the winner's page (reported as `raced`). What + parallelism costs is cross-linking, not safety — concurrent sessions cannot see each other's new + pages — which is why the default is 1. The redirect this rides on is context-local + (`config.wiki_redirect` → a ContextVar behind `config.wiki_dir()`; children get their wiki via + `config.child_env()`), never a `config.WIKI_DIR`/`os.environ` assignment. + **One mutating run per workspace**: ingest and curate take an exclusive run lock (`runlock.py`, a dotfile sibling of the wiki; stale locks reclaimed via dead-pid/mtime, refreshed per source) so a second concurrent run fails loud instead of silently destroying the first one's staging/promotes; manifest + failures saves are atomic @@ -408,8 +422,12 @@ save-the-transcript-as-a-file lane for whole conversations). `rawsource.py` back ## Conventions specific to this codebase -- **`config.*` is read at call time** (`from . import config` then `config.WIKI_DIR`), never imported +- **`config.*` is read at call time** (`from . import config` then `config.RAW_DIR`), never imported by value — so tests can monkeypatch the whole filesystem layout. Honor this when adding code. + The WIKI path is read through the ACCESSORS (`config.wiki_dir()`, `index_path()`, + `sources_index_path()`, `log_path()`, `manifest_path()`, `failures_path()`), never as + `config.WIKI_DIR`: ingest redirects it per source through a ContextVar, so only the accessors see + the staging copy. The module attributes stay the process-wide base tests monkeypatch. - **Tests redirect everything to `tmp_path`** by monkeypatching `config.*` (including `WORKSPACE_ROOT`, which the agent's `cwd` reads) and replace `llm.run_ingest_session` with a fake that writes files into the temp wiki. No test spawns a real LLM CLI. Follow that pattern; keep @@ -437,7 +455,8 @@ save-the-transcript-as-a-file lane for whole conversations). `rawsource.py` back `READ_ONLY`), `CITADEL_LLM_VERBOSE`, `CITADEL_LLM_LOG_DIR`, `CITADEL_REPO_SUPPORT`, `CITADEL_IMAGE_SUPPORT` (read images visually), `CITADEL_AUDIO_SUPPORT` (opt-in whisper transcript ingest for audio/video, with `CITADEL_WHISPER_CLI`/ - `CITADEL_WHISPER_MODEL`/`CITADEL_WHISPER_TIMEOUT` tuning the seam), `CITADEL_MAX_SOURCE_CHARS` + `CITADEL_WHISPER_MODEL`/`CITADEL_WHISPER_TIMEOUT` tuning the seam), `CITADEL_JOBS` (how many sources ingest folds in + concurrently; 1 = serial), `CITADEL_MAX_SOURCE_CHARS` (large-source chunking threshold), `CITADEL_RESUME` (resume checkpoints for those chunked sources: continue at the segment an interrupted run died on instead of re-paying for the earlier ones; default on), `CITADEL_DEDUP_BY_BASENAME` (skip same-basename document diff --git a/citadel/catalogs.py b/citadel/catalogs.py index 15e6344..b863a6b 100644 --- a/citadel/catalogs.py +++ b/citadel/catalogs.py @@ -192,7 +192,7 @@ def rebuild_indexes(pages: list[Page] | None = None) -> None: # separate O(sources × pages) scans the two consumers used to run independently. refs_by_key = citing_pages_map(manifest_dict, pages) sources_body = _render_sources_catalog(manifest_dict, pages, failures_dict, refs_by_key) - sources_path = config.SOURCES_INDEX_PATH + sources_path = config.sources_index_path() if sources_body is not None: config.robust_mkdir(sources_path.parent) sources_path.write_text(sources_body, encoding="utf-8") @@ -204,7 +204,7 @@ def rebuild_indexes(pages: list[Page] | None = None) -> None: # sources catalog it is written when there is at least one point and removed when there are none. open_points = collect_open_points(pages) open_points_body = _render_open_points_catalog(open_points, title_by_path) - open_points_path = config.WIKI_DIR / OPEN_POINTS_INDEX_REL + open_points_path = config.wiki_dir() / OPEN_POINTS_INDEX_REL if open_points_body is not None: config.robust_mkdir(open_points_path.parent) open_points_path.write_text(open_points_body, encoding="utf-8") @@ -303,8 +303,8 @@ def rebuild_indexes(pages: list[Page] | None = None) -> None: lines.append("") body = "\n".join(lines).rstrip("\n") + "\n" - config.robust_mkdir(config.WIKI_DIR) - (config.WIKI_DIR / "index.md").write_text(body, encoding="utf-8") + config.robust_mkdir(config.wiki_dir()) + (config.wiki_dir() / "index.md").write_text(body, encoding="utf-8") # ----- per-directory index.md (also frontmatter-free) ----- for folder in sorted(folders): @@ -315,6 +315,6 @@ def rebuild_indexes(pages: list[Page] | None = None) -> None: flines.append(f"- [{page.title}]({rel_in_folder}) — {page.description}") flines.append("") fbody = "\n".join(flines).rstrip("\n") + "\n" - folder_dir = config.WIKI_DIR / folder + folder_dir = config.wiki_dir() / folder config.robust_mkdir(folder_dir) (folder_dir / "index.md").write_text(fbody, encoding="utf-8") diff --git a/citadel/cli.py b/citadel/cli.py index 87cdadb..ade50fb 100644 --- a/citadel/cli.py +++ b/citadel/cli.py @@ -116,6 +116,18 @@ def build_parser() -> argparse.ArgumentParser: "without them, so a whole-corpus re-read (one agent session per source) can never " "happen by accident.", ) + p_ingest.add_argument( + "--jobs", + "-j", + type=int, + default=None, + metavar="N", + help="Fold up to N sources CONCURRENTLY (default 1 = serial, or CITADEL_JOBS). Each " + "source keeps its own staging copy and its own all-or-nothing promote; promotes are " + "serialized, and a source that raced another over the same page is re-run serially. " + "Faster on a large backlog of unrelated sources; the trade-off is cross-linking, since " + "concurrent sessions cannot see each other's new pages.", + ) p_ingest.set_defaults(func=cmd_ingest) p_curate = sub.add_parser( @@ -188,6 +200,17 @@ def build_parser() -> argparse.ArgumentParser: help="Write a transcript file per source to DIR (see `citadel ingest --log-dir`). " "Overrides CITADEL_LLM_LOG_DIR.", ) + p_refresh.add_argument( + "--jobs", + "-j", + type=int, + default=None, + metavar="N", + help="Re-verify up to N sources CONCURRENTLY (default 1 = serial, or CITADEL_JOBS) — the " + "same knob, and the same trade-off, as `citadel ingest --jobs`. A refresh slice is ordered " + "by last-checked time rather than by topic, so its sources are usually unrelated and " + "parallelism costs little cross-linking here.", + ) p_refresh.set_defaults(func=cmd_refresh) p_status = sub.add_parser( @@ -388,9 +411,16 @@ def cmd_ingest(args: argparse.Namespace) -> int: ``--force`` requires explicit paths: a forced re-read runs one agent session per source, so forcing the ENTIRE corpus must never happen by accident — the - flag alone is refused with exit 2, before ``ingest.ingest`` is ever called.""" + flag alone is refused with exit 2, before ``ingest.ingest`` is ever called. + + ``--jobs N`` is a usage error below 1 (exit 2, like ``--force`` without paths) rather than an + exception out of the API layer; omitted, the run takes ``CITADEL_JOBS`` (default 1, serial).""" from . import config, ingest + if args.jobs is not None and args.jobs < 1: + print(f"error: --jobs must be at least 1 (got {args.jobs}); 1 means the serial default.", file=sys.stderr) + return 2 + if args.force and not args.paths: print( "error: --force requires explicit paths (a forced re-read runs one agent session per " @@ -414,7 +444,9 @@ def cmd_ingest(args: argparse.Namespace) -> int: # CITADEL_LLM_VERBOSE — not just the --verbose flag — also drops the spinner that would # otherwise clobber the streamed transcript. progress = ConsoleProgress(spinner=not config.LLM_VERBOSE) - report = ingest.ingest(args.paths or None, progress=progress, full_rescan=args.full_rescan, force=args.force) + report = ingest.ingest( + args.paths or None, progress=progress, full_rescan=args.full_rescan, force=args.force, jobs=args.jobs + ) print(report.render()) # Non-zero on a per-source error OR a structural problem left behind (a broken # cross-link the agent introduced) — so ingest gates the wiki's integrity in CI. @@ -450,6 +482,9 @@ def cmd_refresh(args: argparse.Namespace) -> int: file=sys.stderr, ) return 2 + if args.jobs is not None and args.jobs < 1: + print(f"error: --jobs must be at least 1 (got {args.jobs}); 1 means the serial default.", file=sys.stderr) + return 2 if args.verbose: config.LLM_VERBOSE = True if args.log_dir is not None: @@ -459,7 +494,9 @@ def cmd_refresh(args: argparse.Namespace) -> int: from .progress import ConsoleProgress progress = ConsoleProgress(spinner=not config.LLM_VERBOSE) - report = refresh.refresh(limit=args.limit, min_age_days=args.min_age_days, dry_run=args.dry_run, progress=progress) + report = refresh.refresh( + limit=args.limit, min_age_days=args.min_age_days, dry_run=args.dry_run, progress=progress, jobs=args.jobs + ) print(report.render(), end="") ing = report.ingest_report return 1 if ing is not None and (ing.errors or ing.broken_links) else 0 @@ -719,7 +756,7 @@ def cmd_check(args: argparse.Namespace) -> int: issues = validate.validate_all(pages) missing: list[str] = [] if args.paths: - wiki_root = config.WIKI_DIR.resolve() + wiki_root = config.wiki_dir().resolve() wanted: set[str] = set() for arg in args.paths: rel = arg.replace(os.sep, "/") @@ -735,7 +772,7 @@ def cmd_check(args: argparse.Namespace) -> int: known = {p.rel_path for p in pages} for rel in sorted(wanted - known): try: - on_disk = okf.safe_join(config.WIKI_DIR, rel).is_file() + on_disk = okf.safe_join(config.wiki_dir(), rel).is_file() except okf.OKFError: on_disk = False if on_disk: diff --git a/citadel/config.py b/citadel/config.py index af10e15..494b97f 100644 --- a/citadel/config.py +++ b/citadel/config.py @@ -24,18 +24,23 @@ No logic beyond path/setting/rules resolution (plus the tiny content hash over the effective rules tree, :func:`rules_version`, which is pure derivation over that resolution). -NOTE: other modules reference ``config.WIKI_DIR`` / ``config.INGEST_MODEL`` / -``config.LLM_CLI`` / etc. at call-time (``from . import config`` then -``config.WIKI_DIR``) so tests can monkeypatch these attributes. +NOTE: other modules reference ``config.INGEST_MODEL`` / ``config.LLM_CLI`` / ``config.RAW_DIR`` / +etc. at call-time (``from . import config`` then ``config.RAW_DIR``) so tests can monkeypatch +these attributes. The WIKI path is the one exception: it is read through the +:func:`wiki_dir`/:func:`index_path`/… ACCESSORS, because ingest redirects it per source (see +:func:`wiki_redirect`) — the module attributes stay the process-wide base those accessors fall +back to, so monkeypatching them still configures the whole layout. """ from __future__ import annotations import contextlib +import contextvars import hashlib import os import shutil import stat +import threading import time from pathlib import Path, PurePosixPath @@ -442,7 +447,10 @@ def atomic_write_text(path: Path | str, text: str, attempts: int = 4) -> None: staging copy already skips, and the replace retries briefly to ride out the transient sharing violations SMB shares and AV scanners cause on Windows (the ``_robust_*`` pattern).""" p = Path(path) - tmp = p.with_name(f"{p.name}.{os.getpid()}.citadeltmp") + # pid AND thread id: `citadel ingest --jobs N` runs several sources in one process, so a + # pid-only temp name is no longer unique — two writers would share one temp file and the loser's + # os.replace would fail on a file the winner already moved. + tmp = p.with_name(f"{p.name}.{os.getpid()}.{threading.get_ident()}.citadeltmp") tmp.write_text(text, encoding="utf-8") try: for attempt in range(attempts): @@ -591,6 +599,96 @@ def rules_version() -> str: # dropped) and surfaced in wiki/sources/index.md. FAILURES_PATH: Path = WIKI_DIR / ".citadel_failures.json" + +# --- The wiki in effect right HERE (per-context, never process-global) ------------------------ +# Ingest runs every source's agent session against a per-source STAGING copy of the wiki, so for +# the duration of that session "the wiki" is the staging dir, not WIKI_DIR. That redirect used to +# ASSIGN the module attributes below (and os.environ), which made it process-global: one process +# could only ever be inside one redirect, and two sources could never be in flight at once — the +# blocker the 2026-07 audit named (finding 1.2.5) under `--jobs N`. +# +# It is a ContextVar instead: the override is CONTEXT-local rather than process-global, so N worker +# threads can each hold their OWN staging redirect while the main thread still sees the live wiki. +# Two properties of CPython make that work, and they pull in opposite directions: +# * a thread does NOT inherit the context of the thread that started or submitted to it — neither +# a plain `threading.Thread` nor a `ThreadPoolExecutor` worker (unlike an asyncio task, which +# copies its context), so a worker starts out seeing the live wiki, which is correct; +# * but a POOLED thread is REUSED across work items, and a value left set by one item is still +# there for the next one on that thread. +# So the `finally` reset in :func:`wiki_redirect` is load-bearing, not tidiness: it is what keeps a +# finished source's staging path from being handed to the next source that lands on the same worker. +# Unset (the overwhelmingly common case: every read path, every CLI command, the MCP server) the +# accessors return the module attributes verbatim, so the process-wide layout — and every test that +# monkeypatches it — behaves exactly as before. +_WIKI_OVERRIDE: contextvars.ContextVar["Path | None"] = contextvars.ContextVar("citadel_wiki_dir", default=None) + + +def wiki_dir() -> Path: + """The wiki directory THIS context reads and writes: the active :func:`wiki_redirect` target + (ingest's per-source staging copy) when one is in effect, else the process-wide + :data:`WIKI_DIR`. Every in-package consumer goes through this accessor rather than reading + ``config.WIKI_DIR`` directly — that is what lets two sources stage concurrently.""" + override = _WIKI_OVERRIDE.get() + return override if override is not None else WIKI_DIR + + +def index_path() -> Path: + """:data:`INDEX_PATH` (``/index.md``), redirect-aware — see :func:`wiki_dir`.""" + override = _WIKI_OVERRIDE.get() + return override / "index.md" if override is not None else INDEX_PATH + + +def sources_index_path() -> Path: + """:data:`SOURCES_INDEX_PATH` (``/sources/index.md``), redirect-aware.""" + override = _WIKI_OVERRIDE.get() + return override / "sources" / "index.md" if override is not None else SOURCES_INDEX_PATH + + +def log_path() -> Path: + """:data:`LOG_PATH` (``/log.md``), redirect-aware.""" + override = _WIKI_OVERRIDE.get() + return override / "log.md" if override is not None else LOG_PATH + + +def manifest_path() -> Path: + """:data:`MANIFEST_PATH` (``/.citadel_ingested.json``), redirect-aware.""" + override = _WIKI_OVERRIDE.get() + return override / ".citadel_ingested.json" if override is not None else MANIFEST_PATH + + +def failures_path() -> Path: + """:data:`FAILURES_PATH` (``/.citadel_failures.json``), redirect-aware.""" + override = _WIKI_OVERRIDE.get() + return override / ".citadel_failures.json" if override is not None else FAILURES_PATH + + +@contextlib.contextmanager +def wiki_redirect(target: Path | str): + """Point :func:`wiki_dir` (and every path derived from it) at ``target`` for the duration of + the block, in THIS context only — a sibling thread's redirect, and the main thread's live + wiki, are unaffected. Restored on every exit path, including an exception. + + The raw/docs dirs are deliberately untouched: staging is a SIBLING of the live wiki, so every + relative citation the agent writes (``../../raw/x.md``) resolves identically before and after + the promote.""" + token = _WIKI_OVERRIDE.set(Path(target)) + try: + yield + finally: + _WIKI_OVERRIDE.reset(token) + + +def child_env() -> dict[str, str]: + """The environment to hand a CHILD process that must see THIS context's wiki: ``os.environ`` + plus an explicit ``CITADEL_WIKI_DIR``. The agentic CLI (and the ``citadel check`` it shells + out to) resolves its own config at import, so the redirect has to reach it through the + environment — passed per spawn rather than assigned into ``os.environ``, which is + process-global and would make two concurrent sessions overwrite each other's wiki.""" + env = dict(os.environ) + env["CITADEL_WIKI_DIR"] = str(wiki_dir()) + return env + + # Ingest backend: which coding-agent CLI to shell out to, and (for the claude # CLI) which model alias/id to pass. No API key is used. LLM_CLI: str = os.environ.get("CITADEL_LLM_CLI", "claude") @@ -745,6 +843,34 @@ def _pdf_text_mode() -> str: RESUME: bool = _bool_env("CITADEL_RESUME", True) +def _jobs_setting() -> int: + """Resolve ``CITADEL_JOBS`` to a worker count >= 1. A value below 1 is a misconfiguration, not + an "unlimited" request: it clamps to 1 (strictly serial — the default) and records a + :data:`CONFIG_WARNINGS` entry, so ``citadel doctor`` names it instead of silently ingesting + serially while the user believes otherwise.""" + value = _int_env("CITADEL_JOBS", 1) + if value < 1: + CONFIG_WARNINGS.append(f"CITADEL_JOBS={value} is not a worker count (>= 1) - using 1 (serial)") + return 1 + return value + + +# How many raw sources ingest may fold in CONCURRENTLY (`citadel ingest --jobs N` overrides it per +# run). 1 — the default — is the strictly serial behavior citadel has always had. Higher values run +# that many agent sessions at once, each against its OWN staging copy of the wiki (the isolation +# primitive was already there); promotion onto the live wiki stays serialized and base-aware, and a +# source whose session raced another one's promote is re-run serially before the run ends, so the +# wiki is never the sum of two sessions that could not see each other. +# +# The trade-off is cross-linking, not safety: concurrent sessions each read the wiki as it was when +# they started, so a page one of them creates is invisible to the others (a later `citadel curate` +# pass is the designed cleanup for that, and the serial re-run resolves the collisions). Keep it at +# 1 for the richest cross-linking; raise it for a large backlog of unrelated sources, where the run +# is dominated by per-session latency. Sensible ceiling: your agent CLI's own rate limits, which is +# what you will hit first — citadel imposes no maximum. +JOBS: int = _jobs_setting() + + def _page_cache_mode() -> str: """Resolve ``CITADEL_PAGE_CACHE`` to one of ``auto``/``on``/``off``, mirroring :func:`_pdf_text_mode`. Blank/unset and anything unrecognized mean ``auto``.""" diff --git a/citadel/curate.py b/citadel/curate.py index 9a532c5..db91487 100644 --- a/citadel/curate.py +++ b/citadel/curate.py @@ -563,7 +563,7 @@ def _page_text(rel_path: str) -> str | None: """The on-disk text of one wiki page, or None when it cannot be read — the raw bytes the ``--diff`` snapshots compare (no okf parse).""" try: - return okf.safe_join(config.WIKI_DIR, rel_path).read_text(encoding="utf-8") + return okf.safe_join(config.wiki_dir(), rel_path).read_text(encoding="utf-8") except (OSError, okf.OKFError): return None @@ -580,12 +580,12 @@ def _texts_on_disk() -> dict[str, str]: ``store``'s 'what is a page' rule (index/log/sources-catalog/dotfiles skipped) so the diff ignores generated files.""" out: dict[str, str] = {} - for dirpath, dirnames, filenames in os.walk(config.WIKI_DIR): + for dirpath, dirnames, filenames in os.walk(config.wiki_dir()): dirnames[:] = [d for d in dirnames if not d.startswith(".")] for name in filenames: if not name.endswith(".md") or store.is_skipped_name(name): continue - rel_path = os.path.relpath(os.path.join(dirpath, name), config.WIKI_DIR).replace(os.sep, "/") + rel_path = os.path.relpath(os.path.join(dirpath, name), config.wiki_dir()).replace(os.sep, "/") text = _page_text(rel_path) if text is not None: out[rel_path] = text @@ -625,7 +625,7 @@ def _select_pages(pages: list[Page], paths: list[str] | None) -> set[str] | None whole wiki. A path may be a rel_path (``concepts/x.md``) or an absolute/OS path under the wiki.""" if not paths: return None - wiki_root = config.WIKI_DIR.resolve() + wiki_root = config.wiki_dir().resolve() known = {p.rel_path for p in pages} wanted: set[str] = set() for arg in paths: @@ -696,7 +696,7 @@ def emit(event: str, **data) -> None: # failures save are all destructive under concurrency (see runlock's module docstring). The # dry-run path above stays lock-free — it is read-only by contract. with runlock.hold("curate"): - ingest._sweep_stale_staging(config.WIKI_DIR) + ingest._sweep_stale_staging(config.wiki_dir()) pages_by_path = {p.rel_path: p for p in pages} # One backlink-graph pass for the whole run; every cluster's findings slice it. inbound = store.inbound_map(pages) diff --git a/citadel/doctor.py b/citadel/doctor.py index daa0704..5249447 100644 --- a/citadel/doctor.py +++ b/citadel/doctor.py @@ -236,7 +236,7 @@ def check_manifest() -> Check: Reads the manifest through :func:`manifest.inspect` — ONE parse that also stashes the stamp for the mismatch probe below, so doctor never re-reads the file or reaches into manifest internals.""" - path = config.MANIFEST_PATH + path = config.manifest_path() fmt, count, error = manifest.inspect() if error == "missing": return Check(OK, "manifest", f"no manifest yet ({path.name}) - nothing ingested") @@ -434,7 +434,7 @@ def check_wiki_git() -> Check: if shutil.which("git") is None: detail = "git not found on PATH - wiki history skipped" return Check(WARN if mode == "init" else OK, "wiki git", detail) - state = wikigit.repo_state(Path(config.WIKI_DIR)) + state = wikigit.repo_state(Path(config.wiki_dir())) remote = f" (push: {config.WIKI_GIT_REMOTE})" if config.WIKI_GIT_REMOTE else "" if state == wikigit.REPO: return Check(OK, "wiki git", f"wiki dir is a git repo - changes commit after each ingest/curate{remote}") @@ -594,7 +594,7 @@ def check_workspace_coherence() -> Check: OK, "workspace coherence", f"all {total} source citations resolve under the configured raw/docs roots" ) page_rel, target, abs_path = example - suggested = config.WIKI_DIR.parent / "raw" + suggested = config.wiki_dir().parent / "raw" return Check( WARN, "workspace coherence", diff --git a/citadel/failures.py b/citadel/failures.py index 4d73fb9..6f2f32d 100644 --- a/citadel/failures.py +++ b/citadel/failures.py @@ -40,7 +40,7 @@ def load() -> dict[str, dict]: """json.loads(FAILURES_PATH) or {} if missing/empty/corrupt (mirrors :func:`manifest.load`).""" - path = config.FAILURES_PATH + path = config.failures_path() try: text = path.read_text(encoding="utf-8") except (OSError, FileNotFoundError): @@ -57,7 +57,7 @@ def load() -> dict[str, dict]: def save(failures: dict[str, dict]) -> None: """Write ``failures`` to FAILURES_PATH (sorted, indented, trailing newline), or REMOVE the file when empty so a wiki with nothing stuck carries no stale failures sidecar.""" - path = config.FAILURES_PATH + path = config.failures_path() if not failures: try: path.unlink() diff --git a/citadel/grammar.py b/citadel/grammar.py index c522fbf..3da87f1 100644 --- a/citadel/grammar.py +++ b/citadel/grammar.py @@ -352,7 +352,7 @@ def link_abs(page_rel: str, target: str) -> str | None: and works on synthetic or not-yet-existing paths.""" if is_external(target): return None - page_dir = os.path.dirname(str(config.WIKI_DIR / page_rel)) + page_dir = os.path.dirname(str(config.wiki_dir() / page_rel)) return os.path.normpath(os.path.join(page_dir, target)) diff --git a/citadel/ingest.py b/citadel/ingest.py index cb057ae..5e3892f 100644 --- a/citadel/ingest.py +++ b/citadel/ingest.py @@ -35,8 +35,10 @@ import stat import sys import tempfile +import threading import time from collections.abc import Callable +from concurrent import futures from dataclasses import dataclass, field from pathlib import Path, PurePosixPath @@ -107,6 +109,11 @@ class IngestReport: # rel-keys of tracked sources that VANISHED from disk (a full run only): their provenance is # reconciled out of the wiki by a cleanup agent session, then the manifest key is dropped. sources_deleted: list[str] = field(default_factory=list) + # `--jobs N` only: sources whose session raced a CONCURRENT source's promote over the same page + # and were therefore re-run serially afterwards (the re-run's own success/failure is reported + # like any other source's). Surfaced because it is the one place parallel ingest costs money a + # serial run would not have spent — a corpus that races often wants a lower --jobs. + raced: list[str] = field(default_factory=list) # Chunked sources that CONTINUED from an earlier run's checkpoint instead of restarting at # segment 1 ("raw/book.txt (segments 1-3 of 7 restored)") — see citadel/resume.py. Recorded # whether or not the resumed source then succeeded: the earlier work was reused either way. @@ -160,6 +167,9 @@ def render(self) -> str: if self.resumed: lines.append("Resumed (continued from an earlier run's checkpoint):") lines.extend(f" - {r}" for r in self.resumed) + if self.raced: + lines.append("Re-run serially (raced another source's promote over the same page):") + lines.extend(f" - {r}" for r in self.raced) if self.unreadable: lines.append("Unreadable (no extractable text; not ingested):") for p in self.unreadable: @@ -916,7 +926,7 @@ def _hash_pages(pages: list[Page]) -> dict[str, str]: snap: dict[str, str] = {} for page in pages: try: - target = okf.safe_join(config.WIKI_DIR, page.rel_path) + target = okf.safe_join(config.wiki_dir(), page.rel_path) snap[page.rel_path] = hashlib.sha256(target.read_bytes()).hexdigest() except (okf.OKFError, OSError): continue @@ -1100,6 +1110,20 @@ def _robust_copy_file(src: Path, dst: Path, attempts: int = _RMTREE_ATTEMPTS) -> # Monotonic per-process counter so each staging dir gets a UNIQUE name — see _make_staging. _STAGING_SEQ = 0 +# Guards the counter itself: with `--jobs N` several workers mint staging names at once, and two +# sources sharing a staging directory is exactly the merge-into-leftover-content failure the unique +# name exists to prevent. +_STAGING_SEQ_LOCK = threading.Lock() + +# THE serialization point for every touch of the LIVE wiki: cloning it into a staging copy (plus +# recording that clone's base state) and promoting a finished source back onto it. Sessions — the +# minutes-long part — run fully in parallel OUTSIDE this lock; what it serializes is milliseconds of +# file copying, so it costs throughput nothing and buys the two properties `--jobs N` needs: +# * a staging copy is a point-in-time image of the live wiki, never a half-promoted mixture; +# * two promotes can never interleave their copy-over and prune phases. +# It is a plain lock, not the workspace run lock (:mod:`runlock`): that one keeps two PROCESSES off +# one workspace, this one keeps two threads of ONE run off one live wiki. +_LIVE_WIKI_LOCK = threading.Lock() def _staging_prefix(live: Path) -> str: @@ -1138,8 +1162,10 @@ def _make_staging(live: Path) -> Path: global _STAGING_SEQ parent = live.parent prefix = _staging_prefix(live) - _STAGING_SEQ += 1 - staging = parent / f"{prefix}{os.getpid()}.{_STAGING_SEQ}" + with _STAGING_SEQ_LOCK: + _STAGING_SEQ += 1 + seq = _STAGING_SEQ + staging = parent / f"{prefix}{os.getpid()}.{seq}" _robust_rmtree(staging) # paranoia: clear an identical-named leftover before a clean copy try: if live.is_dir(): @@ -1156,30 +1182,19 @@ def _make_staging(live: Path) -> Path: return staging -@contextlib.contextmanager def _redirect_wiki(staging: Path): - """Point every wiki-derived config path — and ``CITADEL_WIKI_DIR`` for child processes (the agentic - CLI and the ``citadel check`` it shells out to) — at ``staging`` for the duration of one - session, so the agent reads/writes/validates the STAGING copy rather than the live wiki. The - raw/docs dirs are left untouched. Everything is restored on exit (including an originally-unset - ``CITADEL_WIKI_DIR``), so the redirect is invisible to the surrounding run.""" - staging = Path(staging) - saved = (config.WIKI_DIR, config.INDEX_PATH, config.LOG_PATH, config.MANIFEST_PATH) - env_had = "CITADEL_WIKI_DIR" in os.environ - env_prev = os.environ.get("CITADEL_WIKI_DIR") - config.WIKI_DIR = staging - config.INDEX_PATH = staging / "index.md" - config.LOG_PATH = staging / "log.md" - config.MANIFEST_PATH = staging / ".citadel_ingested.json" - os.environ["CITADEL_WIKI_DIR"] = str(staging) - try: - yield - finally: - config.WIKI_DIR, config.INDEX_PATH, config.LOG_PATH, config.MANIFEST_PATH = saved - if env_had: - os.environ["CITADEL_WIKI_DIR"] = env_prev # type: ignore[assignment] - else: - os.environ.pop("CITADEL_WIKI_DIR", None) + """Point every wiki-derived config path — and ``CITADEL_WIKI_DIR`` for the child processes the + session spawns (the agentic CLI and the ``citadel check`` it shells out to) — at ``staging`` + for the duration of one session, so the agent reads/writes/validates the STAGING copy rather + than the live wiki. The raw/docs dirs are left untouched. + + Thin alias for :func:`config.wiki_redirect`, kept under ingest's own name because this is where + the staging discipline lives. The redirect is CONTEXT-local, not process-global: it never + assigns ``config.WIKI_DIR`` or ``os.environ`` (the child's copy is built per spawn by + ``config.child_env``), which is precisely what lets ``--jobs N`` keep several sources staged at + once — each worker thread holds its own redirect, and the main thread still sees the live + wiki.""" + return config.wiki_redirect(staging) def _is_reserved_name(name: str) -> bool: @@ -1206,7 +1221,60 @@ def _content_files(root: Path) -> dict[str, Path]: return out -def _promote(staging: Path, live: Path, allow_emptying: bool = False) -> None: +def _sha256_or_none(path: Path) -> str | None: + """:func:`_sha256` that answers None instead of raising on an unreadable/vanished file — so a + comparison against a recorded base hash treats it as "not what the base had" (the conservative + answer) rather than blowing up a promote.""" + try: + return _sha256(path) + except OSError: + return None + + +def _content_hashes(root: Path) -> dict[str, str]: + """``{relposix: sha256}`` over :func:`_content_files` — a wiki's content state as bytes, not as + timestamps. Taken (`--jobs N` only) of the fresh STAGING clone, which is byte-for-byte the live + wiki this source started from, so its promote can tell "this page is exactly what I started + from" from "another source changed it while I was working". A file that vanishes or cannot be + read mid-walk is simply omitted, which reads as "absent" — the conservative answer, since it + makes the promote treat it as changed rather than silently overwriting it.""" + out: dict[str, str] = {} + for rel, path in _content_files(root).items(): + with contextlib.suppress(OSError): + out[rel] = _sha256(path) + return out + + +class _ConcurrentChange(Exception): + """The live wiki moved under a staged source: a page this promote would write or prune is no + longer what it was when the source was cloned, because a CONCURRENT source's promote (only + possible under ``--jobs N``) landed there first. + + Never a data-loss path — it is raised BEFORE the promote writes anything, so the live wiki keeps + the other source's work untouched. The caller re-runs this source SERIALLY, at the end of its + group: the fresh session then sees the page the other source wrote and merges into it, which is + exactly what a serial run would have done in the first place.""" + + +def _assert_base_unchanged(live: Path, base: dict[str, str], rels: set[str]) -> None: + """Raise :class:`_ConcurrentChange` if any of ``rels`` no longer matches ``base`` in the live + wiki. Only the paths a promote is about to TOUCH are checked: a concurrent source that created + or rewrote some unrelated page is no conflict at all — that is the whole point of running + sources in parallel.""" + for rel in sorted(rels): + target = live / rel + current: str | None = None + with contextlib.suppress(OSError): + if target.is_file(): + current = _sha256(target) + if current != base.get(rel): + raise _ConcurrentChange( + f"another source changed {rel} while this one was being folded in " + "(re-running it serially so it merges into the current wiki)" + ) + + +def _promote(staging: Path, live: Path, allow_emptying: bool = False, base: dict[str, str] | None = None) -> None: """Copy a validated STAGING wiki's CONTENT onto the LIVE wiki WITHOUT ever emptying or half-writing it. @@ -1224,7 +1292,26 @@ def _promote(staging: Path, live: Path, allow_emptying: bool = False) -> None: while the live wiki has some, the promote is REFUSED — raising so the caller fails the source and retries it next run, with the live wiki left exactly as it was rather than emptied. ``allow_emptying`` lifts that guard for a ``delete`` cleanup, where removing the last source's - only page legitimately leaves the wiki empty.""" + only page legitimately leaves the wiki empty. + + ``base`` — the content hashes of the wiki this source was CLONED from (:func:`_content_hashes` + over its staging copy, recorded only under ``--jobs N``) — makes the promote base-aware, which is what lets two sources + promote onto one wiki without eating each other's work: + + * WHAT IS WRITTEN is this source's own delta — the staging files that differ from the BASE, not + from the current live wiki. Staging also holds untouched copies of every page the source did + not write, and judging those by "differs from live" would let this promote revert a page a + concurrent source had just rewritten; + * the PRUNE is likewise derived from the base, so a page a concurrent source created (absent + from this source's base AND from its staging copy) is left alone instead of being deleted as + "the agent removed it"; + * every path this promote would write or prune must still match the base, else + :class:`_ConcurrentChange` is raised BEFORE anything is written and the source is re-run + serially — the two sessions disagreed about one page, and a stale session must never win. + + ``base=None`` (the default, and the whole of the serial path) keeps the original semantics + byte-for-byte: prune whatever live has and staging does not, no conflict check, no extra + hashing.""" staging, live = Path(staging), Path(live) config.robust_mkdir(live) @@ -1232,32 +1319,61 @@ def _promote(staging: Path, live: Path, allow_emptying: bool = False) -> None: live_content = _content_files(live) staging_pages = [r for r in staging_content if r.endswith(".md")] - live_pages = [r for r in live_content if r.endswith(".md")] - if not allow_emptying and not staging_pages and live_pages: + # What "the wiki had pages" means for the anti-emptying valve: with a base, the wiki AS THIS + # SOURCE FOUND IT — a page a concurrent source added in the meantime is not this session's to + # answer for, in either direction. + had_pages = [r for r in (live_content if base is None else base) if r.endswith(".md")] + if not allow_emptying and not staging_pages and had_pages: raise okf.OKFError( "refusing to promote: the session left the wiki with no content pages " "(treated as a failed source so the live wiki is not emptied)" ) + if base is None: + changed = {rel: src for rel, src in staging_content.items() if not _files_equal(src, live / rel)} + pruned = set(live_content) - set(staging_content) + else: + # With a base, a promote applies THIS source's own delta and nothing else. Which pages the + # source touched is decided against its base — not against the current live wiki — because + # its staging copy also holds untouched copies of every page it did NOT write: judging by + # "differs from live" would make a page a CONCURRENT source just rewrote look like this + # source's change, and copying the staging copy over it would silently revert that work. + changed = {rel: src for rel, src in staging_content.items() if _sha256_or_none(src) != base.get(rel)} + pruned = (set(base) & set(live_content)) - set(staging_content) + # NOTE for the reader: this is a per-FILE merge, not a per-line one. Two sources that wrote + # the same page do not get merged here — that is what the conflict below is for. + # Before the first byte is written: refuse a promote built on a wiki that has moved on. + _assert_base_unchanged(live, base, set(changed) | pruned) + # A page whose bytes already match live needs no write (a re-run that reproduced it exactly). + changed = {rel: src for rel, src in changed.items() if not _files_equal(src, live / rel)} + # 1. Copy-over FIRST (atomic per page, only when the bytes differ). - for rel, src in staging_content.items(): + for rel, src in changed.items(): dst = live / rel - if not _files_equal(src, dst): - config.robust_mkdir(dst.parent) - _robust_copy_file(src, dst) + config.robust_mkdir(dst.parent) + _robust_copy_file(src, dst) # 2. Prune the content pages the agent deleted (reserved/generated files are left untouched). - for rel in set(live_content) - set(staging_content): + for rel in pruned: with contextlib.suppress(OSError): (live / rel).unlink() # 3. Best-effort sweep of any leftover *.citadeltmp from an earlier promote that was hard-killed # between copyfile and os.replace. They are excluded from sync AND prune (reserved), so # without this they could linger on the live wiki indefinitely. + # Only the CONTENT temps this step can actually have created are swept: a HIDDEN temp + # (`.citadel_ingested.json..citadeltmp`) belongs to an in-flight atomic_write_text of the + # manifest/failures catalog — under `--jobs N` the main thread is saving one of those while a + # worker promotes, and deleting it out from under the writer turned a routine manifest save + # into a FileNotFoundError. Those temps are the writer's own to clean up. with contextlib.suppress(OSError): - for tmp in live.rglob("*.citadeltmp"): - with contextlib.suppress(OSError): - tmp.unlink() + for dirpath, dirnames, filenames in os.walk(live): + dirnames[:] = [d for d in dirnames if not d.startswith(".")] + for name in filenames: + if name.startswith(".") or not name.endswith(".citadeltmp"): + continue + with contextlib.suppress(OSError): + (Path(dirpath) / name).unlink() # 4. Drop directories left empty by the prune (bottom-up), but keep the live root itself. # Hidden trees are exempt, exactly like the sync/prune above (_content_files skips them): @@ -1299,6 +1415,11 @@ class _SourceOutcome: carried_usage: llm.SessionUsage | None = None # Human-readable note when this source continued from a checkpoint ("" when it did not). resumed_note: str = "" + # `--jobs N` only: the session was clean, but a CONCURRENT source's promote had changed a page + # this one would have written (:class:`_ConcurrentChange`), so nothing was promoted. Not a + # failure — the caller re-runs the source serially before the run ends, and only if THAT fails + # does it become one. + conflict: bool = False @dataclass @@ -1423,9 +1544,11 @@ def _sha_shared_by_other_entry(manifest_dict: dict, sha: str | None, exclude_key return False -def _checkpoint_delta(staging: Path, live: Path) -> tuple[list[str], list[str]] | None: - """``(changed, removed)`` — what promoting ``staging`` onto ``live`` right now would do — or - None when that delta must not be recorded at all. +def _checkpoint_delta( + staging: Path, live: Path, base: dict[str, str] | None = None +) -> tuple[list[str], list[str]] | None: + """``(changed, removed)`` — this source's own delta, exactly as :func:`_promote` computes it — + or None when it must not be recorded at all. Computed with the PROMOTE's own file-level view (:func:`_content_files` + :func:`_files_equal`), never from the per-segment page diffs: those miss the link repairs ``_repair_renames`` writes @@ -1433,15 +1556,28 @@ def _checkpoint_delta(staging: Path, live: Path) -> tuple[list[str], list[str]] even resurrect a page a later segment deliberately deleted. A checkpoint must describe exactly what would have shipped, so it is derived from exactly what ships. + ``base`` — the clone snapshot, present only under ``--jobs N`` — is what keeps that true when + the wiki is moving. Measured against the CURRENT live wiki, a page a CONCURRENT source created + between this source's clone and this checkpoint reads as "in live, not in my staging", i.e. as a + deletion THIS source made, and one it rewrote reads as a change of this source's with the clone's + stale bytes. A checkpoint is durable, so such a delta outlives the parallel run: replayed by a + later — even strictly serial — run, it deletes a fully-ingested source's page off the live wiki + with no conflict, no error and no delete session. Measured against the base, the delta is this + source's alone (the promote's exact rule), and the concurrent work is simply not in it. + The refusal mirrors the promote's anti-emptying valve: a staging tree with no content page while - live has some is a wipe-the-wiki delta (a vanished/rm-tree'd staging reads exactly like this), - and :func:`_promote` would refuse it — so it must never be persisted as a replayable one.""" + the wiki this source started from had some is a wipe-the-wiki delta (a vanished/rm-tree'd staging + reads exactly like this), and :func:`_promote` would refuse it — so it must never be persisted as + a replayable one.""" staged = _content_files(staging) - current = _content_files(live) - if not [rel for rel in staged if rel.endswith(".md")] and [rel for rel in current if rel.endswith(".md")]: + started_from = _content_files(live) if base is None else base + if not [rel for rel in staged if rel.endswith(".md")] and [rel for rel in started_from if rel.endswith(".md")]: return None - changed = sorted(rel for rel, src in staged.items() if not _files_equal(src, live / rel)) - removed = sorted(set(current) - set(staged)) + if base is None: + changed = sorted(rel for rel, src in staged.items() if not _files_equal(src, live / rel)) + else: + changed = sorted(rel for rel, src in staged.items() if _sha256_or_none(src) != base.get(rel)) + removed = sorted(set(started_from) - set(staged)) return changed, removed @@ -1487,7 +1623,9 @@ def _adopt_checkpoint(ctx: _Resume, staging: Path, live: Path, rel_key: str) -> return created, updated, sorted(rel for rel in ctx.checkpoint.removed if rel.endswith(".md")) -def _write_checkpoint(ctx: _Resume, completed: int, staging: Path, live: Path, usage: dict) -> None: +def _write_checkpoint( + ctx: _Resume, completed: int, staging: Path, live: Path, usage: dict, base: dict[str, str] | None = None +) -> None: """Record ``completed`` segments' work for this source. Best-effort by contract — any failure just means the next run starts at segment 1, so nothing here may raise into the session loop. @@ -1497,15 +1635,25 @@ def _write_checkpoint(ctx: _Resume, completed: int, staging: Path, live: Path, u strictly cheaper than the per-source staging COPY the same run already makes — and invisible beside the agent session each segment costs.""" try: - delta = _checkpoint_delta(staging, live) + delta = _checkpoint_delta(staging, live, base) if delta is not None: - resume.save(ctx.plan, completed, staging, live, delta[0], delta[1], usage) + # `base` travels on to resume.save as the state the delta must be guarded against: the + # wiki this source was CLONED from, not the (possibly moved-on) live wiki at save time. + # Recording the latter would have the replay guard verify "live unchanged since I + # saved" while the delta means "live as of my clone" — the wrong invariant, and one a + # concurrent promote slips straight through. + resume.save(ctx.plan, completed, staging, live, delta[0], delta[1], usage, bases=base) except Exception: # noqa: BLE001 - a checkpoint may never cost a run its source pass def _run_agent_sessions( - session_fns, rel_key: str, extra_check=None, allow_emptying: bool = False, resume_ctx: _Resume | None = None + session_fns, + rel_key: str, + extra_check=None, + allow_emptying: bool = False, + resume_ctx: _Resume | None = None, + concurrent: bool = False, ) -> _SourceOutcome: """Run one source's agent session(s) — a single pass, or every segment of a chunked source — against ONE staging copy, with full all-or-nothing safety. Shared by every job kind @@ -1543,12 +1691,21 @@ def _run_agent_sessions( loop to capture. Staging is always discarded in ``finally``. The caller owns the manifest + report bookkeeping (different for a completed source vs. a removed one). + ``concurrent`` (set only by the ``--jobs N`` driver) says another source may be staging or + promoting at the same time. It changes nothing about a session — it makes the two moments that + touch the LIVE wiki safe under that: the clone is taken under :data:`_LIVE_WIKI_LOCK` together + with a hash snapshot of what was cloned, and the promote runs under the same lock against that + base (see :func:`_promote`). A promote refused because a concurrent source got to one of these + pages first comes back as ``conflict`` rather than an error — the driver re-runs the source + serially, and the live wiki is untouched either way. + An EMPTY ``session_fns`` (a deleted source nothing cites) succeeds immediately with zero page changes — before a staging copy is even made.""" started = time.monotonic() if not session_fns: return _SourceOutcome(True) - live = config.WIKI_DIR + live = config.wiki_dir() + base: dict[str, str] | None = None staging: Path | None = None created: list[str] = [] updated: list[str] = [] @@ -1559,8 +1716,22 @@ def _run_agent_sessions( # What earlier runs already paid for the segments a checkpoint restores (see _SourceOutcome). carried: dict = {} resumed_note = "" + + def clone() -> tuple[Path, dict[str, str] | None]: + """A staging copy of the live wiki plus (concurrent runs only) the base state it copied. + + The lock covers the COPY alone; the base is then hashed off the fresh staging tree, which + is a byte-exact copy of exactly what was cloned — same hashes, half the disk I/O (the wiki + is read once, not twice), and every other worker's clone/promote is unblocked that much + sooner. It must still happen HERE, before anything mutates staging: a resume replay writes + into it a few lines below, and those pages are not part of the wiki this source started + from.""" + with _LIVE_WIKI_LOCK: + staging = _make_staging(live) + return staging, (_content_hashes(staging) if concurrent else None) + try: - staging = _make_staging(live) + staging, base = clone() # RESUME: replay an earlier run's completed segments into this fresh staging copy, so only # the remaining ones have to be paid for again. Every guard failure falls back to a full # start on a clean staging copy IN THIS RUN — never a failed source, never a wasted session. @@ -1571,7 +1742,7 @@ def _run_agent_sessions( resume.clear(rel_key) resume_ctx.checkpoint = None _robust_rmtree(staging) - staging = _make_staging(live) + staging, base = clone() else: created, updated, deleted = list(seeded[0]), list(seeded[1]), list(seeded[2]) start_at = resume_ctx.checkpoint.completed @@ -1613,6 +1784,7 @@ def _run_agent_sessions( staging, live, _usage_fields(llm.combine_usage([*usage_parts, _usage_from_fields(carried)])), + base, ) if i + 1 < len(session_fns): # Re-baseline on the validated/re-stamped state, so the next segment's diff @@ -1638,8 +1810,11 @@ def _run_agent_sessions( # Every session was clean: commit the source onto the live wiki (config now points back # at live). This is the ONLY step that touches the live wiki, it happens ONCE per source, - # and it is non-destructive — so an interrupt here still cannot empty it. - _promote(staging, live, allow_emptying=allow_emptying) + # and it is non-destructive — so an interrupt here still cannot empty it. Under `--jobs N` + # it is also the only step that has to exclude its siblings (one promote at a time, checked + # against the state this source was cloned from). + with _LIVE_WIKI_LOCK: + _promote(staging, live, allow_emptying=allow_emptying, base=base) if resume_ctx is not None: resume.clear(rel_key) # the work is live now: the checkpoint has nothing left to save return _SourceOutcome( @@ -1653,6 +1828,20 @@ def _run_agent_sessions( carried_usage=_usage_from_fields(carried), resumed_note=resumed_note, ) + except _ConcurrentChange as exc: + # Not a failure: the work was fine, the wiki simply moved under it (only reachable with + # `--jobs N`). Nothing was promoted, so the live wiki still holds the OTHER source's + # version; the driver re-runs this source serially against it. The spend is reported like + # any other — that session was paid for. + return _SourceOutcome( + False, + errors=[f"{rel_key}: {exc}"], + seconds=time.monotonic() - started, + usage=llm.combine_usage(usage_parts), + carried_usage=_usage_from_fields(carried), + resumed_note=resumed_note, + conflict=True, + ) except Exception as exc: # noqa: BLE001 - collect per-source, keep going; live wiki untouched # A raising session never returned its usage, but the backend may still have reported # what the FAILED attempt cost (claude's error envelope, gemini's stats file) — llm @@ -1715,7 +1904,141 @@ class _SourceJob: sha_stat: tuple[str | None, os.stat_result | None] = (None, None) -def _run_source_jobs(jobs: list[_SourceJob], emit, report: IngestReport, failures_dict, model) -> BaseException | None: +@dataclass +class _JobRun: + """One ATTEMPT at one source, as the driver sees it — the value a worker hands back. + + Exactly one of the three outcome fields is set: ``prepare_exc`` (planning raised — a per-source + failure, never a run-aborting one), ``interrupt`` (a ``BaseException`` such as Ctrl+C escaped the + session runner, which already rolled the source back), or ``outcome`` (the session runner's + verdict, including a ``conflict`` that asks for a serial re-run). ``index``/``total`` are the + source's position in its group — carried on the run so a re-run keeps the number the progress + output already showed.""" + + job: _SourceJob + index: int + total: int + outcome: _SourceOutcome | None = None + prepare_exc: Exception | None = None + interrupt: BaseException | None = None + + +def _attempt_source(job: _SourceJob, index: int, total: int, emit, concurrent: bool) -> _JobRun: + """Plan and run ONE source's session(s) — the whole minutes-long part — and return what + happened, touching NO shared bookkeeping. That is what makes it safe to call from a worker + thread under ``--jobs N``: the report, the manifest and the failures catalog are the main + thread's alone (:func:`_record_source_run`), so nothing here needs a lock beyond the live-wiki + one the session runner takes around its clone and promote.""" + # Keep the run lock's mtime fresh at every source boundary, so a long multi-source run never + # crosses the staleness window another process could reclaim the lock through. + runlock.heartbeat() + emit("source_start", index=index, total=total, source=job.key) + run = _JobRun(job=job, index=index, total=total) + # Plan the session(s). A prepare failure (a temp write, a digest build) is a per-source error, + # NOT a run-aborting one. + try: + sessions, tmpdirs, resume_ctx = job.build_sessions() + except Exception as exc: # noqa: BLE001 - per-source, keep going + run.prepare_exc = exc + return run + try: + run.outcome = _run_agent_sessions( + sessions, + job.key, + extra_check=job.extra_check, + allow_emptying=job.allow_emptying, + resume_ctx=resume_ctx, + concurrent=concurrent, + ) + except BaseException as exc: # noqa: BLE001 - Ctrl+C etc.: runner rolled back; captured + run.interrupt = exc + finally: + # Always remove every temp dir the plan produced (success, error, or interrupt). + for tmp in tmpdirs: + shutil.rmtree(tmp, ignore_errors=True) + return run + + +def _record_spend(outcome: _SourceOutcome, report: IngestReport) -> None: + """Book what one attempt COST and what it REUSED — the two facts that hold whatever its verdict + was, so they have exactly one owner instead of one per branch. + + The run's usage total counts every outcome: a failed (or raced) source's sessions were paid for + too; only the per-source manifest stamp is success-only. A restored checkpoint is recorded the + same way — the earlier segments were reused whether or not this attempt went on to promote. + + Called from :func:`_record_source_run` and, separately, from the ``--jobs N`` conflict path, + whose source is re-run serially instead of being recorded: it still spent a session and may + still have replayed a checkpoint, and a report that hid that would be understating the run. + Deliberately NOT wired through the interrupt path: an interrupted run re-raises and its report + is never rendered (_ingest_run's capture-finalize-reraise), so the in-flight source's partial + usage has no surface to appear on — the completed sources' manifest stamps were already saved + per-source with their usage intact.""" + report.usage = llm.combine_usage([report.usage, outcome.usage]) + if outcome.resumed_note: + report.resumed.append(outcome.resumed_note) + + +def _record_source_run(run: _JobRun, emit, report: IngestReport, failures_dict, model) -> None: + """Book ONE finished attempt into the run's shared state — report lists, the persistent + failures catalog, the job's success hook (manifest stamp + save), and the closing progress + event. MAIN THREAD ONLY, so the manifest/failures/report writes stay single-threaded exactly as + they were before ``--jobs N``; the concurrency lives entirely in :func:`_attempt_source`. + + Page changes reach the report only on success — a failed or interrupted source promotes + nothing, so the report claims nothing for it. A ``conflict`` outcome never reaches here: the + driver re-runs that source serially first.""" + job, index, total = run.job, run.index, run.total + sha, st = job.sha_stat + if run.prepare_exc is not None: + detail = f"{job.key}: {job.prepare_error}: {run.prepare_exc}" + report.errors.append(detail) + failures.record(failures_dict, job.key, failures.ERROR, detail, model, sha=sha, st=st) + emit("source_error", index=index, total=total, source=job.key, error=str(run.prepare_exc), seconds=0.0) + return + outcome = run.outcome + if outcome is None: # an interrupted attempt: the caller re-raises, nothing to book + return + _record_spend(outcome, report) # cost + any restored checkpoint, before the ok/failed branch + if not outcome.ok: + # Nothing was promoted (the live wiki is untouched) and the source is NOT marked + # done, so it is retried next run. Persist the failure for triage. + report.errors.extend(outcome.errors) + detail = outcome.errors[0] if outcome.errors else f"{job.key}: agent session failed" + failures.record(failures_dict, job.key, failures.reason_for(detail), detail, model, sha=sha, st=st) + emit( + "source_error", + index=index, + total=total, + source=job.key, + error=outcome.errors[0] if outcome.errors else "", + seconds=outcome.seconds, + ) + return + report.pages_created.extend(outcome.created) + report.pages_updated.extend(outcome.updated) + report.pages_written.extend(outcome.created + outcome.updated) + report.pages_deleted.extend(outcome.deleted) + # The manifest stamp covers every session whose work this promote landed — this run's plus + # whatever an earlier run already paid for the segments a checkpoint restored — while + # ``report.usage`` above stays strictly this run's spend, so nothing is double-counted + # across runs and `citadel status` never under-reports a resumed source. + job.on_success(llm.combine_usage([outcome.usage, outcome.carried_usage])) + emit( + "source_done", + index=index, + total=total, + source=job.key, + created=len(outcome.created), + updated=len(outcome.updated), + deleted=len(outcome.deleted), + seconds=outcome.seconds, + ) + + +def _run_source_jobs( + jobs: list[_SourceJob], emit, report: IngestReport, failures_dict, model, workers: int = 1 +) -> BaseException | None: """Drive one GROUP of :class:`_SourceJob`s (deletion cleanups, files, or repos) through the ONE shared per-source loop: emit ``source_start``, plan the session(s), run them all-or-nothing against a single staging copy, then either record the failure (report + persistent failures @@ -1726,81 +2049,124 @@ def _run_source_jobs(jobs: list[_SourceJob], emit, report: IngestReport, failure former loops emitted. Page changes reach the report only on success — a failed or interrupted source promotes nothing, so the report claims nothing for it. + ``workers`` > 1 (``citadel ingest --jobs N``) runs that many sources CONCURRENTLY — see + :func:`_run_source_jobs_parallel`. ``workers`` of 1, and any group of a single source, take the + serial path, which is the original loop line for line. + A ``BaseException`` (Ctrl+C) is RETURNED, not raised — the caller captures it, skips the remaining groups, finalizes the completed sources, and re-raises (the frozen capture-finalize-reraise pattern). The in-flight source was already rolled back by the session runner's ``finally``.""" + if workers <= 1 or len(jobs) <= 1: + return _run_serially(list(enumerate(jobs, 1)), len(jobs), emit, report, failures_dict, model) + return _run_source_jobs_parallel(jobs, emit, report, failures_dict, model, workers) + + +def _run_serially( + numbered: list[tuple[int, _SourceJob]], total: int, emit, report: IngestReport, failures_dict, model +) -> BaseException | None: + """Run ``(index, job)`` pairs one after another — the whole serial path, and the tail of the + parallel one (a source whose promote raced another is re-run here, keeping its original + index/total so the progress numbering stays honest).""" + for index, job in numbered: + run = _attempt_source(job, index, total, emit, concurrent=False) + if run.interrupt is not None: + return run.interrupt + _record_source_run(run, emit, report, failures_dict, model) + return None + + +def _run_source_jobs_parallel( + jobs: list[_SourceJob], emit, report: IngestReport, failures_dict, model, workers: int +) -> BaseException | None: + """Run one group's sources through a bounded thread pool, then re-run serially whichever of + them raced another source's promote. + + What each worker does is exactly what the serial loop does — plan, stage, run the session(s), + validate, promote — with two differences, both inside :func:`_run_agent_sessions`: the clone and + the promote take :data:`_LIVE_WIKI_LOCK`, and the promote is checked against the state its clone + was taken from. Everything a run SHARES (report lists, the manifest, the failures catalog) is + written here on the main thread as results arrive, so those writes stay serial and the manifest + is still saved per completed source. + + Sources are folded in in COMPLETION order, which is not submission order — the wiki is a set of + pages, not a log, so nothing depends on it; the report's lists simply read in the order sources + finished. + + Interrupts: a Ctrl+C reaches the main thread (and, being in the same process group, the agent + subprocesses too). The first ``BaseException`` from either side wins — queued sources are + cancelled and their workers told to stop before starting anything new, in-flight ones roll + themselves back — and it is returned for the caller's capture-finalize-reraise. Sources that had + already finished cleanly are still recorded, so an interrupt never throws away work the run + already paid for and promoted.""" total = len(jobs) - for index, job in enumerate(jobs, 1): - # Keep the run lock's mtime fresh at every source boundary, so a long multi-source run - # never crosses the staleness window another process could reclaim the lock through. - runlock.heartbeat() - emit("source_start", index=index, total=total, source=job.key) - sha, st = job.sha_stat - # Plan the session(s). A prepare failure (a temp write, a digest build) is a per-source - # error, NOT a run-aborting one. - try: - sessions, tmpdirs, resume_ctx = job.build_sessions() - except Exception as exc: # noqa: BLE001 - per-source, keep going - detail = f"{job.key}: {job.prepare_error}: {exc}" - report.errors.append(detail) - failures.record(failures_dict, job.key, failures.ERROR, detail, model, sha=sha, st=st) - emit("source_error", index=index, total=total, source=job.key, error=str(exc), seconds=0.0) - continue + abort = threading.Event() + interrupt: BaseException | None = None + conflicted: list[tuple[int, _SourceJob]] = [] + recorded: set[int] = set() + + def attempt(index: int, job: _SourceJob) -> _JobRun | None: + # A cancelled-too-late worker must not start a session: once the run is aborting, the only + # correct thing a queued source can do is nothing at all. + if abort.is_set(): + return None + return _attempt_source(job, index, total, emit, concurrent=True) + + with futures.ThreadPoolExecutor(max_workers=min(workers, total), thread_name_prefix="citadel-ingest") as pool: + submitted = [pool.submit(attempt, index, job) for index, job in enumerate(jobs, 1)] try: - outcome = _run_agent_sessions( - sessions, job.key, extra_check=job.extra_check, allow_emptying=job.allow_emptying, resume_ctx=resume_ctx - ) - except BaseException as exc: # noqa: BLE001 - Ctrl+C etc.: runner rolled back; captured - return exc - finally: - # Always remove every temp dir the plan produced (success, error, or interrupt). - for tmp in tmpdirs: - shutil.rmtree(tmp, ignore_errors=True) - # The run's usage total counts every outcome — a failed source's sessions were paid for - # too; only the per-source manifest stamp below is success-only. Deliberately NOT wired - # through the BaseException path (the return above): an interrupted run re-raises and - # its report is never rendered (_ingest_run's capture-finalize-reraise), so the in-flight - # source's partial usage has no surface to appear on — the completed sources' manifest - # stamps were already saved per-source with their usage intact. - report.usage = llm.combine_usage([report.usage, outcome.usage]) - if outcome.resumed_note: - # Recorded before the ok/failed branch: the earlier segments were reused either way. - report.resumed.append(outcome.resumed_note) - if not outcome.ok: - # Nothing was promoted (the live wiki is untouched) and the source is NOT marked - # done, so it is retried next run. Persist the failure for triage. - report.errors.extend(outcome.errors) - detail = outcome.errors[0] if outcome.errors else f"{job.key}: agent session failed" - failures.record(failures_dict, job.key, failures.reason_for(detail), detail, model, sha=sha, st=st) - emit( - "source_error", - index=index, - total=total, - source=job.key, - error=outcome.errors[0] if outcome.errors else "", - seconds=outcome.seconds, - ) - continue - report.pages_created.extend(outcome.created) - report.pages_updated.extend(outcome.updated) - report.pages_written.extend(outcome.created + outcome.updated) - report.pages_deleted.extend(outcome.deleted) - # The manifest stamp covers every session whose work this promote landed — this run's plus - # whatever an earlier run already paid for the segments a checkpoint restored — while - # ``report.usage`` above stays strictly this run's spend, so nothing is double-counted - # across runs and `citadel status` never under-reports a resumed source. - job.on_success(llm.combine_usage([outcome.usage, outcome.carried_usage])) - emit( - "source_done", - index=index, - total=total, - source=job.key, - created=len(outcome.created), - updated=len(outcome.updated), - deleted=len(outcome.deleted), - seconds=outcome.seconds, - ) + for future in futures.as_completed(submitted): + if future.cancelled(): + # A cancelled future is only ever produced by the abort below, so its + # `CancelledError` would be caught there and discarded in favour of the + # interrupt that caused it — correct, but only by that reasoning. Skipping it + # here (as the drain loop already does) keeps the property local: `result()` is + # called on completed work only. + continue + run = future.result() + if run is None: + continue + if run.interrupt is not None: + interrupt = interrupt if interrupt is not None else run.interrupt + abort.set() + for pending in submitted: + pending.cancel() + continue + if run.outcome is not None and run.outcome.conflict: + # Nothing was promoted, so this attempt is NOT recorded as a source outcome — + # but what it spent and what it replayed are facts of this run either way, and + # the source goes into the serial tail below (where it can no longer race + # anybody). + _record_spend(run.outcome, report) + report.raced.append(run.job.key) + conflicted.append((run.index, run.job)) + emit("source_retry", index=run.index, total=total, source=run.job.key, seconds=run.outcome.seconds) + continue + _record_source_run(run, emit, report, failures_dict, model) + recorded.add(run.index) + except BaseException as exc: # noqa: BLE001 - Ctrl+C while waiting: stop dispatching + interrupt = interrupt if interrupt is not None else exc + abort.set() + for pending in submitted: + pending.cancel() + # Leaving the `with` joins whatever is still running (their sessions roll back on the same + # interrupt); their results are drained below rather than dropped. + if interrupt is not None: + for future in submitted: + if not future.done() or future.cancelled(): + continue + with contextlib.suppress(Exception): + run = future.result() + # Only completed, PROMOTED work is booked during an abort: it is already on the live + # wiki, so leaving it out of the manifest would just make the next run pay again. + if run is not None and run.outcome is not None and run.outcome.ok and run.index not in recorded: + _record_source_run(run, emit, report, failures_dict, model) + recorded.add(run.index) + return interrupt + if conflicted: + # The serial tail: no other source can be promoting now, so these re-runs see the wiki the + # winner left behind and merge into it — the result a serial run would have produced. + return _run_serially(sorted(conflicted, key=lambda pair: pair[0]), total, emit, report, failures_dict, model) return None @@ -2002,7 +2368,11 @@ def _pending_session( @pagecache.bypass def ingest( - paths: list[str] | None = None, progress=None, full_rescan: bool = False, force: bool = False + paths: list[str] | None = None, + progress=None, + full_rescan: bool = False, + force: bool = False, + jobs: int | None = None, ) -> IngestReport: """Run one ingest. Exactly one source = one all-or-nothing agent job (a chunked source runs several ``llm.run_ingest_session`` passes inside that one job). @@ -2061,10 +2431,23 @@ def ingest( :func:`_run_source_jobs`): deletion cleanups, files, and repos differ only in how their sessions are planned and in their post-success bookkeeping. + ``jobs`` (``citadel ingest --jobs N``; None takes :data:`config.JOBS`, default 1) is how many + sources may be folded in CONCURRENTLY. 1 is the strictly serial behavior citadel has always + had. Above 1, each source still gets its own staging copy and its own all-or-nothing promote — + what changes is that N of them are in flight at once, promoting one at a time against the wiki + state they were cloned from; a source whose promote raced another one over the same page is + re-run serially before the run ends (``report.raced``). Every guarantee is unmoved: one promote + per source, nothing partial on the live wiki, the manifest still saved per completed source. The + real cost is cross-linking — concurrent sessions cannot see each other's new pages — which is + why the default stays 1 and the knob is documented as a throughput trade, not a free win. + ``progress`` is an optional ``progress(event, data)`` callback (run start, before/after each source, before finalization); None for non-interactive callers. A failing callback never breaks ingest. """ + workers = config.JOBS if jobs is None else jobs + if workers < 1: + raise ValueError(f"--jobs must be at least 1 (got {workers}); 1 means the serial default.") if force and not paths: # The API-layer twin of the CLI's exit-2 refusal (which pre-empts this with the same # message), so a programmatic caller cannot force the whole corpus by accident either. @@ -2078,18 +2461,29 @@ def ingest( # failures saves are all destructive under concurrency (see runlock's module docstring). # A second run fails loud here instead of silently eating the first one's work. with runlock.hold("ingest"): - _sweep_stale_staging(config.WIKI_DIR) + _sweep_stale_staging(config.wiki_dir()) # Same place, same reason: under the exclusive lock, leftovers on disk belong to dead runs. # Age-based only — a checkpoint's own guards decide whether it is USABLE (see resume.sweep). resume.sweep() - return _ingest_run(paths, progress, full_rescan=full_rescan, force=force) + return _ingest_run(paths, progress, full_rescan=full_rescan, force=force, jobs=workers) -def _ingest_run(paths: list[str] | None, progress, *, full_rescan: bool, force: bool) -> IngestReport: +def _ingest_run(paths: list[str] | None, progress, *, full_rescan: bool, force: bool, jobs: int = 1) -> IngestReport: """The body of :func:`ingest`, running under the exclusive workspace run lock.""" + # `--jobs N` emits from WORKER threads (a source's start/done event fires where the work + # happens), so the callback — which is whatever the caller passed — is serialized here. That + # keeps "your progress callback is never invoked concurrently" a property of the API rather than + # something each caller has to discover, and it costs nothing: emit fires a handful of times per + # source, around sessions that take minutes. The console reporter is safe under it either way + # (its writes are locked and the spinner is off when jobs > 1), but the contract must not depend + # on that reasoning holding for every future callback. + emit_lock = threading.Lock() + def emit(event: str, **data) -> None: - if progress is not None: + if progress is None: + return + with emit_lock: try: progress(event, data) except Exception: # noqa: BLE001 - progress must never break ingest @@ -2350,6 +2744,7 @@ def emit(event: str, **data) -> None: unreadable=len(report.unreadable), deleted=len(deleted_sources), repos=len(repo_pending), + jobs=jobs, ) # --- The per-source jobs (the SourceJob loop): DELETION cleanups first, then files, then repos, @@ -2555,7 +2950,7 @@ def done(_usage: llm.SessionUsage | None) -> None: ) for group in groups: if pending_interrupt is None: - pending_interrupt = _run_source_jobs(group, emit, report, failures_dict, model) + pending_interrupt = _run_source_jobs(group, emit, report, failures_dict, model, workers=jobs) if workspace_shifted and full_rescan: # The guard's advertised remedy must not loop: --full-rescan keeps the sweep refused diff --git a/citadel/linkgraph.py b/citadel/linkgraph.py index 2c8c6e0..df95f18 100644 --- a/citadel/linkgraph.py +++ b/citadel/linkgraph.py @@ -92,7 +92,7 @@ def rewrite_links(rename_map: dict[str, str], pages: list[Page] | None = None) - for page in pages: new_body = _rewrite_body_links(page.rel_path, page.body, rename_map) if new_body != page.body: - target = okf.safe_join(config.WIKI_DIR, page.rel_path) + target = okf.safe_join(config.wiki_dir(), page.rel_path) target.write_text(okf.dump(page.frontmatter, new_body), encoding="utf-8") changed.append(page.rel_path) return changed @@ -121,7 +121,7 @@ def source_key_to_page_link(page_rel: str, key: str) -> str: link still resolves rather than raising. Emitted through ``grammar.format_link_target``, so a key containing spaces comes back angle-wrapped — the ONE parseable citation form — and every emitter (the citation rewriter, sources/index.md, the index reflinks) stays round-trippable.""" - page_dir = os.path.dirname(str(config.WIKI_DIR / page_rel)) + page_dir = os.path.dirname(str(config.wiki_dir() / page_rel)) target_abs = str(config.source_path_for_key(key)) try: link = os.path.relpath(target_abs, page_dir).replace(os.sep, "/") @@ -173,7 +173,7 @@ def rewrite_raw_references(old_rel: str, new_rel: str, pages: list[Page] | None frontmatter["resource"] = new_rel new_body = _rewrite_raw_body_links(page.rel_path, page.body, old_rel, new_rel) if fm_changed or new_body != page.body: - target = okf.safe_join(config.WIKI_DIR, page.rel_path) + target = okf.safe_join(config.wiki_dir(), page.rel_path) target.write_text(okf.dump(frontmatter, new_body), encoding="utf-8") changed.append(page.rel_path) return changed diff --git a/citadel/llm.py b/citadel/llm.py index bacbc1b..23bc1b5 100644 --- a/citadel/llm.py +++ b/citadel/llm.py @@ -183,7 +183,7 @@ def _external_dirs(rel_key: str, read_path: str | None = None) -> list[str]: Empty for the all-under-workspace dev-checkout layout (cwd already covers the rules), so that invocation is byte-for-byte unchanged.""" candidates = [ - config.WIKI_DIR, + config.wiki_dir(), *config.source_roots(), # every raw source root (multi-root: an out-of-workspace root needs a grant) config.DOCS_DIR, config.PACKAGED_RULES_DIR, @@ -389,7 +389,7 @@ def _build_instruction( Office source, one segment's slice of a large source (with ``segment=(part, total)``), or a repo digest — the agent reads it for content while citing ``rel_key`` as the source of record (per the task/format briefs).""" - wiki_rel = _agent_path(config.WIKI_DIR) + wiki_rel = _agent_path(config.wiki_dir()) # The raw-dir bullet names the root that COVERS this source (in a multi-root corpus a # second-root source must not be pointed at the primary); an out-of-root explicit path # falls back to the primary RAW_DIR. Lexical lookup — no disk access, delete-safe. @@ -701,8 +701,12 @@ def _last_result_envelope(text: str) -> dict | None: return found -# Monotonic per-process counter so concurrent/same-second transcript files never collide. +# Monotonic per-process counter so concurrent/same-second transcript files never collide. Under +# `citadel ingest --jobs N` "concurrent" is literal — several worker threads write transcripts at +# once — and `_LOG_SEQ += 1` is a read-modify-write, so without the lock two sessions can be handed +# the same number and the guarantee this counter exists for quietly stops holding. _LOG_SEQ = 0 +_LOG_SEQ_LOCK = threading.Lock() def _decode_partial(data) -> str: @@ -746,6 +750,7 @@ def _stream_subprocess(cli: str, argv: list[str], stdin_text: str | None) -> tup errors="replace", bufsize=1, cwd=str(config.WORKSPACE_ROOT), + env=config.child_env(), ) if stdin_text is not None and proc.stdin is not None: try: @@ -804,14 +809,16 @@ def _write_transcript( return try: global _LOG_SEQ - _LOG_SEQ += 1 + with _LOG_SEQ_LOCK: + _LOG_SEQ += 1 + seq = _LOG_SEQ directory = Path(log_dir) if not directory.is_absolute(): directory = config.WORKSPACE_ROOT / directory config.robust_mkdir(directory) stamp = time.strftime("%Y%m%d-%H%M%S") safe = "".join(c if (c.isalnum() or c in "-._") else "_" for c in (label or "session"))[:80] - path = directory / f"{stamp}.{os.getpid()}.{_LOG_SEQ}.{safe}.log" + path = directory / f"{stamp}.{os.getpid()}.{seq}.{safe}.log" body = [ "# citadel ingest — LLM agent session transcript", f"time: {stamp}", @@ -869,6 +876,10 @@ def _run_session( errors="replace", timeout=config.LLM_TIMEOUT, cwd=str(config.WORKSPACE_ROOT), + # The wiki this session must edit is passed EXPLICITLY (ingest's per-source staging + # copy, via config.wiki_redirect) rather than through a process-global + # os.environ assignment — two concurrent sessions each get their own. + env=config.child_env(), ) returncode, out_raw, err_raw = proc.returncode, proc.stdout, proc.stderr except subprocess.TimeoutExpired as exc: @@ -958,7 +969,7 @@ def run_ingest_session( file is the WHOLE transcript/extraction, shared by every pass, so locator line numbers never rebase. - The agent's edits under ``config.WIKI_DIR`` are the real result — ``ingest`` discovers what + The agent's edits under ``config.wiki_dir()`` are the real result — ``ingest`` discovers what changed via a filesystem diff. The return value is only the session's best-effort :class:`SessionUsage` (the backend's own cost/usage report: claude's result envelope, gemini's ``--session-summary`` stats file when its binary advertises the flag; None when diff --git a/citadel/manifest.py b/citadel/manifest.py index 5dacdad..9166f79 100644 --- a/citadel/manifest.py +++ b/citadel/manifest.py @@ -367,7 +367,7 @@ def _check_workspace(meta: dict) -> None: return _warned_workspaces.add(stamped) print( - f"WARNING: the ingest manifest ({config.MANIFEST_PATH}) was written by a workspace rooted at\n" + f"WARNING: the ingest manifest ({config.manifest_path()}) was written by a workspace rooted at\n" f" {stamped}\n" f"but the current workspace root is\n" f" {current}\n" @@ -382,7 +382,7 @@ def _read() -> tuple[object, str | None]: JSON value and None on success, else ``(None, code)`` with ``code`` one of ``"missing"`` (no file), ``"empty"`` (present but blank), or ``"corrupt"`` (unparseable JSON). The single reader both :func:`load` and :func:`inspect` share, so neither re-reads the file.""" - path = config.MANIFEST_PATH + path = config.manifest_path() try: text = path.read_text(encoding="utf-8") except UnicodeError: @@ -485,7 +485,7 @@ def save(manifest: dict[str, Entry]) -> None: """Write ``{"meta": {format, workspace}, "sources": manifest}`` to MANIFEST_PATH (sort_keys, indent=2, trailing newline). ``manifest`` is the flat sources dict the callers hold; the workspace stamp records WHICH workspace the keys are relative to.""" - path = config.MANIFEST_PATH + path = config.manifest_path() config.robust_mkdir(path.parent) data = {"meta": {"format": MANIFEST_FORMAT, "workspace": _workspace_stamp()}, "sources": manifest} text = json.dumps(data, sort_keys=True, indent=2) + "\n" diff --git a/citadel/pagecache.py b/citadel/pagecache.py index d3a674f..1d4cf85 100644 --- a/citadel/pagecache.py +++ b/citadel/pagecache.py @@ -21,7 +21,7 @@ window is checked when the snapshot is STORED and never again, and that is sufficient: once a stamp is older than the coarsest tick, any later write necessarily lands in a later tick and so moves it. -3. *A single slot, keyed by the wiki directory.* Ingest redirects ``config.WIKI_DIR`` at its +3. *A single slot, keyed by the wiki directory.* Ingest redirects ``config.wiki_dir()`` at its per-source staging copy; a snapshot of one directory can never be served for another, and the single slot means the unbounded stream of staging dirs cannot accumulate entries. diff --git a/citadel/pdftext.py b/citadel/pdftext.py index 593382a..5da9fd4 100644 --- a/citadel/pdftext.py +++ b/citadel/pdftext.py @@ -98,7 +98,7 @@ def is_pdf_text_source(path: Path) -> bool: def cache_dir() -> Path: """The extraction cache directory: a dotdir sibling of the wiki dir (read at call time so tests can monkeypatch the config layout — exactly like ``transcribe.cache_dir``).""" - return Path(config.WIKI_DIR).parent / CACHE_DIR_NAME + return Path(config.wiki_dir()).parent / CACHE_DIR_NAME # A sha256 hexdigest and nothing else — the ONLY string shape allowed to become a cache filename diff --git a/citadel/progress.py b/citadel/progress.py index 00f321e..d053803 100644 --- a/citadel/progress.py +++ b/citadel/progress.py @@ -64,8 +64,22 @@ def _ignore(self, **_) -> None: pass def on_start( - self, pending: int, skipped: int, moved: int = 0, unreadable: int = 0, deleted: int = 0, repos: int = 0 + self, + pending: int, + skipped: int, + moved: int = 0, + unreadable: int = 0, + deleted: int = 0, + repos: int = 0, + jobs: int = 1, ) -> None: + # `--jobs N`: several sources are in flight at once, so there is no single "current source" + # for the spinner to name — it would be restarted and relabelled by whichever worker last + # began, and its rewritten line would swallow the others' completion lines. Fall back to the + # same START-line-per-source mode the non-TTY/--verbose paths already use, which reads + # correctly when the lines interleave. + if jobs > 1: + self.spinner = False bits = [] if skipped: bits.append(f"{skipped} already up to date") @@ -120,6 +134,17 @@ def on_source_done( summary = ", ".join(bits) if bits else "no changes" self._finishln(f"[{index}/{total}] OK {config.display_key(source)} - {summary} ({seconds:.1f}s)") + def on_source_retry(self, index: int, total: int, source: str, seconds: float) -> None: + """``--jobs N`` only: this source's session was clean but a CONCURRENT source promoted a + page it also wrote, so nothing was promoted and it is re-run serially at the end of its + group (where it merges into what the other source left). Neither an OK nor an ERR — the + source's real verdict is the re-run's.""" + self._stop_spinner() + self._finishln( + f"[{index}/{total}] RE-RUN {config.display_key(source)} - " + f"raced another source; re-running serially ({seconds:.1f}s)" + ) + def on_source_error(self, index: int, total: int, source: str, error: str, seconds: float) -> None: self._stop_spinner() self._finishln(f"[{index}/{total}] ERR {config.display_key(source)} - {error} ({seconds:.1f}s)") diff --git a/citadel/refresh.py b/citadel/refresh.py index 73dd6b5..fde3633 100644 --- a/citadel/refresh.py +++ b/citadel/refresh.py @@ -137,13 +137,20 @@ def plan(min_age_days: int = 0) -> list[RefreshCandidate]: return _age_floor(out, min_age_days) -def refresh(limit: int = 1, min_age_days: int = 0, dry_run: bool = False, progress=None) -> RefreshReport: +def refresh( + limit: int = 1, min_age_days: int = 0, dry_run: bool = False, progress=None, jobs: int | None = None +) -> RefreshReport: """Run one refresh: take the ``limit`` least-recently-checked sources off the queue and re-verify each through a forced ingest run (``kind="reconcile"`` per source, all-or-nothing staging, manifest re-stamped on success). ``limit`` must be >= 1 — the budget is always explicit, an unbounded refresh (one agent session per source, corpus-wide) can never happen by accident, mirroring ``--force``'s refusal without paths. ``dry_run`` computes and returns - the plan with zero sessions. ``progress`` is threaded through to ingest untouched.""" + the plan with zero sessions. ``progress`` is threaded through to ingest untouched. + + ``jobs`` (None = :data:`config.JOBS`, default 1 = serial) is handed to ingest as-is: a refresh + slice is ordered by LAST-CHECKED time, not by topic, so its sources are typically unrelated — + the case ``--jobs`` exists for. Everything the knob costs and guarantees is ingest's + (:func:`citadel.ingest.ingest`); refresh only chooses which sources to hand it.""" if limit < 1: raise ValueError("refresh limit must be >= 1 (each refreshed source runs one agent session).") eligible = plan() @@ -157,5 +164,5 @@ def refresh(limit: int = 1, min_age_days: int = 0, dry_run: bool = False, progre # Hand ingest ABSOLUTE paths: manifest keys are workspace-relative, but ingest resolves # requested path strings against the CWD — which need not be the workspace root. paths = [str(config.source_path_for_key(c.key)) for c in selected] - report.ingest_report = ingest.ingest(paths, progress=progress, force=True) + report.ingest_report = ingest.ingest(paths, progress=progress, force=True, jobs=jobs) return report diff --git a/citadel/resume.py b/citadel/resume.py index 9fe3a50..5129c5d 100644 --- a/citadel/resume.py +++ b/citadel/resume.py @@ -109,9 +109,9 @@ def enabled() -> bool: def cache_dir() -> Path: """The checkpoint store: a dotdir sibling of the wiki dir. Read at call time — and deliberately derived from the wiki's PARENT, so it resolves to the same directory while ingest has - ``config.WIKI_DIR`` redirected at a per-source staging copy (staging is a SIBLING of the live + ``config.wiki_dir()`` redirected at a per-source staging copy (staging is a SIBLING of the live wiki — the same reason ``transcribe``/``pdftext``/``runlock`` resolve their siblings this way).""" - return Path(config.WIKI_DIR).parent / CACHE_DIR_NAME + return Path(config.wiki_dir()).parent / CACHE_DIR_NAME @dataclass(frozen=True) @@ -182,14 +182,30 @@ def slot_for(key: str) -> Path: return cache_dir() / hashlib.sha256(key.encode("utf-8")).hexdigest()[:32] -def save(plan: Plan, completed: int, staging: Path, live: Path, changed, removed, usage: dict) -> bool: +def save( + plan: Plan, + completed: int, + staging: Path, + live: Path, + changed, + removed, + usage: dict, + bases: dict[str, str] | None = None, +) -> bool: """Record ``completed`` segments' work as this source's checkpoint; True when one is on disk afterwards. - ``changed`` are the rel_paths whose staged bytes differ from live (new or rewritten) and - ``removed`` the ones the segments took away — i.e. exactly the promote that WOULD happen right - now. The blobs are copied out of ``staging`` and the live wiki's current hash for every touched - path is recorded as the base state a later replay must still find. + ``changed`` are the rel_paths whose staged bytes differ from the state this source started from + (new or rewritten) and ``removed`` the ones the segments took away — i.e. exactly the promote + that WOULD happen. The blobs are copied out of ``staging``, and for every touched path the state + the delta was measured against is recorded as the base a later replay must still find. + + ``bases`` IS that state, when the caller knows it: the clone snapshot ``citadel ingest --jobs N`` + takes of the live wiki per source. Without it the base is read from ``live`` right now, which is + the same thing on a serial run (nothing else promotes) and WRONG on a concurrent one — the + delta would then be guarded against a wiki that had already moved on since it was computed, so + the guard would confirm "nobody touched these since I saved" while the delta itself embodies a + stale view. A page a concurrent source has changed must fail the guard, not pass it. Best-effort by contract: any problem returns False and leaves the source to restart at segment 1. A checkpoint can never fail a run — the money is already spent either way. @@ -229,11 +245,16 @@ def save(plan: Plan, completed: int, staging: Path, live: Path, changed, removed if sha is None: # unreadable right after writing it: refuse rather than record a lie return _abort(staged_new) blobs[rel] = sha - bases = _live_state(Path(live), list(blobs) + list(removed)) + touched = list(blobs) + list(removed) + # A caller-supplied base is a snapshot mapping (present -> sha, absent -> no key), so a + # missing key means "absent", exactly what _live_state records as None. + base_state: dict[str, str | None] = ( + _live_state(Path(live), touched) if bases is None else {rel: bases.get(rel) for rel in touched} + ) # A recorded deletion whose base is unknown could not be guarded on replay — and an # unguarded deletion is the one operation that can destroy another source's work. The same # goes for any path whose live state could not be read at all: no base, no checkpoint. - if any(bases.get(rel) is None for rel in removed) or UNREADABLE in bases.values(): + if any(base_state.get(rel) is None for rel in removed) or UNREADABLE in base_state.values(): return _abort(staged_new) record = { "format": FORMAT, @@ -247,7 +268,7 @@ def save(plan: Plan, completed: int, staging: Path, live: Path, changed, removed "workspace": workspace_stamp(), "pages": blobs, "removed": list(removed), - "bases": bases, + "bases": base_state, "usage": _clean_usage(usage), } config.atomic_write_text(staged_new / RECORD_NAME, json.dumps(record, indent=2, sort_keys=True) + "\n") diff --git a/citadel/runlock.py b/citadel/runlock.py index 4365881..860ab4a 100644 --- a/citadel/runlock.py +++ b/citadel/runlock.py @@ -45,7 +45,7 @@ class RunLockError(RuntimeError): def lock_path() -> Path: """The lockfile's location: a dotfile sibling of the wiki dir (read at call time so tests can monkeypatch the config layout).""" - return config.WIKI_DIR.parent / LOCK_NAME + return config.wiki_dir().parent / LOCK_NAME def _stale_after_s() -> float: diff --git a/citadel/server.py b/citadel/server.py index e2afb89..a9cc38f 100644 --- a/citadel/server.py +++ b/citadel/server.py @@ -390,7 +390,7 @@ def wiki_validate(rel_path: str = "") -> str: # with zero issues and a typo'd path would otherwise be indistinguishable. if want not in {p.rel_path for p in pages}: try: - on_disk = okf.safe_join(config.WIKI_DIR, want).is_file() + on_disk = okf.safe_join(config.wiki_dir(), want).is_file() except okf.OKFError: on_disk = False if on_disk: diff --git a/citadel/store_core.py b/citadel/store_core.py index 0fe8368..2f8b5a1 100644 --- a/citadel/store_core.py +++ b/citadel/store_core.py @@ -38,7 +38,7 @@ def is_skipped_name(name: str) -> bool: def load() -> list[Page]: - """Walk config.WIKI_DIR; parse each *.md (not index.md/log.md, not a dotfile) + """Walk config.wiki_dir(); parse each *.md (not index.md/log.md, not a dotfile) into a Page whose rel_path is its posix path relative to WIKI_DIR. Missing 'type' is surfaced by lint, not load, so failing pages are still included. Return the list sorted by rel_path. @@ -48,7 +48,7 @@ def load() -> list[Page]: re-parse. The cache is off by default, always off inside the mutating lifecycles, and every consult re-checks the filesystem, so this function's contract is unchanged: it never returns anything a fresh walk would not have produced.""" - wiki_dir = config.WIKI_DIR + wiki_dir = config.wiki_dir() cached = pagecache.get(wiki_dir) if cached is not None: return cached @@ -346,7 +346,7 @@ def search(query: str, pages: list[Page] | None = None, limit: int = 8) -> list[ def read_page(rel_path: str) -> Page: """okf.safe_join(WIKI_DIR, rel_path); read text; okf.parse; return Page. Raise FileNotFoundError if absent.""" - target = okf.safe_join(config.WIKI_DIR, rel_path) + target = okf.safe_join(config.wiki_dir(), rel_path) if not target.is_file(): raise FileNotFoundError(rel_path) text = target.read_text(encoding="utf-8") @@ -379,7 +379,7 @@ def read_page_text(rel_path: str) -> str: an OS/decoding error (an undecodable file) — the callers translate those into a CLI exit code or an MCP error string.""" rel_path = _normalize_rel_path(rel_path) - target = okf.safe_join(config.WIKI_DIR, rel_path) + target = okf.safe_join(config.wiki_dir(), rel_path) if not target.is_file(): raise FileNotFoundError(rel_path) return target.read_text(encoding="utf-8-sig") @@ -399,7 +399,7 @@ def neighbors_text(rel_path: str) -> str: from . import grammar, linkgraph rel_path = _normalize_rel_path(rel_path) - okf.safe_join(config.WIKI_DIR, rel_path) # validate the path (raises okf.OKFError on traversal/escape) + okf.safe_join(config.wiki_dir(), rel_path) # validate the path (raises okf.OKFError on traversal/escape) pages = load() by_path = {p.rel_path: p for p in pages} page = by_path.get(rel_path) @@ -520,13 +520,13 @@ def define_text(term: str, pages: list[Page] | None = None) -> str: def index_text() -> str: """The generated ``wiki/index.md`` catalog text. Raises FileNotFoundError when no index exists yet (nothing ingested), or an OS error when the path is unreadable.""" - return config.INDEX_PATH.read_text(encoding="utf-8") + return config.index_path().read_text(encoding="utf-8") def sources_text() -> str: """The generated ``wiki/sources/index.md`` provenance catalog text. Raises FileNotFoundError when nothing has been ingested yet, or an OS error when the path is unreadable.""" - return config.SOURCES_INDEX_PATH.read_text(encoding="utf-8") + return config.sources_index_path().read_text(encoding="utf-8") def _is_reserved_name(rel_path: str) -> bool: @@ -554,7 +554,7 @@ def write_page(rel_path: str, frontmatter: dict, body: str) -> Page: if _is_reserved_name(rel_path): raise okf.OKFError(f"refusing to write protected file: {rel_path!r}") okf.validate(frontmatter) - target = okf.safe_join(config.WIKI_DIR, rel_path) + target = okf.safe_join(config.wiki_dir(), rel_path) config.robust_mkdir(target.parent) frontmatter = dict(frontmatter) frontmatter["timestamp"] = utc_now_iso() @@ -575,7 +575,7 @@ def delete_page(rel_path: str) -> bool: folder is inert and drops out of the catalog on the next rebuild.""" if _is_reserved_name(rel_path): raise okf.OKFError(f"refusing to delete protected file: {rel_path!r}") - target = okf.safe_join(config.WIKI_DIR, rel_path) # rejects ''/absolute/'..' + target = okf.safe_join(config.wiki_dir(), rel_path) # rejects ''/absolute/'..' if target.is_file(): target.unlink() pagecache.invalidate() # as in write_page: the next load() must not see the page @@ -605,7 +605,7 @@ def append_log(line: str) -> None: entries are grouped under ``## YYYY-MM-DD`` date headings (per the OKF reserved-file convention). A new date heading is opened the first time a given UTC day is logged; prior lines are never rewritten (append-only audit trail).""" - log_path = config.LOG_PATH + log_path = config.log_path() config.robust_mkdir(log_path.parent) now = datetime.now(timezone.utc) day, stamp = now.strftime("%Y-%m-%d"), now.strftime("%H:%M:%SZ") diff --git a/citadel/templates/env.example b/citadel/templates/env.example index 5187cc0..e62e4ef 100644 --- a/citadel/templates/env.example +++ b/citadel/templates/env.example @@ -64,6 +64,14 @@ CITADEL_INGEST_MODEL=sonnet # (PDF first, then modern Office, then legacy); 0 ingests every format separately. # CITADEL_DEDUP_BY_BASENAME=1 +# --- Throughput --- +# How many sources ingest folds in CONCURRENTLY (`citadel ingest --jobs N` overrides per run). +# 1 = the serial default. Higher runs that many agent sessions at once, each on its own staging +# copy; promotes stay serialized and a source that raced another over the same page is re-run +# serially. Trade-off: concurrent sessions cannot see each other's new pages, so cross-linking +# suffers — see docs/configuration.md "Parallel ingest". +# CITADEL_JOBS=1 + # --- Large sources & repos --- # A source longer than this many chars is folded in over several merging passes; 0 disables chunking. # CITADEL_MAX_SOURCE_CHARS=300000 diff --git a/citadel/transcribe.py b/citadel/transcribe.py index bb9eaa3..ac16aa4 100644 --- a/citadel/transcribe.py +++ b/citadel/transcribe.py @@ -113,7 +113,7 @@ def is_audio_source(path: Path) -> bool: def cache_dir() -> Path: """The transcript cache directory: a dotdir sibling of the wiki dir (read at call time so tests can monkeypatch the config layout — exactly like ``runlock.lock_path``).""" - return Path(config.WIKI_DIR).parent / CACHE_DIR_NAME + return Path(config.wiki_dir()).parent / CACHE_DIR_NAME # A sha256 hexdigest and nothing else — the ONLY string shape allowed to become a cache filename. diff --git a/citadel/viewer/__init__.py b/citadel/viewer/__init__.py index 0b8c42b..9ba6be1 100644 --- a/citadel/viewer/__init__.py +++ b/citadel/viewer/__init__.py @@ -148,7 +148,7 @@ def build_bundle(pages=None) -> dict: tags = {tag: [p.rel_path for p in tagged] for tag, tagged in store.tag_catalog(pages).items()} return { - "wiki_name": config.WIKI_DIR.name, + "wiki_name": config.wiki_dir().name, "pages": pages_json, "tags": tags, "types": {k: sorted(v) for k, v in types.items()}, @@ -188,7 +188,7 @@ def _source_view_id(abs_path: str | os.PathLike) -> str: citation maps to it: the source's posix path relative to ``WIKI_DIR``'s parent (the repo root, or the shared root on a mounted drive). For an in-repo source this is just ``raw/x.md`` — which is exactly what the JS ``resolveLink`` returns for a citation that climbs out of ``wiki/``.""" - parent = config.WIKI_DIR.parent + parent = config.wiki_dir().parent try: return os.path.relpath(str(abs_path), str(parent)).replace(os.sep, "/") except ValueError: # different drive — no relative path exists @@ -239,7 +239,7 @@ def _source_href(abs_path: str | os.PathLike) -> str | None: binary natively. None when no relative path exists (a different drive). The link assumes the viewer was written to its default ``wiki/`` location; the embedded text always works regardless.""" try: - return os.path.relpath(str(abs_path), str(config.WIKI_DIR)).replace(os.sep, "/") + return os.path.relpath(str(abs_path), str(config.wiki_dir())).replace(os.sep, "/") except ValueError: # different drive — no relative path exists return None @@ -514,10 +514,10 @@ def build_html(pages=None) -> str: def write_viewer(out_path=None, pages=None) -> Path: - """Write the viewer document; default ``config.WIKI_DIR/.citadel_viewer.html``. Returns the + """Write the viewer document; default ``config.wiki_dir()/.citadel_viewer.html``. Returns the absolute path written.""" if out_path is None: - out_path = config.WIKI_DIR / VIEWER_FILENAME + out_path = config.wiki_dir() / VIEWER_FILENAME out_path = Path(out_path) out_path.parent.mkdir(parents=True, exist_ok=True) out_path.write_text(build_html(pages), encoding="utf-8") @@ -596,7 +596,7 @@ def _open_obsidian() -> int: """Best-effort: deep-link the wiki folder into Obsidian and always print the path.""" import urllib.parse - folder = config.WIKI_DIR.resolve() + folder = config.wiki_dir().resolve() deep = "obsidian://open?path=" + urllib.parse.quote(str(folder)) print(f"Open this folder as an Obsidian vault: {folder}") print(" tip: open the repository root instead to resolve raw/ citation links.") diff --git a/citadel/wikigit.py b/citadel/wikigit.py index 046b0d0..2de9aca 100644 --- a/citadel/wikigit.py +++ b/citadel/wikigit.py @@ -2,7 +2,7 @@ The wiki is plain markdown files, so git is the natural long-term changelog: after each ingest or curate run that changed pages, :func:`autocommit` stages and commits EVERYTHING under -``config.WIKI_DIR`` (pages, the regenerated indexes, ``log.md``, the manifest) so every run is one +``config.wiki_dir()`` (pages, the regenerated indexes, ``log.md``, the manifest) so every run is one reviewable diff — what changed, in which page, attributable to the run that did it. An optional remote (``CITADEL_WIKI_GIT_REMOTE`` — a name or URL, GitHub/GitLab/anything) is pushed to after each commit. @@ -21,7 +21,7 @@ would otherwise stall every run on an interactive pinentry. These are automated audit commits — sign by hand if a signed history matters. -Config is read at call time (``config.WIKI_GIT`` / ``config.WIKI_GIT_REMOTE`` / ``config.WIKI_DIR``) +Config is read at call time (``config.WIKI_GIT`` / ``config.WIKI_GIT_REMOTE`` / ``config.wiki_dir()``) so tests monkeypatch it like everything else. """ @@ -107,7 +107,7 @@ def _identity_args(wiki_dir: Path) -> list[str]: def autocommit(message: str) -> str | None: - """Commit every change under ``config.WIKI_DIR`` as one commit with ``message``; push to + """Commit every change under ``config.wiki_dir()`` as one commit with ``message``; push to ``config.WIKI_GIT_REMOTE`` when set. Returns a one-line human note for the run report ("wiki git: committed ", or a warning describing what was skipped and why) — or None when there is nothing to say (mode off, no repo in auto mode, or a clean tree). NEVER raises: the @@ -115,7 +115,7 @@ def autocommit(message: str) -> str | None: mode = config.WIKI_GIT if mode == "off": return None - wiki_dir = Path(config.WIKI_DIR) + wiki_dir = Path(config.wiki_dir()) if not wiki_dir.is_dir(): return None if not (wiki_dir / ".git").exists() and mode != "init": diff --git a/docs/audit-2026-07.md b/docs/audit-2026-07.md index ede9569..a09b4c0 100644 --- a/docs/audit-2026-07.md +++ b/docs/audit-2026-07.md @@ -76,6 +76,12 @@ comparison — and each has a design that closes it without compromising the pro `.env` edits; one process = one workspace; and ingest's staging redirect mutates `config.WIKI_DIR` + `os.environ` in place — safe under the strictly-serial design, a hard blocker for any future parallel ingest or multi-workspace server. + *✅ The parallel-ingest blocker resolved 2026-07-25 — backlog #11 shipped (see § 4): the wiki + redirect is a `ContextVar` behind `config.wiki_dir()`/`index_path()`/… and child processes get + their wiki through an explicit per-spawn env, so N sources stage concurrently in one process. + The other two halves stay open by design-choice, not by blocker: settings are still read once at + import (a `serve` process does not see `.env` edits), and one process still serves one + workspace.* 6. **MCP read tools reload the entire wiki from disk on every call** (`store.load()` per call, no cache) — see § 1.3 for the scale impact. *✅ Resolved 2026-07-24 — backlog #15 shipped (see § 4): `citadel/pagecache.py` keeps a @@ -132,6 +138,9 @@ a headless-Chromium smoke test drives the built viewer's JS in CI.* copies the wiki ~200×. Nothing (hardlinks, dirty-set copies) mitigates it yet. - **Strictly serial** — no intra-run parallelism knob (blocked by finding 5), while the per-source staging copy is already exactly the isolation primitive parallel ingest would need. + *✅ Resolved 2026-07-25 — backlog #11 shipped (see § 4): `citadel ingest --jobs N` / + `CITADEL_JOBS` runs N sources at once on exactly that primitive, with promotion serialized and + base-aware. The default stays 1 — the trade-off it buys back is cross-linking, not safety.* - **No watch mode / scheduler** — `refresh --min-age-days` is designed for external cron, but scheduling is entirely the user's problem; no file-watcher exists. *✅ Recipe half closed 2026-07-24 — backlog #14 shipped (see § 4): `docs/recipes.md` documents @@ -309,6 +318,11 @@ still beats substring matching on first-shot hit rate. The pragmatic ladder: - **Parallelism**: per-source staging is already the isolation primitive; a bounded `--jobs N` (promotes serialized under the run lock) is the credible throughput win — blocked today by the process-global config mutation (finding 1.2.5). + *✅ Done 2026-07-25 — backlog #11, with one correction to the scoped design: serializing promotes + is necessary but NOT sufficient. A promote also has to know WHICH wiki it started from — the + original prune ("delete whatever live has and staging does not") would have deleted every page a + concurrent source had just added. The shipped promote is base-aware, and a page that moved under + a session sends that source to a serial re-run instead of letting a stale session win.* - **Audio/video is the largest un-ingestable content class left**, and fits the no-SDK philosophy perfectly: a `CITADEL_AUDIO_SUPPORT` knob shelling out to a whisper-CLI binary on PATH (exactly like the agent-CLI seam), caching the transcript as the text the session reads, plus a @@ -344,12 +358,23 @@ still beats substring matching on first-shot hit rate. The pragmatic ladder: | 8 | **Test the two risky hand-rolled surfaces** — CFBF fixtures for `citadel/extract_ole.py`, a headless-browser smoke test for the viewer JS | Thinnest coverage vs. risk in the repo | `tests/` | M | ✅ 2026-07-24 — shipped both halves. CFBF: `tests/test_extract_ole.py` grew a spec-shaped MS-CFB **writer** fixture (`_build_cfbf` — FAT, DIFAT sector chain past the header's 109 slots, multi-sector directory, mini-FAT/mini-stream), pinning offline every container path the kontor corpus previously reached only through a live LLM run: byte-exact round-trips (mini + multi-sector big streams), the BIFF5 `Book` fallback + `Workbook` preference, and the corruption guards — FAT/mini-FAT cycles terminate, an out-of-file chain, truncation, or a bad sector shift degrade to whole-file salvage, a rootless container yields empty text, never a crash (writing the tests confirmed one nuance: a chain pointing past the file aborts the parse into salvage rather than ending the chain — now pinned as intended behavior). Viewer: `tests/test_viewer_browser.py` loads the real built document in headless Chromium and drives boot/sidebar, search filter-and-recover, hash-routed page open, the citation hover popover, and `/`-focus, asserting **zero JS page errors** throughout — opt-in via a new `browser` dependency group (self-skips without playwright; the default suite stays offline), run in CI as the dedicated ubuntu `viewer-smoke` job, with `CITADEL_TEST_BROWSER` for pre-provisioned chromium builds. | | 9 | **Resumable chunked ingest** (capture session IDs, retry from segment N) | Converts the documented discard-N−1-segments trade-off into saved spend | `citadel/ingest.py`, `citadel/llm.py` | M | ✅ 2026-07-24 — shipped, but NOT via session IDs: the scoped `claude --resume` route cannot work here (the staging tree the conversation edited is rm-tree'd on every failure path, so a resumed conversation would believe it had written pages that no longer exist — and copilot/gemini expose no equivalent flag at all). Instead a new `citadel/resume.py` banks, after each completed segment, the **delta that segment set would have promoted** — computed with the promote's own file-level view (`_content_files` + byte equality), so `_repair_renames`' link rewrites in untouched pages and any non-`.md` file are in it, which the per-segment page diffs are not — into `.citadel_resume/` beside the wiki (`CITADEL_RESUME`, default on; only a CHUNKED source ever writes one). The next run replays that delta into its FRESH staging copy and opens at segment N; a promote that failed *after* the last segment replays with zero sessions. Promote-once is untouched: still exactly once, still only after the last segment, still nothing partial on a failure — the guarantee did not move, only the bill. Guards, all offline and all BEFORE the first resumed session: identity (source sha — a null sha opts the source out, since `None == None` must never read as "unchanged" — model, rules version, the segment plan's CONTENT rather than its temp paths, and the prompt-shaping knobs `rules_version` misses: `CITADEL_WIKI_LANG`/`STYLE_PROFILES`/`PDF_MODE`/`IMAGE_SUPPORT`, so a language flip can't merge German segments into English pages), blob integrity, per-page live base state (a page another source changed in between is never clobbered; a banked DELETION carries a mandatory base hash, so it can never destroy work a different source has since done under that path), re-validation of every replayed page plus a no-NEW-broken-links check (a raw file deleted under a checkpointed page drops the checkpoint instead of promoting a `bad_source` page), an attempt cap (a deterministically poisonous segment N would otherwise wedge the source into failing cheaply forever), and `runlock.owned()` on every write (new predicate, extracted from `heartbeat`) so a stalled run never overwrites the run that reclaimed its lock. **Every guard failure falls back to a full restart at segment 1 in the SAME run** — the pre-resume behavior is the floor, never the failure mode. Cost stays honest on both sides: the run report counts only what that run spent, the manifest stamps the source's whole import cost across the runs that paid it. `doctor` lists what is waiting; `.gitignore`/SECURITY.md/recipes register the new plaintext sidecar. No rules edit was needed (a resumed segment sees exactly the wiki an unbroken run's segment would have), so no `rules_version` bump and no corpus re-grade. | | 10 | **pypdf text-layer pre-pass** for PDFs (real line locators + correct chunking; keeps images mode for figures) | Fixes the locator-verifiability hole for the PDF class (finding 1.2.2), pure-Python dep | `citadel/ingest.py`, `citadel/lint.py` | M | ✅ 2026-07-24 — shipped as the audio pattern applied to PDFs: a new `citadel/pdftext.py` seam (pypdf as a BUNDLED runtime dep — PDFs are a common `raw/` class, so the offline path is on by default, not an opt-in extra; `CITADEL_PDF_TEXT`, default auto = on-when-importable) extracts a genuine PDF's text layer once per content into a `[p. N]`-page-marked, line-stable text, cached content-addressed in `.citadel_pdftext/` beside the wiki and pruned with the source. New `pdf`/`pdf-reconcile` kinds have the agent read the extraction (formats/pdf.md rewritten for both setups) while citing the original `.pdf` with `lines A-B` locators that `lint`/curate, `wiki_raw`, and the viewer now verify offline against the same cache; large PDFs chunk as line windows over ONE full extraction (numbering never rebases — the audio contract). Strictly best-effort: scanned / encrypted / corrupt / CITADEL_PDF_TEXT=0 / pypdf force-removed → per-source fallback to the pre-existing agent-native read (`p. N`, agent-verified — still the rule for figure-only facts in images mode, which keeps having the agent open the original PDF's figures); no failure of the pre-pass can cost a session. `doctor` gained the "PDF text" advisory. | -| 11 | **Bounded parallel ingest (`--jobs N`)** — requires first un-globalizing the config mutation (finding 1.2.5) | Throughput at real-corpus scale; staging already isolates | `citadel/config.py`, `citadel/ingest.py` | L | open | +| 11 | **Bounded parallel ingest (`--jobs N`)** — requires first un-globalizing the config mutation (finding 1.2.5) | Throughput at real-corpus scale; staging already isolates | `citadel/config.py`, `citadel/ingest.py` | L | ✅ 2026-07-25 — shipped in two steps, the unblock and the feature. **Unblock (finding 1.2.5):** the per-source staging redirect no longer ASSIGNS `config.WIKI_DIR`/`INDEX_PATH`/`LOG_PATH`/`MANIFEST_PATH` and `os.environ["CITADEL_WIKI_DIR"]` — the wiki path is read through accessors (`config.wiki_dir()` and friends) backed by a `ContextVar`, which is per-thread by construction, so N workers each hold their own redirect while the main thread still sees the live wiki; unset, the accessors return the module attributes verbatim, so every read path, every CLI command, the MCP server — and every test that monkeypatches the layout — behave exactly as before. The agent session's child processes get their staging wiki through an explicit per-spawn `env` instead of a global mutation. **Feature:** `citadel ingest --jobs N` (`CITADEL_JOBS`, default 1 = the serial behavior line for line) folds N sources in at once, each on its own staging copy. Every guarantee is unmoved — one promote per source, all-or-nothing, nothing partial on the live wiki, the manifest still saved per completed source — because what is SHARED is serialized rather than raced: ONE lock guards the two moments that touch the live wiki (the clone, taken together with a hash snapshot of what was cloned, and the promote), while the minutes-long sessions run entirely outside it. Two design points moved during implementation, and they are the substance of the work: (a) serializing promotes is not enough — the promote has to be **base-aware**, pruning only what its OWN clone had and its staging lacks, since the original "prune whatever live has and staging does not" would have deleted every page a concurrent source had just promoted (the pinned regression test); (b) two sessions that wrote the same page must not be resolved by last-writer-wins — the second promote is REFUSED before it writes a byte (`_ConcurrentChange`), and that source is **re-run serially** at the end of its group, where it sees the winner's page and merges into it, i.e. the result a serial run would have produced. Those re-runs are surfaced on the report (*Re-run serially*) because they are the one place parallelism costs a session serial ingest would not have spent — a corpus that races a lot wants a lower `--jobs`. Report/manifest/failures bookkeeping stays on the main thread (worker threads only plan, stage, run and promote), so those writes are as single-threaded as before; an interrupt cancels queued sources, lets in-flight ones roll themselves back, and still records work that was already promoted. The default stays 1 on purpose: the real cost is **cross-linking**, not safety — concurrent sessions cannot see each other's new pages, so they link less richly and can duplicate a topic (`curate` is the designed cleanup). One further rule fell out of an ADVERSARIAL CROSS-REVIEW (a second model re-reviewing the concurrency): a CHUNKED source's resume checkpoint measured its delta against the CURRENT live wiki, so a page a concurrent source created read as a deletion this source had made, and one it rewrote read as this source's change with stale bytes — and since a checkpoint is durable, that delta outlived the parallel run and could prune a fully-ingested source's page in a later, even strictly serial, run (both manifestations reproduced offline before the fix, and pinned since). The delta, and the base state a replay is guarded against, now both come from the source's own clone snapshot — exactly the promote's rule — so a page another source touched fails the guard instead of passing it, and a raced chunked source really re-runs instead of replaying a stale checkpoint with zero sessions. Three defects the parallel path exposed were fixed at the root rather than papered over: the promote's leftover-temp sweep deleted HIDDEN `*.citadeltmp` files — another writer's in-flight atomic manifest/failures save — `config.atomic_write_text`'s temp name was unique per PID but not per thread, and the transcript log's sequence counter was bumped without a lock while its own comment promised concurrent files never collide. 17 offline tests drive the concurrency for real (fake sessions synchronize on a `threading.Barrier`, so a test that requires two sources in flight at once cannot pass on a serial run) and pin each safety property to the failure it prevents. | | 12 | **Streamable HTTP transport (opt-in, token-guarded)** | Reach from claude.ai/phones; FastMCP supports it | `citadel/server.py`, docs | M | ✅ 2026-07-25 — shipped as a new `citadel/httpserve.py` (the transport only — the tool/prompt/resource surface stays `server.py`'s, shared byte-for-byte with stdio) behind `citadel serve --http`. Stdio remains the default and is untouched. Because this is the ONLY network surface citadel has, every default is the strict one: the bearer token is **mandatory** (`CITADEL_HTTP_TOKEN`, ≥16 chars — `serve --http` refuses to start without one, so there is no "unauthenticated for a minute" mode), checked in a ~30-line ASGI wrapper BEFORE the MCP session layer (constant-time `hmac.compare_digest`, 401 + `WWW-Authenticate`, and the same answer for a missing and a wrong token — an unauthenticated caller cannot even open a session); the bind is loopback by default and a public bind WARNs, because the transport is plain HTTP and the intended remote path is a TLS-terminating tunnel (cloudflared / tailscale / `ssh -R`), not a raw port; the SDK's DNS-rebinding protection is enabled against the bound host (+ its loopback aliases), so a page in the user's browser cannot drive the server. One addition beyond the scoped design: `--read-only` (`CITADEL_HTTP_READ_ONLY`) has the two mutating tools REFUSE while the eleven readers keep working — the remote-exposure asymmetry is real (`wiki_ingest` spawns the coding-agent CLI on the serving host, which is a bigger promise than "read my wiki") — and it declines to ACT without changing the ADVERTISED tool list, so a client's surface never silently changes shape. The scoped OAuth-shaped route was declined on purpose: citadel has no user model, so the shared token IS the user, and the SDK's `TokenVerifier`/`AuthSettings` machinery would add an issuer/resource-server story with nothing behind it. Zero new dependencies (starlette + uvicorn already ship with `mcp`), the HTTP server opts into the page cache exactly like stdio, an HTTP flag passed to a stdio run is a usage error rather than a silently ignored setting, and `doctor` gained an "HTTP serve" line (where it would listen, whether the writers are exposed, WARN on a short token or a public bind — the refusal you would otherwise meet only at start-up). 70+ offline tests drive the whole stack in-process through starlette's TestClient (no port is ever bound in the suite): the 401/421/403 paths, a real `initialize` + `tools/list` over the transport, the read-only refusals, and the CLI wiring. One design point moved during review: the Host/Origin admission policy is citadel's own (in the same ASGI wrapper), not the SDK's transport-security middleware, because that middleware derives its allowlist from the BIND address — which is exactly wrong for the deployments this feature exists for. A tunnel forwards its own hostname and a wildcard bind is a name no client ever sends, so the shipped design would have answered `421` to every request it was built to serve. Now: `CITADEL_HTTP_ALLOWED_HOSTS` names the hosts (a tunnel's, a proxy's; `*` when something in front already filters), a wildcard bind with no allowlist REFUSES to start (naming the knob) instead of binding happily and rejecting everything, and `CITADEL_HTTP_ALLOWED_ORIGINS` — empty by default, so any browser-`Origin` request is refused — is what admits a browser-based client. | | 13 | **Sync recipe docs** (markdown+git multi-device patterns) | Landscape-norm parity for the cost of a docs page | `docs/` | S | ✅ 2026-07-24 — shipped with #14 as one page, `docs/recipes.md` (linked from the docs hub): the **one-writer-many-readers** rule up front (the run lock protects one filesystem, not a sync service — so mutating lifecycles stay on one machine and every synced copy is read-only), then the recipes — wiki-history git + a private remote as the recommended lane (`git init` in `wiki/` + `CITADEL_WIKI_GIT_REMOTE`; the in-wiki manifest means a pulled mirror answers `status`/`search`/`serve` correctly), whole-workspace git for provenance portability (with the exact `.gitignore` — `.env`, lock, regenerable caches, viewer artifact — and the note that this layout commits via the scheduled run, since wikigit only auto-commits a wiki that is its own repo), file-sync services scoped to what they fit (capture inbox + read-only mirrors, never multi-writer), and the three phone-reading lanes (git host UI, Obsidian mobile, the one-file `citadel view` artifact). | | 14 | **Scheduled-ingest recipe** (cron/systemd/Claude-Routines examples driving `ingest` + `refresh --min-age-days`) | llmwiki's nightly autonomous fold-in is a headline feature; citadel already has the self-limiting knobs, just no documented recipe | `docs/` | S | ✅ 2026-07-24 — shipped with #13 in `docs/recipes.md`: leads with *why unattended runs are safe by design* (sha-idempotent no-op nights, `--min-age-days` self-limiting refresh, run-lock overlap fails loud, all-or-nothing promotion, exit-code + cost-stamp auditability), names the canonical pair (nightly `ingest --quiet`, weekly `refresh --limit 5 --min-age-days 30`), then gives four concrete schedulers: cron (with the real gotcha — minimal environment — solved via the auto-loaded `.env`'s `*_CLI_PATH` overrides instead of fighting cron's PATH), a systemd user timer pair (`Persistent=true`, linger), Windows Task Scheduler on the portable invocation, and the llmwiki-parity **"smart cron"** lane: a scheduled agent session (e.g. a Claude Code Routine) running the same commands and then triaging the report — clean nesting guaranteed by the hermetic-session default (#6). Closes with the deliberate no-watch-mode stance (schedule + sha idempotency beats a watcher daemon). | | 15 | **Serve-side page cache** — stop re-walking and re-parsing the whole wiki on every MCP call (finding 1.2.6; the latency half of § 1.3) | The one open piece of the retrieval story after #1 shipped ranking: at 1000 pages every read tool pays ~0.7 s of disk+parse before scoring, and `citadel serve` pays it per call, forever | `citadel/pagecache.py`, `citadel/store_core.py`, `citadel/server.py` | S–M | ✅ 2026-07-24 — shipped as a new `citadel/pagecache.py` behind the unchanged `store_core.load()`: the last load is held in memory and re-validated on **every** consult by a stat-only `os.scandir` fingerprint — `(rel_path, size, mtime_ns, ctime_ns)` over exactly the files `load()` parses, mirroring its traversal (hidden dirs skipped, symlinked dirs not entered, file symlinks stat'd through to their target) — plus a per-snapshot memo for search's per-page term-frequency tables, the other half of a query's cost. Measured at 1000 pages: `load()` ~700 ms → ~9 ms, `wiki_search` ~1.4 s → ~50 ms, `tag:`/`type:` queries ~630 ms → ~11 ms. **Nothing is persisted** — the audit's own "the wiki IS the database" line holds literally: no index file, nothing to go stale, and the filesystem re-answers "did anything change?" on every single call. The FTS5-shaped alternative (a persisted or rebuilt-at-load index) was not revisited: #1 already settled that ranking question, and this is a pure I/O problem. Staleness is designed out rather than hoped away, and every guard is cheap and offline: the fingerprint is taken BEFORE and AFTER the load and must match (a page rewritten mid-load would otherwise pair old content with a new stamp — the one race that could stale forever), a snapshot whose newest stamp is younger than a 2 s settle window is never stored (FAT-class timestamp granularity can hide a same-length same-tick rewrite; a network share whose clock runs ahead simply never caches, which costs speed and never correctness), the single slot is keyed by wiki directory (ingest's `config.WIKI_DIR` staging redirect can never be served the live wiki, and the unbounded stream of staging dirs cannot accumulate entries), `write_page`/`delete_page` invalidate directly, any walk error means "cannot vouch for this" → uncached, and `get()` hands out a fresh list per call so a caller sorting or clearing it cannot corrupt the snapshot. The memo is keyed by `id()` but only for pages the snapshot itself keeps alive, so a foreign page can never collide with a recycled address. **The mutating lifecycles do not participate**: `ingest()` and `curate()` wear `@pagecache.bypass` (re-entrant), so the staged diff-by-hash that decides what to promote always reads truth from disk — no speed-up on a path whose unit of work is a multi-minute agent session is worth that risk. Off by default (`CITADEL_PAGE_CACHE=auto`): `citadel serve` opts in — it is the long-lived reader the finding is about — `1` enables it wherever citadel reads the wiki, `0` restores the pre-cache behavior everywhere. 25 offline tests pin the hit, every way a wiki can change (edit, same-length rewrite, add, delete, rename, symlink target), and every guard. | +**State of the backlog (2026-07-25): all 15 ranked items are shipped.** What is still open lives +outside the table and is tracked where it was found: § 1.2's remaining findings — #1 (contradiction +"detection" is marker-counting), #2's Office/image slice (their locators stay agent-verified), #5's +other two halves (settings are read once at import; one process = one workspace), #7 (`extract.py` +swallows every error, so an encrypted file is indistinguishable from a text-free one), #9 (`log.md` +is unbounded and re-read per append) — plus § 1.5's operational gaps (per-source staging is O(wiki +size) × sources, a blocking `wiki_ingest` over MCP, plaintext-everything, no restore path), § 2.2's +landscape gaps 1 and 3-10, and § 3.1's MCP remainder (subscribe/`listChanged`, elicitation, +long-running tasks — the last being the natural fit for a non-blocking `wiki_ingest`). Ranking those +into a second backlog is a fresh audit's job, not this one's. + **Legitimate declines** (the evidence supports saying no): required embeddings/vector DB, hosted OAuth serving, heavyweight PDF parsers (AGPL/torch), memory-decay policies, multi-user permissions. These are the SaaS/framework lane, not citadel's local-first, provenance-first lane. diff --git a/docs/configuration.md b/docs/configuration.md index 249d9b6..ba41800 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -118,6 +118,7 @@ $env:CITADEL_LLM_CLI = "copilot" | `CITADEL_PDF_MODE` | `text` | `text` ingests body text only; `images` also has the agent look at figures/diagrams/charts (needs a backend whose reader renders PDF pages). | | `CITADEL_PDF_TEXT` | `auto` | The pypdf text-layer pre-pass (pypdf ships as a bundled runtime dep — no extra install): PDFs ingest via an extracted, content-addressed cached text layer (`.citadel_pdftext/` next to the wiki), giving real `lines A-B` locators that `lint`/`wiki_raw`/the viewer verify offline — and letting large PDFs chunk. `auto` uses it when [pypdf](https://pypi.org/project/pypdf/) imports (it does by default); `1` forces it (per-source fallback + `doctor` WARN if pypdf was force-removed); `0` forces agent-native PDF reading (`p. N` locators, agent-verified). Scanned/encrypted/unparsable PDFs always fall back to agent-native reading. | | `CITADEL_STYLE_PROFILES` | `0` | When `1`, first-person sources also yield attributed, dated, cited opinions + a per-person writing-style profile. Leave off for many-person corpora. | +| `CITADEL_JOBS` | `1` | How many sources ingest folds in **concurrently** (`citadel ingest --jobs N` overrides it per run). `1` is the serial behavior citadel has always had. Above 1, each source still gets its own staging copy and its own all-or-nothing promote — promotes are serialized and checked against the wiki state the source was cloned from, and a source that raced another over the same page is re-run serially before the run ends (reported as *Re-run serially*). The cost is **cross-linking**, not safety: concurrent sessions cannot see each other's new pages, so they link less richly and can create two pages for one topic (a later `citadel curate` pass is the designed cleanup). Best on a large backlog of unrelated sources; keep `1` when the corpus is one connected topic. Both `citadel ingest --jobs N` and `citadel refresh --jobs N` override it per run; `curate` is always serial — its clusters share pages by construction, so nearly every pair would race. | | `CITADEL_MAX_SOURCE_CHARS` | `300000` | A source longer than this is ingested over several sequential passes that merge into earlier pages. `0` disables chunking. Images — and PDFs without an extracted text layer — are never chunked. | | `CITADEL_RESUME` | `1` | Resume checkpoints for those chunked sources: each completed segment banks the delta it produced (`.citadel_resume/` next to the wiki), so a run that dies at segment N continues there next time instead of re-buying segments 1…N-1. Promotion stays all-or-nothing — nothing partial ever reaches the wiki — and every guard (changed source/model/rules/knobs, a page changed underneath, a replay that no longer validates) falls back to a full restart. `0` turns it off; only chunked sources ever write one. | | `CITADEL_DEDUP_BY_BASENAME` | `1` | When several same-folder files share a basename and are all export formats (e.g. `report.pptx` + `report.pdf`), ingest one (PDF → modern Office → legacy) and record the rest as skipped duplicates. | diff --git a/docs/recipes.md b/docs/recipes.md index 970c11f..e4480a6 100644 --- a/docs/recipes.md +++ b/docs/recipes.md @@ -111,6 +111,28 @@ Together they keep the wiki current in both directions: new knowledge lands with imports are re-verified round-robin under the current model + rules (~20 sources/month at these numbers — size the budget against `citadel status`'s recorded cost). +### Working off a big backlog: `--jobs N` + +A nightly run has hours, so it can afford to be serial — and serial is what gives the richest +cross-linking, because every session sees the pages the previous one wrote. When you are folding in +a *backlog* of largely unrelated sources (a first import, a newly mounted archive), the run is +dominated by per-session latency instead, and `--jobs` trades some of that linking for wall-clock: + +```bash +citadel ingest --jobs 4 # or set CITADEL_JOBS=4 for every run in this workspace +``` + +`citadel refresh --jobs N` takes the same flag, and is arguably where it fits best: a refresh +slice is ordered by *last-checked time*, not by topic, so its sources are usually unrelated. + +Each source keeps its own staging copy and its own all-or-nothing promote — nothing about the +safety model changes — but concurrent sessions cannot see each other's new pages, so they link less +richly and can create two pages for one topic. Two things absorb that: a source whose promote raced +another one over the same page is automatically re-run serially (the report lists it under *Re-run +serially*), and a later `citadel curate` pass merges and re-grounds what parallel sessions left +apart. Lots of races in the report means the corpus is more connected than the run assumed — lower +`--jobs`. Rate limits on your agent CLI are the practical ceiling; citadel imposes none. + ### cron (Linux, macOS) ```cron diff --git a/tests/conftest.py b/tests/conftest.py index 0e4a471..4343514 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -63,17 +63,23 @@ class CitadelTmp: never by re-deriving paths from ``tmp_path`` or by assuming which ``config.*`` attributes were patched: a later PR swaps the root-resolution internals (workspace discovery), and only this interface is guaranteed to survive that swap. + + The fixture WIRES the process-wide ``config`` attributes (``config.WIKI_DIR`` and friends); + the wiki-derived fields below are annotated with their ACCESSOR (``config.wiki_dir()``) + because that is how code READS them — and the distinction matters inside a fake session: + ingest redirects the wiki per source through a ContextVar, so only the accessor sees the + staging copy the session is supposed to edit, while the attribute keeps naming the live wiki. """ root: Path # the (fake) workspace root -> config.WORKSPACE_ROOT - wiki: Path # config.WIKI_DIR + wiki: Path # config.wiki_dir() raw: Path # config.RAW_DIR docs: Path # config.DOCS_DIR - index_path: Path # config.INDEX_PATH - sources_index_path: Path # config.SOURCES_INDEX_PATH - log_path: Path # config.LOG_PATH - manifest_path: Path # config.MANIFEST_PATH - failures_path: Path # config.FAILURES_PATH + index_path: Path # config.index_path() + sources_index_path: Path # config.sources_index_path() + log_path: Path # config.log_path() + manifest_path: Path # config.manifest_path() + failures_path: Path # config.failures_path() packaged_rules: Path # config.PACKAGED_RULES_DIR (a stub tree at /citadel/rules) def read_manifest(self) -> dict: @@ -217,12 +223,12 @@ def tmp_citadel_external(make_citadel, tmp_path: Path) -> CitadelTmp: def seed_page() -> Callable[..., Path]: """Write a canonical OKF page directly under the CONFIGURED wiki (bypassing ingest). - Reads ``config.WIKI_DIR`` at call time, so it composes with any layout fixture + Reads ``config.wiki_dir()`` at call time, so it composes with any layout fixture (``tmp_citadel``, ``tmp_citadel_external``, or a custom ``make_citadel`` layout). """ def _seed(rel_path: str, frontmatter: dict, body: str = "Body.\n") -> Path: - target = Path(config.WIKI_DIR) / rel_path + target = Path(config.wiki_dir()) / rel_path target.parent.mkdir(parents=True, exist_ok=True) target.write_text(okf.dump(frontmatter, body), encoding="utf-8") return target @@ -242,7 +248,7 @@ def _cite_page(rel_path: str, rel_key: str, body_fact: str) -> None: which would emit a broken ``../../C:/...`` link and fail every fake session for an out-of-workspace source on Windows.""" link = rel_key if Path(rel_key).is_absolute() else f"../../{rel_key}" - target = config.WIKI_DIR / rel_path + target = config.wiki_dir() / rel_path target.parent.mkdir(parents=True, exist_ok=True) target.write_text( okf.dump( @@ -256,7 +262,7 @@ def _cite_page(rel_path: str, rel_key: str, body_fact: str) -> None: @pytest.fixture def cite_page() -> Callable[[str, str, str], None]: """``seed_page``'s little sibling for fake-session bodies: ``cite_page(rel_path, rel_key, - fact)`` writes one valid page whose single fact cites ``rel_key``. Reads ``config.WIKI_DIR`` + fact)`` writes one valid page whose single fact cites ``rel_key``. Reads ``config.wiki_dir()`` at call time, so inside a session it lands in ingest's per-source staging copy.""" return _cite_page @@ -286,7 +292,7 @@ def delete_citing_pages(rel_key: str) -> None: ``rel_key`` from the CONFIGURED wiki — ingest's per-source staging copy at call time, exactly like the real agent's file edits — enough to satisfy the delete post-condition.""" for rel in store.find_raw_references(rel_key): - (Path(config.WIKI_DIR) / rel).unlink(missing_ok=True) + (Path(config.wiki_dir()) / rel).unlink(missing_ok=True) def _make_pptx(path: Path, slides: list[list[str]]) -> None: @@ -344,7 +350,7 @@ class FakeAgent: 1. records ``(rel_key, kind)`` in ``self.calls`` (assert on ``calls``/``count``); 2. raises ``error`` if given (a failed/timed-out session); 3. writes ``pages`` into the configured wiki — ingest's per-source STAGING copy, since - ``config.WIKI_DIR`` is read at call time (``{rel_path: (frontmatter, body)}`` dumped + ``config.wiki_dir()`` is read at call time (``{rel_path: (frontmatter, body)}`` dumped via ``okf.dump``, or ``{rel_path: str}`` written verbatim); 4. passes the original args through to ``side_effect`` for bespoke per-call behavior (e.g. deleting the pages that cite a removed source); @@ -380,7 +386,7 @@ def __call__(self, *args, **kwargs) -> llm.SessionUsage | None: if self.error is not None: raise self.error for rel_path, content in self.pages.items(): - target = Path(config.WIKI_DIR) / rel_path + target = Path(config.wiki_dir()) / rel_path target.parent.mkdir(parents=True, exist_ok=True) text = content if isinstance(content, str) else okf.dump(*content) target.write_text(text, encoding="utf-8") diff --git a/tests/test_curate.py b/tests/test_curate.py index 64c949d..ea623b7 100644 --- a/tests/test_curate.py +++ b/tests/test_curate.py @@ -375,7 +375,7 @@ def test_applied_cluster_appends_edit_summary_to_log(tmp_citadel, seed_page, fak ) curate.curate() - log_text = config.LOG_PATH.read_text(encoding="utf-8") + log_text = config.log_path().read_text(encoding="utf-8") assert "curate" in log_text.lower() assert "concepts/alice.md" in log_text diff --git a/tests/test_ingest_chunking.py b/tests/test_ingest_chunking.py index 793b92a..dd139e9 100644 --- a/tests/test_ingest_chunking.py +++ b/tests/test_ingest_chunking.py @@ -134,7 +134,7 @@ def fake(rel_key, kind="ingest", read_path=None, segment=None): def fake_retry(rel_key, kind="ingest", read_path=None, segment=None): segments.append(segment) # Segment 1's page is already in the staging copy when the resumed segment opens. - assert (Path(config.WIKI_DIR) / "misc" / "big.md").exists() + assert (Path(config.wiki_dir()) / "misc" / "big.md").exists() fake_agent(side_effect=fake_retry) second = ingest.ingest() @@ -175,7 +175,7 @@ def fake(rel_key, kind="ingest", read_path=None, segment=None): # Mid-source, the LIVE wiki must never hold this source's page yet. "live_clean": not (wiki / "misc" / "big.md").exists(), # Segments > 1 MERGE into what the earlier segments wrote in the shared staging. - "staging_has_earlier": (config.WIKI_DIR / "misc" / "big.md").exists(), + "staging_has_earlier": (config.wiki_dir() / "misc" / "big.md").exists(), } ) if segment[0] == 1: @@ -208,7 +208,7 @@ def fake(rel_key, kind="ingest", read_path=None, segment=None): if segment[0] == 1: cite_page("misc/big.md", rel_key, "A fact from segment one.") elif segment[0] == 2: - (Path(config.WIKI_DIR) / "misc" / "invalid.md").write_text("no frontmatter at all\n", encoding="utf-8") + (Path(config.wiki_dir()) / "misc" / "invalid.md").write_text("no frontmatter at all\n", encoding="utf-8") fake_agent(side_effect=fake) report = ingest.ingest() diff --git a/tests/test_ingest_core.py b/tests/test_ingest_core.py index c236c70..8fe67b0 100644 --- a/tests/test_ingest_core.py +++ b/tests/test_ingest_core.py @@ -61,7 +61,7 @@ def fake(rel_key, kind="ingest"): # Windows. The frontmatter is well-formed; only the BOM precedes it. This reproduces # the run that failed with 'missing required field' on every field of every page — # the BOM hid the frontmatter from the parser so it looked empty. - target = config.WIKI_DIR / "concepts" / "transformer.md" + target = config.wiki_dir() / "concepts" / "transformer.md" target.parent.mkdir(parents=True, exist_ok=True) target.write_text("\ufeff" + okf.dump(frontmatter, body), encoding="utf-8") @@ -244,7 +244,7 @@ def fake(rel_key, kind="ingest"): {"type": "Concept", "title": "Foo", "description": "d", "tags": ["x"], "resource": "raw/notes.md"}, "A fact.[^s1]\n\n## Sources\n\n[^s1]: [raw/notes.md](../../raw/notes.md) - n\n", ) - (config.WIKI_DIR / "index.md").write_text("GARBAGE the agent should not write\n", encoding="utf-8") + (config.wiki_dir() / "index.md").write_text("GARBAGE the agent should not write\n", encoding="utf-8") fake_agent(side_effect=fake) (raw / "notes.md").write_text("x\n", encoding="utf-8") @@ -289,7 +289,7 @@ def fake(rel_key, kind="ingest"): "Self-attention subsumes attention.[^s1]\n\n## Sources\n\n" "[^s1]: [raw/notes.md](../../raw/notes.md) - notes (ingested 2026-06-22)\n", ) - (config.WIKI_DIR / "concepts" / "attention.md").unlink() + (config.wiki_dir() / "concepts" / "attention.md").unlink() # The agent repoints the inbound link itself. seed_page( "concepts/linker.md", @@ -332,7 +332,7 @@ def test_repair_renames_repoints_after_rename(tmp_citadel, fake_agent, seed_page def fake(rel_key, kind="ingest"): # Rename a.md -> aa.md (SAME title 'Alpha'); do NOT touch linker. - (config.WIKI_DIR / "concepts" / "a.md").unlink() + (config.wiki_dir() / "concepts" / "a.md").unlink() seed_page( "concepts/aa.md", {"type": "Concept", "title": "Alpha", "description": "d", "tags": ["x"], "resource": "raw/old.md"}, @@ -368,7 +368,7 @@ def test_agent_delete_leaves_broken_link_surfaced(tmp_citadel, fake_agent, seed_ ) def fake(rel_key, kind="ingest"): - (config.WIKI_DIR / "concepts" / "a.md").unlink() # nothing created in its place + (config.wiki_dir() / "concepts" / "a.md").unlink() # nothing created in its place fake_agent(side_effect=fake) report = ingest.ingest([str(raw / "notes.md")]) diff --git a/tests/test_ingest_parallel.py b/tests/test_ingest_parallel.py new file mode 100644 index 0000000..e8a372f --- /dev/null +++ b/tests/test_ingest_parallel.py @@ -0,0 +1,424 @@ +"""Bounded parallel ingest (``citadel ingest --jobs N``) and the context-local wiki redirect it +rides on — all offline, no CLI, no network (the agent seam is the usual :class:`FakeAgent`). + +The concurrency is proven, not assumed: the fake sessions synchronize on a ``threading.Barrier``, +so a test that requires two sources to be in flight AT ONCE simply cannot pass on a serial run (the +barrier times out and the sources fail). The safety properties get their own tests, each pinned to +the failure it prevents: + +* a concurrent source's pages must survive another source's promote (the base-aware prune); +* two sessions that wrote the SAME page must not silently pick one — the loser is re-run serially, + against the wiki the winner left behind; +* the manifest/report bookkeeping stays single-threaded, and an interrupt still keeps the work that + was already promoted. +""" + +from __future__ import annotations + +import io +import threading +import time +from pathlib import Path + +import pytest + +from citadel import cli, config, ingest, llm, manifest + + +BARRIER_TIMEOUT = 10 # generous: a loaded CI box must never flake, a serial run still fails fast + + +def _sources(cit, names: list[str]) -> None: + """Write one trivial raw source per name (``raw/.md``).""" + for name in names: + (cit.raw / f"{name}.md").write_text(f"Content of {name}.\n", encoding="utf-8") + + +def _page_writer(cite_page, barrier: threading.Barrier | None = None, page_for=None): + """A fake-session body that (optionally) meets its siblings at ``barrier`` — proving the + sessions overlap — and then writes ONE valid page for the source it was called with. + + ``page_for(rel_key, attempt)`` chooses the page path; the default gives every source its own + page. The page is written into ``config.wiki_dir()`` at call time, i.e. into ingest's per-source + staging copy, exactly like the real agent's edits.""" + attempts: dict[str, int] = {} + lock = threading.Lock() + + def session(rel_key: str, kind: str = "ingest", *_args, **_kwargs) -> None: + with lock: + attempts[rel_key] = attempts.get(rel_key, 0) + 1 + attempt = attempts[rel_key] + if barrier is not None and attempt == 1: + barrier.wait(timeout=BARRIER_TIMEOUT) + slug = rel_key.rsplit("/", 1)[-1].replace(".", "-") + rel_path = page_for(rel_key, attempt) if page_for is not None else f"misc/{slug}.md" + cite_page(rel_path, rel_key, f"A fact from {slug} (attempt {attempt}).") + + session.attempts = attempts # type: ignore[attr-defined] + return session + + +# --- the redirect that unblocked this: context-local, never process-global -------------------- + + +def test_wiki_redirect_is_per_thread(tmp_citadel): + """The staging redirect must be invisible to every OTHER thread — that is the whole reason + parallel sources can each stage their own copy of the wiki. + + Both other-thread shapes are checked while the redirect is held: a SIBLING worker (neither + thread started the other — the actual ``--jobs`` shape, two sources in flight) and the MAIN + thread that started the holder, which is where the run's own bookkeeping reads the live wiki.""" + staging = tmp_citadel.root / "staging" + seen: dict[str, Path] = {} + inside = threading.Event() + release = threading.Event() + + def other() -> None: + sibling = threading.Thread(target=lambda: seen.__setitem__("sibling", config.wiki_dir())) + sibling.start() + sibling.join(timeout=BARRIER_TIMEOUT) + seen["main"] = config.wiki_dir() + release.set() + + def holder() -> None: + with config.wiki_redirect(staging): + seen["holder"] = config.wiki_dir() + seen["holder_manifest"] = config.manifest_path() + seen["holder_env"] = Path(config.child_env()["CITADEL_WIKI_DIR"]) + inside.set() + release.wait(timeout=BARRIER_TIMEOUT) + seen["holder_after"] = config.wiki_dir() + + t = threading.Thread(target=holder) + t.start() + inside.wait(timeout=BARRIER_TIMEOUT) + other() + t.join(timeout=BARRIER_TIMEOUT) + + assert seen["holder"] == staging + assert seen["holder_manifest"] == staging / ".citadel_ingested.json" + assert seen["holder_env"] == staging # the child process sees the redirect through its own env + assert seen["sibling"] == tmp_citadel.wiki # a sibling worker keeps the live wiki + assert seen["main"] == tmp_citadel.wiki # and so does the thread that started the holder + assert seen["holder_after"] == tmp_citadel.wiki # and the redirect is restored on exit + assert config.WIKI_DIR == tmp_citadel.wiki # the module attribute is never assigned at all + + +# --- the concurrency itself ------------------------------------------------------------------- + + +def test_jobs_runs_sources_concurrently(tmp_citadel, fake_agent, cite_page): + """Three sources, ``--jobs 3``: all three sessions must be in flight at the same moment. The + barrier is the proof — on a serial run it would time out and every source would fail.""" + _sources(tmp_citadel, ["a", "b", "c"]) + barrier = threading.Barrier(3) + agent = fake_agent(side_effect=_page_writer(cite_page, barrier)) + + report = ingest.ingest(jobs=3) + + assert agent.count == 3 + assert sorted(report.processed) == ["raw/a.md", "raw/b.md", "raw/c.md"] + assert report.errors == [] + assert report.raced == [] + assert sorted(manifest.load()) == ["raw/a.md", "raw/b.md", "raw/c.md"] + for name in ("a", "b", "c"): + assert (tmp_citadel.wiki / "misc" / f"{name}-md.md").is_file() + + +def test_default_run_stays_strictly_serial(tmp_citadel, fake_agent, cite_page): + """The default is unchanged behavior: never more than one session in flight.""" + _sources(tmp_citadel, ["a", "b", "c"]) + in_flight = 0 + peak = 0 + lock = threading.Lock() + + def session(rel_key: str, kind: str = "ingest", *_args, **_kwargs) -> None: + nonlocal in_flight, peak + with lock: + in_flight += 1 + peak = max(peak, in_flight) + try: + slug = rel_key.rsplit("/", 1)[-1].replace(".", "-") + cite_page(f"misc/{slug}.md", rel_key, "A fact.") + finally: + with lock: + in_flight -= 1 + + fake_agent(side_effect=session) + report = ingest.ingest() # no jobs= -> config.JOBS, which defaults to 1 + + assert peak == 1 + assert sorted(report.processed) == ["raw/a.md", "raw/b.md", "raw/c.md"] + + +def test_concurrent_promote_keeps_the_other_source_pages(tmp_citadel, fake_agent, cite_page): + """The base-aware prune. Both sources clone the wiki BEFORE either promotes, so each one's + staging copy lacks the other's page. Pruning "everything live has and staging does not" would + delete the source that promoted first — the page must survive.""" + _sources(tmp_citadel, ["a", "b"]) + fake_agent(side_effect=_page_writer(cite_page, threading.Barrier(2))) + + report = ingest.ingest(jobs=2) + + assert report.errors == [] + assert report.raced == [] + assert (tmp_citadel.wiki / "misc" / "a-md.md").is_file() + assert (tmp_citadel.wiki / "misc" / "b-md.md").is_file() + + +def test_seeded_pages_survive_a_parallel_run(tmp_citadel, fake_agent, cite_page, seed_page): + """A page NO source in this run touches is never collateral damage of a base-aware prune.""" + seed_page( + "concepts/existing.md", + {"type": "Concept", "title": "Existing", "description": "d", "tags": ["t"]}, + "Prior knowledge.[^llm1]\n\n## Sources\n\n[^llm1]: LLM - model knowledge\n", + ) + _sources(tmp_citadel, ["a", "b"]) + fake_agent(side_effect=_page_writer(cite_page, threading.Barrier(2))) + + ingest.ingest(jobs=2) + + assert (tmp_citadel.wiki / "concepts" / "existing.md").is_file() + + +def test_a_promote_applies_only_its_own_delta(tmp_citadel, fake_agent, cite_page, seed_page): + """A source's staging copy also holds untouched copies of every page it did NOT write. Judging + "what changed" against the LIVE wiki instead of against the clone would make a page a concurrent + source just rewrote look like this source's change — and copying the untouched staging copy over + it would silently revert that work. Here source ``b`` touches only its own page while ``a`` + rewrites a pre-existing one: ``a``'s rewrite must survive, and nothing may count as a race.""" + seed_page( + "concepts/shared.md", + {"type": "Concept", "title": "Shared", "description": "d", "tags": ["t"]}, + "Original text.[^llm1]\n\n## Sources\n\n[^llm1]: LLM - model knowledge\n", + ) + _sources(tmp_citadel, ["a", "b"]) + barrier = threading.Barrier(2) + + def session(rel_key: str, kind: str = "ingest", *_args, **_kwargs) -> None: + barrier.wait(timeout=BARRIER_TIMEOUT) # both clones are taken before either promote + slug = rel_key.rsplit("/", 1)[-1].replace(".", "-") + cite_page(f"misc/{slug}.md", rel_key, "A fact.") + if rel_key == "raw/a.md": + cite_page("concepts/shared.md", rel_key, "Rewritten by a.") + + fake_agent(side_effect=session) + report = ingest.ingest(jobs=2) + + assert report.raced == [] # b never touched shared.md, so there is nothing to race over + assert report.errors == [] + assert "Rewritten by a." in (tmp_citadel.wiki / "concepts" / "shared.md").read_text(encoding="utf-8") + + +def test_racing_sources_are_re_run_serially(tmp_citadel, fake_agent, cite_page): + """Two sessions that both write the SAME page cannot both be right: the second promote is + refused (its base no longer matches), and that source is re-run serially afterwards — where it + sees the winner's page and merges into it. Nothing is reported as an error, and the extra + session is surfaced as a race so a corpus that races a lot can be dialled back.""" + _sources(tmp_citadel, ["a", "b"]) + barrier = threading.Barrier(2) + + def merged_body(rel_key: str, attempt: int) -> str: + # Both sources aim at ONE page; the re-run (attempt 2) writes it again, this time on top of + # what the winner promoted. + return "concepts/shared.md" + + session = _page_writer(cite_page, barrier, page_for=merged_body) + agent = fake_agent(side_effect=session) + + report = ingest.ingest(jobs=2) + + assert len(report.raced) == 1 # exactly one source lost the race + assert report.errors == [] + assert sorted(report.processed) == ["raw/a.md", "raw/b.md"] + assert agent.count == 3 # two first attempts + the loser's serial re-run + loser = report.raced[0] + assert session.attempts[loser] == 2 # type: ignore[attr-defined] + # Both sources are recorded as ingested, and the page holds the re-run's (merged) text. + assert sorted(manifest.load()) == ["raw/a.md", "raw/b.md"] + page = (tmp_citadel.wiki / "concepts" / "shared.md").read_text(encoding="utf-8") + assert "attempt 2" in page + + +def test_race_re_run_failure_is_a_normal_source_failure(tmp_citadel, fake_agent, cite_page): + """If the serial re-run itself fails, the source fails like any other: nothing promoted for it, + an error on the report, and no manifest entry — so the next run retries it.""" + _sources(tmp_citadel, ["a", "b"]) + barrier = threading.Barrier(2) + writer = _page_writer(cite_page, barrier, page_for=lambda rel_key, attempt: "concepts/shared.md") + + def session(rel_key: str, kind: str = "ingest", *args, **kwargs) -> None: + writer(rel_key, kind, *args, **kwargs) + if writer.attempts[rel_key] == 2: # type: ignore[attr-defined] + raise RuntimeError("the re-run session failed") + + fake_agent(side_effect=session) + report = ingest.ingest(jobs=2) + + assert len(report.raced) == 1 + loser = report.raced[0] + assert report.processed == [k for k in ("raw/a.md", "raw/b.md") if k != loser] + assert any("the re-run session failed" in e for e in report.errors) + assert loser not in manifest.load() + + +def test_interrupt_keeps_the_work_already_promoted(tmp_citadel, fake_agent, cite_page): + """A Ctrl+C during a parallel run still re-raises — but a source that had already been promoted + is recorded, so the run never pays twice for work that is on the live wiki.""" + _sources(tmp_citadel, ["a", "b"]) + done_a = threading.Event() + + def session(rel_key: str, kind: str = "ingest", *_args, **_kwargs) -> None: + if rel_key == "raw/a.md": + cite_page("misc/a-md.md", rel_key, "A fact.") + done_a.set() + return + done_a.wait(timeout=BARRIER_TIMEOUT) # let a finish and promote first + raise KeyboardInterrupt + + fake_agent(side_effect=session) + + with pytest.raises(KeyboardInterrupt): + ingest.ingest(jobs=2) + + tracked = manifest.load() + assert "raw/a.md" in tracked # promoted AND recorded + assert "raw/b.md" not in tracked + assert (tmp_citadel.wiki / "misc" / "a-md.md").is_file() + + +def test_a_clone_hashes_identically_to_what_it_copied(tmp_citadel, seed_page): + """The base is hashed off the fresh staging clone rather than off the live wiki, so the lock + covers the copy alone and the wiki is read once instead of twice. That is only sound while a + clone is byte-for-byte what it copied — for exactly the file set `_content_files` considers, + including the non-`.md` files and the nested folders a promote also syncs.""" + seed_page("concepts/a.md", {"type": "Concept", "title": "A", "description": "d", "tags": ["t"]}, "Body.\n") + seed_page("persons/b.md", {"type": "Person", "title": "B", "description": "d", "tags": ["t"]}, "Body.\n") + (tmp_citadel.wiki / "concepts" / "attachment.txt").write_text("not markdown\n", encoding="utf-8") + (tmp_citadel.wiki / ".citadel_ingested.json").write_text("{}", encoding="utf-8") # excluded either way + + staging = ingest._make_staging(tmp_citadel.wiki) + try: + assert ingest._content_hashes(staging) == ingest._content_hashes(tmp_citadel.wiki) + assert set(ingest._content_hashes(staging)) == {"concepts/a.md", "concepts/attachment.txt", "persons/b.md"} + finally: + ingest._robust_rmtree(staging) + + +def test_promote_leaves_in_flight_state_temps_alone(tmp_citadel): + """The promote's leftover-temp sweep must not touch a HIDDEN ``*.citadeltmp``: that is an + in-flight ``config.atomic_write_text`` of the manifest or the failures catalog. Under + ``--jobs N`` the main thread saves the manifest while a worker promotes, and sweeping its temp + turned a routine save into a FileNotFoundError.""" + live = tmp_citadel.wiki + (live / "concepts").mkdir(parents=True, exist_ok=True) + (live / "concepts" / "page.md").write_text("live\n", encoding="utf-8") + staging = tmp_citadel.root / "staging" + (staging / "concepts").mkdir(parents=True) + (staging / "concepts" / "page.md").write_text("live\n", encoding="utf-8") + + in_flight = live / ".citadel_ingested.json.4321.citadeltmp" + in_flight.write_text("{}", encoding="utf-8") + leftover = live / "concepts" / "page.md.citadeltmp" + leftover.write_text("half-written\n", encoding="utf-8") + + ingest._promote(staging, live) + + assert in_flight.is_file() # another writer owns it + assert not leftover.exists() # a hard-killed promote's own leftover is still swept + + +def test_progress_callback_is_never_invoked_concurrently(tmp_citadel, fake_agent, cite_page): + """Progress events now fire from worker threads, so the callback — anything a caller passed — + must still be handed one event at a time. A callback that is not itself thread-safe (the common + case: a counter, a file handle, an accumulating list) would otherwise corrupt silently, since + ingest swallows callback exceptions by design.""" + _sources(tmp_citadel, ["a", "b", "c"]) + overlaps: list[str] = [] + inside = 0 + barrier = threading.Barrier(3) + + def progress(event: str, data: dict) -> None: + nonlocal inside + inside += 1 # deliberately unguarded: this is the caller's naive callback + if inside != 1: + overlaps.append(event) + time.sleep(0.001) # widen the window a racing thread would slip into + inside -= 1 + + fake_agent(side_effect=_page_writer(cite_page, barrier)) + ingest.ingest(jobs=3, progress=progress) + + assert overlaps == [] + + +def test_a_raced_source_still_reports_what_it_spent_and_reused(tmp_citadel, fake_agent, monkeypatch): + """A conflict is not recorded as a source outcome — it is re-run serially — but the session it + already paid for and any checkpoint it already replayed are facts of the run either way. The + session runner is faked here: the point is the DRIVER's bookkeeping, not another real race.""" + _sources(tmp_citadel, ["a", "b"]) + fake_agent() + attempts: dict[str, int] = {} + + def fake_sessions(session_fns, rel_key, *, concurrent=False, **_kw): + attempts[rel_key] = attempts.get(rel_key, 0) + 1 + if rel_key == "raw/a.md" and attempts[rel_key] == 1: + return ingest._SourceOutcome( + False, + conflict=True, + usage=llm.SessionUsage(cost_usd=0.25), + resumed_note="raw/a.md (segments 1-2 of 4 restored from checkpoint)", + ) + return ingest._SourceOutcome(True, usage=llm.SessionUsage(cost_usd=0.25)) + + monkeypatch.setattr(ingest, "_run_agent_sessions", fake_sessions) + report = ingest.ingest(jobs=2) + + assert report.raced == ["raw/a.md"] + assert attempts["raw/a.md"] == 2 # the conflict really was re-run + assert report.resumed == ["raw/a.md (segments 1-2 of 4 restored from checkpoint)"] + # Three sessions were paid for: b, a's raced attempt, and a's serial re-run. + assert report.usage is not None and report.usage.cost_usd == pytest.approx(0.75) + + +# --- the knob --------------------------------------------------------------------------------- + + +def test_jobs_below_one_is_refused(tmp_citadel, fake_agent): + fake_agent() + with pytest.raises(ValueError, match="at least 1"): + ingest.ingest(jobs=0) + + +def test_cli_jobs_is_threaded_through(tmp_citadel, monkeypatch, capsys): + seen: dict = {} + + def fake_ingest(paths=None, progress=None, full_rescan=False, force=False, jobs=None): + seen["jobs"] = jobs + return ingest.IngestReport([], [], [], []) + + monkeypatch.setattr(ingest, "ingest", fake_ingest) + assert cli.main(["ingest", "--jobs", "4", "--quiet"]) == 0 + assert seen["jobs"] == 4 + + +def test_cli_rejects_a_zero_job_count(tmp_citadel, capsys): + assert cli.main(["ingest", "--jobs", "0"]) == 2 + assert "at least 1" in capsys.readouterr().err + + +def test_config_clamps_a_bad_jobs_value(monkeypatch): + monkeypatch.setenv("CITADEL_JOBS", "0") + monkeypatch.setattr(config, "CONFIG_WARNINGS", []) + assert config._jobs_setting() == 1 + assert any("CITADEL_JOBS" in w for w in config.CONFIG_WARNINGS) + + +def test_progress_drops_the_spinner_when_running_parallel(): + """With several sources in flight there is no single "current source" for the spinner to name, + so the console falls back to a start line per source.""" + from citadel.progress import ConsoleProgress + + progress = ConsoleProgress(stream=io.StringIO()) + progress("start", {"pending": 3, "skipped": 0, "jobs": 3}) + assert progress.spinner is False diff --git a/tests/test_ingest_parallel_resume.py b/tests/test_ingest_parallel_resume.py new file mode 100644 index 0000000..f6848c8 --- /dev/null +++ b/tests/test_ingest_parallel_resume.py @@ -0,0 +1,129 @@ +"""Where ``--jobs N`` meets the resume checkpoints of a CHUNKED source (offline). + +A checkpoint is a promise about the live wiki: "these pages should hold these bytes, these should +be gone, and here is what they looked like when I banked it". Under concurrency that promise has a +second party — another source promoting between this one's clone and its checkpoint — so the delta +must be measured against the wiki THIS source was cloned from, never against a live wiki that has +since moved on. Both tests below fail (silently corrupting the wiki) when it is measured against +live: a checkpoint is durable, so the damage outlives the parallel run and lands in a later, +possibly serial, one. +""" + +from __future__ import annotations + +import threading + +import pytest + +from citadel import config, ingest, manifest + + +TIMEOUT = 10 + + +def _paras(n: int) -> str: + """n paragraphs, each individually identifiable — the chunking fixture (mirrors + tests/test_ingest_resume.py).""" + return "\n\n".join(f"Paragraph number {i} with some filler content about topic {i}." for i in range(n)) + + +def _await_live(path, timeout: float = TIMEOUT) -> None: + """Block until ``path`` exists in the LIVE wiki — i.e. until the other source's promote has + landed. The one ordering the tests need: a promote is not observable through the agent seam, + but its result is.""" + deadline = threading.Event() + for _ in range(int(timeout * 100)): + if path.exists(): + return + deadline.wait(0.01) + raise AssertionError(f"timed out waiting for {path} to be promoted") + + +@pytest.fixture +def chunked_and_plain(tmp_citadel, monkeypatch): + """One CHUNKED source (3 segments) plus one ordinary source, so a run with ``--jobs 2`` has a + checkpointing source and a concurrent promoter.""" + monkeypatch.setattr(config, "MAX_SOURCE_CHARS", 120) + (tmp_citadel.raw / "big.txt").write_text(_paras(6), encoding="utf-8") + (tmp_citadel.raw / "b.md").write_text("Content of b.\n", encoding="utf-8") + return tmp_citadel + + +def test_checkpoint_never_banks_a_concurrent_source_page_as_deleted(chunked_and_plain, fake_agent, cite_page): + """A page a CONCURRENT source created is not this source's deletion. + + Run 1 (``--jobs 2``): the chunked source clones the wiki, source ``b`` then promotes + ``misc/b-md.md``, and only afterwards does segment 1 finish and bank its checkpoint. Measured + against live, ``b``'s page is "in live, not in my staging" — a deletion. Run 2 replays that + delta and prunes a fully-ingested source's page off the live wiki: no conflict, no error, no + delete session, just a page gone.""" + cit = chunked_and_plain + b_page = cit.wiki / "misc" / "b-md.md" + + cloned = threading.Event() + + def session(rel_key, kind="ingest", read_path=None, segment=None, **_kw): + if rel_key == "raw/b.md": + # Hold b back until the chunked source has cloned (its first session proves it), so the + # ordering under test is the real one: clone, THEN a concurrent promote, THEN the + # checkpoint. + assert cloned.wait(timeout=TIMEOUT) + cite_page("misc/b-md.md", rel_key, "A fact from b.") + return + if segment[0] == 1: + cloned.set() + _await_live(b_page) # bank the checkpoint AFTER b's promote landed + cite_page("misc/big.md", rel_key, "A fact from segment one.") + if segment[0] == 2: + raise RuntimeError("segment 2 boom") + + fake_agent(side_effect=session) + first = ingest.ingest(jobs=2) + assert first.processed == ["raw/b.md"] # b is in; the chunked source failed at segment 2 + assert b_page.is_file() + + # Run 2 is serial and touches only the chunked source — it has no business deleting anything. + def finish(rel_key, kind="ingest", read_path=None, segment=None, **_kw): + cite_page("misc/big.md", rel_key, f"A fact from segment {segment[0]}.") + + fake_agent(side_effect=finish) + second = ingest.ingest() + + assert second.errors == [] + assert "raw/b.md" in manifest.load() # still recorded as ingested... + assert b_page.is_file(), "a concurrent source's promoted page was pruned by a replayed checkpoint" + + +def test_a_raced_chunked_source_really_re_runs(chunked_and_plain, fake_agent, cite_page): + """The ``raced`` contract must hold for chunked sources too: re-run serially, see the winner's + page, merge into it. + + Both sources write ``concepts/shared.md``; the chunked one banks a checkpoint after its LAST + segment (by design — a promote that then fails should replay for free), and its promote is then + refused because ``b`` got there first. If that checkpoint survives into the serial re-run, the + re-run opens at ``completed == total``, runs ZERO sessions, and promotes its stale copy over the + winner's page — the exact opposite of the documented merge.""" + cit = chunked_and_plain + shared = cit.wiki / "concepts" / "shared.md" + sessions: list[tuple[str, int | None]] = [] + + cloned = threading.Event() + + def session(rel_key, kind="ingest", read_path=None, segment=None, **_kw): + sessions.append((rel_key, segment[0] if segment else None)) + if rel_key == "raw/b.md": + assert cloned.wait(timeout=TIMEOUT) # the chunked source clones first + cite_page("concepts/shared.md", rel_key, "The fact b wrote.") + return + if segment[0] == 1: + cloned.set() + _await_live(shared) # let b win the race for the shared page + cite_page("concepts/shared.md", rel_key, f"The fact big wrote (segment {segment[0]}).") + + fake_agent(side_effect=session) + report = ingest.ingest(jobs=2) + + assert report.raced == ["raw/big.txt"] + re_run = [seg for key, seg in sessions if key == "raw/big.txt" and seg is not None] + assert re_run.count(1) == 2, "the serial re-run must actually open a session, not replay a stale checkpoint" + assert report.errors == [] diff --git a/tests/test_ingest_progress.py b/tests/test_ingest_progress.py index 31eab14..c6ccbec 100644 --- a/tests/test_ingest_progress.py +++ b/tests/test_ingest_progress.py @@ -23,7 +23,7 @@ def test_ingest_emits_progress_events(tmp_citadel, fake_agent, transformer_page) for expected in ("source_start", "source_done", "finalize", "done"): assert expected in names, f"missing event: {expected}" start = next(d for e, d in events if e == "start") - assert start == {"pending": 1, "skipped": 0, "moved": 0, "unreadable": 0, "deleted": 0, "repos": 0} + assert start == {"pending": 1, "skipped": 0, "moved": 0, "unreadable": 0, "deleted": 0, "repos": 0, "jobs": 1} done = next(d for e, d in events if e == "source_done") assert done["source"] == "raw/notes.md" assert done["index"] == 1 and done["total"] == 1 @@ -73,7 +73,17 @@ def fake(rel_key, kind="ingest", read_path=None, segment=None): "finalize", "done", ] - assert events[0][1] == {"pending": 1, "skipped": 0, "moved": 0, "unreadable": 0, "deleted": 1, "repos": 1} + assert events[0][1] == { + "pending": 1, + "skipped": 0, + "moved": 0, + "unreadable": 0, + "deleted": 1, + "repos": 1, + # `--jobs N`: the worker count the run was given, so the console can drop the spinner when + # several sources are in flight at once. 1 is the serial default. + "jobs": 1, + } # Deletions first, then files, then repos — and per-GROUP counters restarting at 1/1. assert [d["source"] for e, d in events if e == "source_start"] == ["raw/gone.md", "raw/note.md", "raw/svc"] for event, data in events: diff --git a/tests/test_ingest_reconcile_delete.py b/tests/test_ingest_reconcile_delete.py index 5ef66cd..0aa78d9 100644 --- a/tests/test_ingest_reconcile_delete.py +++ b/tests/test_ingest_reconcile_delete.py @@ -39,7 +39,7 @@ def test_deleted_source_citations_reconciled_out(tmp_citadel, fake_agent, seed_c def fake(rel_key, kind="ingest"): # The deleted source was this page's only provenance -> remove the page entirely. - (config.WIKI_DIR / "concepts" / "topic.md").unlink() + (config.wiki_dir() / "concepts" / "topic.md").unlink() agent = fake_agent(side_effect=fake) diff --git a/tests/test_ingest_resume.py b/tests/test_ingest_resume.py index 0bffb5d..bcfb347 100644 --- a/tests/test_ingest_resume.py +++ b/tests/test_ingest_resume.py @@ -89,7 +89,7 @@ def test_checkpoint_captures_edits_no_session_diff_reports(chunked_source, fake_ wiki = chunked_source.wiki def page(rel_path: str, title: str, body: str) -> None: - target = Path(config.WIKI_DIR) / rel_path + target = Path(config.wiki_dir()) / rel_path target.parent.mkdir(parents=True, exist_ok=True) target.write_text( okf.dump( @@ -105,7 +105,7 @@ def fake(rel_key, kind="ingest", read_path=None, segment=None): page("concepts/kaffee.md", "Kaffee", "A fact.[^s1]" + sources) page("concepts/hub.md", "Hub", "See [Kaffee](kaffee.md).[^s1]" + sources) elif segment[0] == 2: # rename: same title, new path -> _repair_renames repoints hub.md - (Path(config.WIKI_DIR) / "concepts" / "kaffee.md").unlink() + (Path(config.wiki_dir()) / "concepts" / "kaffee.md").unlink() page("concepts/coffee.md", "Kaffee", "A fact.[^s1]" + sources) elif segment[0] == 3: raise RuntimeError("segment three boom") @@ -264,7 +264,7 @@ def test_replay_that_no_longer_validates_discards_the_checkpoint(chunked_source, def fake(rel_key, kind="ingest", read_path=None, segment=None): if segment[0] == 1: - target = Path(config.WIKI_DIR) / "misc" / "big.md" + target = Path(config.wiki_dir()) / "misc" / "big.md" target.parent.mkdir(parents=True, exist_ok=True) target.write_text( okf.dump( diff --git a/tests/test_ingest_staging.py b/tests/test_ingest_staging.py index 3dce8f0..9e6b44d 100644 --- a/tests/test_ingest_staging.py +++ b/tests/test_ingest_staging.py @@ -363,7 +363,7 @@ def test_agent_edits_staging_sibling_not_live(tmp_citadel, fake_agent, seed_page seen = {} def fake(rel_key, kind="ingest"): - staging = config.WIKI_DIR + staging = config.wiki_dir() seen["staging"] = staging seen["is_sibling"] = staging != wiki and staging.parent == wiki.parent # The live wiki must not yet hold this page while the agent is mid-session. @@ -381,7 +381,7 @@ def fake(rel_key, kind="ingest"): assert seen["is_sibling"] # staging is a sibling of live, not a temp dir assert seen["live_clean_midsession"] # live untouched while the agent worked assert (wiki / "concepts" / "transformer.md").exists() # promoted after a clean session - assert config.WIKI_DIR == wiki # redirect restored + assert config.wiki_dir() == wiki # redirect restored import os as _os assert "CITADEL_WIKI_DIR" not in _os.environ # env restored (was unset) @@ -498,7 +498,7 @@ def test_session_that_deletes_all_pages_is_refused_not_promoted(tmp_citadel, fak def fake(rel_key, kind="ingest"): # The agent wipes every content page from its staging copy (adds nothing back). - for p in config.WIKI_DIR.rglob("*.md"): + for p in config.wiki_dir().rglob("*.md"): if p.name not in ("index.md", "log.md"): p.unlink() diff --git a/tests/test_netdrive.py b/tests/test_netdrive.py index d3b598f..af95653 100644 --- a/tests/test_netdrive.py +++ b/tests/test_netdrive.py @@ -214,9 +214,9 @@ def test_find_and_rewrite_raw_references_with_absolute_keys(tmp_citadel_external def _fake_session(rel_key, kind="ingest"): """Write one Concept page (as the agent would) into the configured WIKI_DIR, citing the raw file by its real RELATIVE path and recording the (possibly absolute) source key as resource.""" - target = config.WIKI_DIR / "concepts" / "transformer.md" + target = config.wiki_dir() / "concepts" / "transformer.md" target.parent.mkdir(parents=True, exist_ok=True) - rel_link = os.path.relpath(config.source_path_for_key(rel_key), config.WIKI_DIR / "concepts").replace(os.sep, "/") + rel_link = os.path.relpath(config.source_path_for_key(rel_key), config.wiki_dir() / "concepts").replace(os.sep, "/") target.write_text( okf.dump( { @@ -280,7 +280,7 @@ def test_ingest_deletes_out_of_repo_source(tmp_citadel_external, seed_page, fake manifest.save({abs_key: "deadbeef"}) def unlink_topic(rel_key, kind="ingest"): - (config.WIKI_DIR / "concepts" / "topic.md").unlink() + (config.wiki_dir() / "concepts" / "topic.md").unlink() agent = fake_agent(side_effect=unlink_topic) @@ -341,7 +341,7 @@ def test_ingest_canonicalizes_shortened_resource_for_out_of_repo_source(tmp_path def shortened_session(rel_key, kind="ingest"): # The agent does the work but SHORTENS the long absolute key to the conventional form. - page = config.WIKI_DIR / "concepts" / "internal-data-analysis.md" + page = config.wiki_dir() / "concepts" / "internal-data-analysis.md" page.parent.mkdir(parents=True, exist_ok=True) rel_link = os.path.relpath(config.source_path_for_key(rel_key), page.parent).replace(os.sep, "/") page.write_text( diff --git a/tests/test_pagecache.py b/tests/test_pagecache.py index 77073ca..e9b3650 100644 --- a/tests/test_pagecache.py +++ b/tests/test_pagecache.py @@ -129,7 +129,7 @@ def test_same_length_rewrite_is_seen(tmp_citadel, seed_page, counted, warm): timestamp resolution.""" _seed_three(seed_page) store.load() - target = Path(config.WIKI_DIR) / "concepts/coffee.md" + target = Path(config.wiki_dir()) / "concepts/coffee.md" before = target.stat() target.write_text(okf.dump(PAGE_FM, "Coffee is brewed from ROASTED beans.\n"), encoding="utf-8") assert target.stat().st_size == before.st_size # the case size cannot catch @@ -149,7 +149,7 @@ def test_new_page_is_seen(tmp_citadel, seed_page, counted, warm): def test_deleted_page_is_seen(tmp_citadel, seed_page, counted, warm): _seed_three(seed_page) store.load() - (Path(config.WIKI_DIR) / "concepts/tea.md").unlink() + (Path(config.wiki_dir()) / "concepts/tea.md").unlink() assert len(store.load()) == 2 assert counted() == 2 @@ -158,7 +158,7 @@ def test_renamed_page_is_seen(tmp_citadel, seed_page, counted, warm): """A move keeps size and content; the fingerprint is keyed by rel_path, so it still misses.""" _seed_three(seed_page) store.load() - wiki = Path(config.WIKI_DIR) + wiki = Path(config.wiki_dir()) (wiki / "concepts/tea.md").rename(wiki / "concepts/black-tea.md") assert "concepts/black-tea.md" in [p.rel_path for p in store.load()] assert counted() == 2 @@ -169,7 +169,7 @@ def test_generated_files_do_not_invalidate(tmp_citadel, seed_page, counted, warm regenerated on every run and are not pages, so rewriting them must not cost a re-parse.""" _seed_three(seed_page) store.load() - wiki = Path(config.WIKI_DIR) + wiki = Path(config.wiki_dir()) (wiki / "index.md").write_text("# Index\n\nregenerated\n", encoding="utf-8") (wiki / "concepts/index.md").write_text("# concepts\n\nregenerated\n", encoding="utf-8") (wiki / "log.md").write_text("# Log\n\n- entry\n", encoding="utf-8") @@ -184,7 +184,7 @@ def test_a_symlinked_page_is_stat_through_to_its_target(tmp_citadel, seed_page, _seed_three(seed_page) target = tmp_path / "external-page.md" target.write_text(okf.dump({**PAGE_FM, "title": "External"}, "External body.\n"), encoding="utf-8") - link = Path(config.WIKI_DIR) / "concepts/external.md" + link = Path(config.wiki_dir()) / "concepts/external.md" try: link.symlink_to(target) except (OSError, NotImplementedError): # pragma: no cover - Windows without symlink privilege @@ -227,14 +227,14 @@ def test_a_freshly_written_wiki_is_not_cached(tmp_citadel, seed_page, counted, m def test_snapshot_is_keyed_by_wiki_directory(tmp_citadel, seed_page, counted, warm, tmp_path): - """Ingest redirects config.WIKI_DIR at a staging copy; a snapshot of one directory must never + """Ingest redirects config.wiki_dir() at a staging copy; a snapshot of one directory must never answer for another (and the single slot means staging dirs cannot accumulate).""" _seed_three(seed_page) assert len(store.load()) == 3 other = tmp_path / "other-wiki" (other / "concepts").mkdir(parents=True) (other / "concepts/solo.md").write_text(okf.dump(PAGE_FM, "Only page.\n"), encoding="utf-8") - original = config.WIKI_DIR + original = config.wiki_dir() try: config.WIKI_DIR = other assert [p.rel_path for p in store.load()] == ["concepts/solo.md"] @@ -285,13 +285,13 @@ def test_ingest_runs_with_the_cache_bypassed(tmp_citadel, seed_page, fake_agent, def test_fingerprint_is_none_on_a_walk_error(tmp_citadel, seed_page, warm, monkeypatch): """A flaky share (or a directory that vanished mid-walk) yields no fingerprint at all.""" _seed_three(seed_page) - assert pagecache.fingerprint(config.WIKI_DIR, store_core.is_skipped_name) is not None + assert pagecache.fingerprint(config.wiki_dir(), store_core.is_skipped_name) is not None def boom(path): raise OSError("scandir failed") monkeypatch.setattr(os, "scandir", boom) - assert pagecache.fingerprint(config.WIKI_DIR, store_core.is_skipped_name) is None + assert pagecache.fingerprint(config.wiki_dir(), store_core.is_skipped_name) is None def test_no_fingerprint_degrades_to_uncached(tmp_citadel, seed_page, counted, warm, monkeypatch): @@ -308,7 +308,7 @@ def test_a_missing_wiki_directory_is_not_cached(tmp_citadel, counted, warm): """An empty (or not-yet-created) wiki dir: [] every time, from disk, never from a snapshot.""" import shutil - shutil.rmtree(config.WIKI_DIR) + shutil.rmtree(config.wiki_dir()) assert store.load() == [] assert store.load() == [] assert counted() == 2 @@ -322,7 +322,7 @@ def test_a_wiki_that_changes_during_the_load_is_not_cached(tmp_citadel, seed_pag def load_then_change(wiki_dir): pages = real(wiki_dir) - target = Path(config.WIKI_DIR) / "concepts/late.md" + target = Path(config.wiki_dir()) / "concepts/late.md" target.write_text(okf.dump({**PAGE_FM, "title": "Late"}, "Written mid-load.\n"), encoding="utf-8") return pages diff --git a/tests/test_refresh.py b/tests/test_refresh.py index 35af10d..9482f62 100644 --- a/tests/test_refresh.py +++ b/tests/test_refresh.py @@ -203,3 +203,30 @@ def test_cli_refresh_runs_the_budgeted_sessions(tmp_citadel, fake_agent, cite_pa assert agent.calls == [("raw/a.md", "reconcile"), ("raw/b.md", "reconcile")] out = capsys.readouterr().out assert "Refreshing 2 of 2" in out + + +def test_refresh_hands_jobs_through_to_ingest(tmp_citadel, monkeypatch): + """`citadel refresh --jobs N` is the same knob as ingest's, because refresh IS a forced ingest + run: refresh only picks the sources. A refresh slice is ordered by last-checked time rather + than by topic, so it is the lifecycle where parallelism costs the least cross-linking.""" + seen: dict = {} + + def fake_ingest(paths=None, progress=None, full_rescan=False, force=False, jobs=None): + seen["jobs"] = jobs + seen["force"] = force + return ingest.IngestReport([], [], [], []) + + (tmp_citadel.raw / "note.md").write_text("Content.\n", encoding="utf-8") + tracked = manifest.load() + tracked["raw/note.md"] = manifest.make_entry("aa" * 32, "claude:sonnet") + manifest.save(tracked) + + monkeypatch.setattr(ingest, "ingest", fake_ingest) + refresh.refresh(limit=1, jobs=3) + + assert seen == {"jobs": 3, "force": True} + + +def test_refresh_cli_rejects_a_zero_job_count(tmp_citadel, capsys): + assert cli.main(["refresh", "--jobs", "0"]) == 2 + assert "at least 1" in capsys.readouterr().err diff --git a/tests/test_runlock.py b/tests/test_runlock.py index 86d76b8..978beca 100644 --- a/tests/test_runlock.py +++ b/tests/test_runlock.py @@ -125,7 +125,7 @@ def test_ingest_fails_loud_while_another_run_holds_the_lock(tmp_citadel, fake_ag def test_make_staging_no_longer_sweeps_siblings_but_run_start_does(tmp_citadel): - live = config.WIKI_DIR + live = config.wiki_dir() config.robust_mkdir(live) stale = live.parent / f"{ingest._staging_prefix(live)}999.1" stale.mkdir() diff --git a/tests/test_usage_accounting.py b/tests/test_usage_accounting.py index e67853d..81f3476 100644 --- a/tests/test_usage_accounting.py +++ b/tests/test_usage_accounting.py @@ -608,7 +608,7 @@ def test_delete_cleanup_usage_counts_in_run_total(tmp_citadel, fake_agent): ingest.ingest() def delete_citing_page(*args, **kwargs): - (Path(config.WIKI_DIR) / "concepts/topic.md").unlink() + (Path(config.wiki_dir()) / "concepts/topic.md").unlink() agent = fake_agent(side_effect=delete_citing_page, usage=llm.SessionUsage(cost_usd=0.02)) src.unlink()