diff --git a/containers/skillogy.Dockerfile b/containers/skillogy.Dockerfile index 3207f6847..d87c32e32 100644 --- a/containers/skillogy.Dockerfile +++ b/containers/skillogy.Dockerfile @@ -22,9 +22,9 @@ RUN apt-get update \ COPY packages/decepticon/decepticon/skillogy ./decepticon/skillogy COPY packages/decepticon/decepticon/skill_audit ./decepticon/skill_audit -# CI-built graph dump. The builder emits MERGE-only Cypher so re-runs -# against an already-loaded Neo4j are idempotent — the boot script -# below replays this file every time SKILLOGY_AUTO_INGEST is set. +# CI-built graph dump. The boot script seeds it into Neo4j only when the +# graph is empty; the builder emits MERGE-only Cypher so the first-boot +# seed (and any out-of-band incremental re-apply) is idempotent. COPY packages/decepticon/decepticon/skills/.graph/skills.cypher /app/skills.cypher RUN touch ./decepticon/__init__.py @@ -50,10 +50,8 @@ ENV SKILLOGY_REST_PORT=9100 ENV SKILLOGY_NEO4J_URI=bolt://neo4j:7687 ENV SKILLOGY_NEO4J_USER=neo4j -# Cypher auto-ingest on boot. SKILLOGY_AUTO_INGEST=0 disables it — -# useful when an operator pre-loads the graph out of band. +# Baked cypher dump the boot seed reads when the graph is empty. ENV SKILLOGY_CYPHER_PATH=/app/skills.cypher -ENV SKILLOGY_AUTO_INGEST=1 EXPOSE 9100 diff --git a/docker-compose.yml b/docker-compose.yml index 276c22833..6bb7da6c6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -605,7 +605,6 @@ services: - SKILLOGY_NEO4J_URI=bolt://neo4j:7687 - SKILLOGY_NEO4J_USER=neo4j - SKILLOGY_NEO4J_PASSWORD=${NEO4J_PASSWORD:-decepticon-graph} - - SKILLOGY_AUTO_INGEST=${SKILLOGY_AUTO_INGEST:-1} # Hybrid retrieval embeds skills + queries through the same litellm # proxy the agents use (ADR-0011 §"Skillogy↔litellm coupling"). # Unset → find_skill falls back to substring search. diff --git a/packages/decepticon/decepticon/skillogy/__main__.py b/packages/decepticon/decepticon/skillogy/__main__.py index 8590dce22..b4f07dc46 100644 --- a/packages/decepticon/decepticon/skillogy/__main__.py +++ b/packages/decepticon/decepticon/skillogy/__main__.py @@ -5,9 +5,12 @@ 1. Build a ``Neo4jBackend`` against the configured Bolt URI (waits for the graph to be reachable; the compose ``depends_on: neo4j (healthy)`` gating means it should already be up). -2. Optionally ingest the CI-built ``skills.cypher`` dump into Neo4j so - a fresh container boot ends up with the corpus loaded (idempotent — - the builder emits only ``MERGE`` statements). +2. Seed the CI-built ``skills.cypher`` dump into Neo4j **only when the + graph is empty** — a fresh/first-boot database self-heals, and a + persistent one (a managed Neo4j, or a compose volume) is left + untouched instead of re-MERGEing the whole corpus on every boot. + Corpus changes (a new SKILL.md) are an out-of-band incremental + ingest, not a boot concern. 3. Start the FastAPI REST app on ``$SKILLOGY_REST_PORT``. Environment variables: @@ -15,8 +18,9 @@ SKILLOGY_NEO4J_URI (default ``bolt://neo4j:7687``) SKILLOGY_NEO4J_USER (default ``neo4j``) SKILLOGY_NEO4J_PASSWORD (default ``decepticon-graph``) + SKILLOGY_NEO4J_DATABASE (default ``neo4j``; managed backends e.g. Aura + name the database after the instance id) SKILLOGY_CYPHER_PATH (default ``/app/skills.cypher`` — baked into the image) - SKILLOGY_AUTO_INGEST (default ``1``; set ``0`` to skip the bulk load) SKILLOGY_API_KEY (optional Bearer-token auth for the protected endpoints) """ @@ -54,17 +58,27 @@ def _build_backend() -> Neo4jBackend: uri=os.environ.get("SKILLOGY_NEO4J_URI", "bolt://neo4j:7687"), user=os.environ.get("SKILLOGY_NEO4J_USER", "neo4j"), password=os.environ.get("SKILLOGY_NEO4J_PASSWORD", "decepticon-graph"), + database=os.environ.get("SKILLOGY_NEO4J_DATABASE", "neo4j"), ) -def _maybe_ingest(backend: Neo4jBackend) -> None: - if os.environ.get("SKILLOGY_AUTO_INGEST", "1").strip().lower() not in { - "1", - "true", - "yes", - "on", - }: - log.info("SKILLOGY_AUTO_INGEST disabled; skipping cypher load") +def _seed_if_empty(backend: Neo4jBackend) -> None: + """Seed ``skills.cypher`` into Neo4j only when the graph is empty. + + The skill graph is persistent (a managed Neo4j, or a self-hosted + instance with a data volume), so re-running the full corpus on every + boot is wasted work — the builder's ``MERGE`` statements re-touch + thousands of already-present nodes for no net change. Instead we seed + once: a fresh/empty database self-heals on first boot, and a populated + one is left untouched. Adding a skill is a separate incremental ingest + run out of band (rebuild the cypher for the new SKILL.md and apply it), + not something a boot re-derives. + + The seed is idempotent MERGE, so if two replicas race on a cold empty + database the double-write is harmless. + """ + if backend.health()["skill_count"] > 0: + log.info("skill graph already populated; skipping seed") return cypher_path = Path(os.environ.get("SKILLOGY_CYPHER_PATH", "/app/skills.cypher")) if not cypher_path.exists(): @@ -76,7 +90,7 @@ def _maybe_ingest(backend: Neo4jBackend) -> None: return cypher_text = cypher_path.read_text(encoding="utf-8") n = backend.bulk_ingest_cypher(cypher_text) - log.info("ingested %d Cypher statements from %s", n, cypher_path) + log.info("seeded %d Cypher statements from %s", n, cypher_path) # Hybrid retrieval (ADR-0011): once the corpus is loaded, create the # vector index and embed each skill through the litellm proxy. The dump @@ -105,27 +119,27 @@ def _start_rest(backend: Neo4jBackend, port: int, started_at: float) -> None: def _ingest_in_background(backend: Neo4jBackend) -> None: - """Run the boot-time cypher ingest off the main thread. - - On a cold container the bundled ``skills.cypher`` is ~3.7 MB and - Neo4j MERGEs ~6000 statements over the bolt driver — that takes - several minutes on the first boot. Running it on the main thread - blocks ``uvicorn.run()`` from binding the listener until the - ingest finishes, which leaves ``/v1/health`` unreachable and - Docker's healthcheck flapping past ``start_period``. We move the - ingest to a daemon thread so the REST server comes up first and - the healthcheck passes immediately; the corpus then loads in the - background and the existing ``skill_count`` field in - ``/v1/health`` reports its progress. + """Run the first-boot seed off the main thread. + + On a cold container with an empty database the bundled + ``skills.cypher`` is ~3.7 MB and Neo4j MERGEs ~6000 statements over + the bolt driver — that takes several minutes. Running it on the main + thread blocks ``uvicorn.run()`` from binding the listener until the + seed finishes, which leaves ``/v1/health`` unreachable and Docker's + healthcheck flapping past ``start_period``. We move it to a daemon + thread so the REST server comes up first and the healthcheck passes + immediately; the corpus then loads in the background and the existing + ``skill_count`` field in ``/v1/health`` reports its progress. When the + graph is already populated the seed is a cheap no-op count check. """ try: - _maybe_ingest(backend) + _seed_if_empty(backend) except Exception as exc: # noqa: BLE001 - # Failing to ingest is loud but not fatal — the operator may + # Failing to seed is loud but not fatal — the operator may # be running against a Neo4j that was pre-loaded out of band # (a different cypher file, or a manual seed). REST stays up # so health probes can report the situation. - log.error("cypher ingest failed: %r — continuing without it", exc) + log.error("cypher seed failed: %r — continuing without it", exc) def main() -> int: diff --git a/packages/decepticon/decepticon/skillogy/embed_ingest.py b/packages/decepticon/decepticon/skillogy/embed_ingest.py index 871d3cb36..499e8c0c9 100644 --- a/packages/decepticon/decepticon/skillogy/embed_ingest.py +++ b/packages/decepticon/decepticon/skillogy/embed_ingest.py @@ -1,7 +1,7 @@ """Boot-time embedding backfill for skillogy hybrid retrieval (ADR-0011). Runs once per container boot, AFTER the ``skills.cypher`` dump has been loaded -(see ``__main__._maybe_ingest``). It creates the vector index and embeds every +(see ``__main__._seed_if_empty``). It creates the vector index and embeds every skill whose embedding-input text changed since the last boot — so the dump stays embedding-free (a model swap is a re-embed, not a rebuild) and an unchanged corpus is a no-op. diff --git a/packages/decepticon/tests/unit/skillogy/test_seed.py b/packages/decepticon/tests/unit/skillogy/test_seed.py new file mode 100644 index 000000000..dce53c9a4 --- /dev/null +++ b/packages/decepticon/tests/unit/skillogy/test_seed.py @@ -0,0 +1,44 @@ +"""Unit test for the boot seed guard (``__main__._seed_if_empty``). + +The skill graph is persistent, so the boot path must seed an empty +database exactly once and never re-run the corpus against a populated +one. These two cases are the whole contract. +""" + +from __future__ import annotations + +from decepticon.skillogy import __main__ as skillogy_main + + +class _FakeBackend: + def __init__(self, skill_count: int) -> None: + self._skill_count = skill_count + self.ingested: list[str] = [] + + def health(self) -> dict: + return {"status": "ok", "skill_count": self._skill_count} + + def bulk_ingest_cypher(self, cypher_text: str) -> int: + self.ingested.append(cypher_text) + return cypher_text.count(";") + + +def test_seed_skipped_when_graph_already_populated() -> None: + backend = _FakeBackend(skill_count=326) + skillogy_main._seed_if_empty(backend) # type: ignore[arg-type] + assert backend.ingested == [] # a populated graph is never re-seeded + + +def test_seed_runs_once_when_graph_empty(monkeypatch, tmp_path) -> None: + cypher = tmp_path / "skills.cypher" + cypher.write_text("MERGE (n:Skill {name: 'x'});\n", encoding="utf-8") + monkeypatch.setenv("SKILLOGY_CYPHER_PATH", str(cypher)) + # Stop before the (network-bound) embedding backfill — the seed itself + # is what this test covers. + import decepticon.skillogy.embed_ingest as embed + + monkeypatch.setattr(embed, "ingest_embeddings", lambda backend: None) + + backend = _FakeBackend(skill_count=0) + skillogy_main._seed_if_empty(backend) # type: ignore[arg-type] + assert len(backend.ingested) == 1 # empty graph → seeded exactly once