From 7ecab9d56645cbfa1849b47cd2421178bb7bc3cc Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 17:15:51 +0800 Subject: [PATCH 01/19] feat(spider2-dbt): AC-2 preflight validates source DuckDB, fails closed Port ade_bench preflight patterns into spider2_dbt/preflight.py: named Spider2WorkspacePreflightError, _read_duckdb_tables round-trip, family-agnostic dbt sources cross-check. No static family contracts. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../benchmarks/spider2_dbt/preflight.py | 200 ++++++++++++++++++ .../test_spider2_dbt_workspace_preflight.py | 173 +++++++++++++++ 2 files changed, 373 insertions(+) create mode 100644 src/razorback/benchmarks/spider2_dbt/preflight.py create mode 100644 tests/unit/test_spider2_dbt_workspace_preflight.py diff --git a/src/razorback/benchmarks/spider2_dbt/preflight.py b/src/razorback/benchmarks/spider2_dbt/preflight.py new file mode 100644 index 0000000..649a558 --- /dev/null +++ b/src/razorback/benchmarks/spider2_dbt/preflight.py @@ -0,0 +1,200 @@ +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from typing import Any + + +PREFLIGHT_LOG_PREFIX = "RAZORBACK_SPIDER2_PREFLIGHT" + + +class Spider2WorkspacePreflightError(RuntimeError): + def __init__(self, payload: dict[str, Any]) -> None: + self.payload = payload + super().__init__( + "spider2-dbt workspace preflight infrastructure failure: " + + json.dumps(payload, sort_keys=True) + ) + + +def preflight_script_text() -> str: + return Path(__file__).read_text() + + +def preflight_spider2_workspace( + *, + task_id: str, + workspace: Path, + db_name: str | None = None, + db_path: Path | None = None, +) -> dict[str, Any]: + """Validate the source DuckDB a spider2-dbt agent will operate against. + + spider2-dbt has no fixed task families, so this validates *structural* + properties — the file is present, openable as a DuckDB, declares at least + one user table, and (when the dbt project ships `sources:` metadata) + contains every declared source table. It fails closed with a named error + so a bad source DuckDB fails the image build rather than the agent run. + """ + workspace = Path(workspace) + resolved_db_path = _resolve_db_path( + workspace=workspace, db_name=db_name, db_path=db_path + ) + payload: dict[str, Any] = { + "status": "checking", + "task_id": task_id, + "db_name": db_name, + "db_path": str(resolved_db_path) if resolved_db_path is not None else None, + "observed_tables": [], + "required_tables": [], + "required_tables_source": None, + "missing_tables": [], + } + + if resolved_db_path is None or not resolved_db_path.is_file(): + payload["status"] = "failed" + payload["reason"] = "duckdb file missing" + raise Spider2WorkspacePreflightError(payload) + + try: + observed_tables = _read_duckdb_tables(resolved_db_path) + except Exception as exc: + payload["status"] = "failed" + payload["reason"] = "duckdb inspection failed" + payload["error"] = repr(exc) + raise Spider2WorkspacePreflightError(payload) from exc + + payload["observed_tables"] = sorted(observed_tables) + + required_tables = _read_dbt_source_tables(workspace) + if required_tables: + payload["required_tables"] = sorted(required_tables) + payload["required_tables_source"] = "dbt_source_metadata" + missing = sorted(required_tables - observed_tables) + payload["missing_tables"] = missing + if missing: + payload["status"] = "failed" + payload["reason"] = "required dbt source tables missing" + raise Spider2WorkspacePreflightError(payload) + elif not observed_tables: + payload["status"] = "failed" + payload["reason"] = "no user tables present" + raise Spider2WorkspacePreflightError(payload) + + payload["status"] = "passed" + return payload + + +def _resolve_db_path( + *, workspace: Path, db_name: str | None, db_path: Path | None +) -> Path | None: + if db_path is not None: + return Path(db_path) + if db_name: + return workspace / f"{db_name}.duckdb" + if workspace.is_dir(): + candidates = sorted(workspace.glob("*.duckdb")) + if candidates: + return candidates[0] + return None + + +def _read_dbt_source_tables(workspace: Path) -> set[str]: + """Read dbt `sources:` table names when the task ships source metadata.""" + try: + import yaml + except Exception: + return set() + + tables: set[str] = set() + for yaml_path in _iter_candidate_dbt_yaml_files(workspace): + try: + document = yaml.safe_load(yaml_path.read_text()) + except Exception: + continue + for source in _iter_dicts(_as_list(_as_dict(document).get("sources"))): + for table in _iter_dicts(_as_list(source.get("tables"))): + name = table.get("identifier") or table.get("name") + if isinstance(name, str) and name.strip(): + tables.add(name.strip().lower()) + return tables + + +def _iter_candidate_dbt_yaml_files(workspace: Path): + if not workspace.is_dir(): + return + excluded_parts = {".git", ".venv", "dbt_packages", "logs", "target"} + for pattern in ("*.yml", "*.yaml"): + for path in sorted(workspace.rglob(pattern)): + if excluded_parts & set(path.relative_to(workspace).parts): + continue + yield path + + +def _as_dict(value: Any) -> dict[str, Any]: + if isinstance(value, dict): + return value + return {} + + +def _as_list(value: Any) -> list[Any]: + if isinstance(value, list): + return value + return [] + + +def _iter_dicts(values: list[Any]): + for value in values: + if isinstance(value, dict): + yield value + + +def _read_duckdb_tables(db_path: Path) -> set[str]: + import duckdb + + conn = duckdb.connect(str(db_path), read_only=True) + try: + rows = conn.execute( + """ + SELECT DISTINCT table_name + FROM information_schema.tables + WHERE table_schema NOT IN ('information_schema', 'pg_catalog') + """ + ).fetchall() + finally: + conn.close() + return {str(row[0]).lower() for row in rows} + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Validate spider2-dbt DuckDB workspace data." + ) + parser.add_argument("--task-id", required=True) + parser.add_argument("--workspace", type=Path, default=Path("/app")) + parser.add_argument("--db-name") + parser.add_argument("--db-path", type=Path) + args = parser.parse_args(argv) + + try: + payload = preflight_spider2_workspace( + task_id=args.task_id, + workspace=args.workspace, + db_name=args.db_name, + db_path=args.db_path, + ) + except Spider2WorkspacePreflightError as exc: + print( + f"{PREFLIGHT_LOG_PREFIX} {json.dumps(exc.payload, sort_keys=True)}", + file=sys.stderr, + ) + return 2 + + print(f"{PREFLIGHT_LOG_PREFIX} {json.dumps(payload, sort_keys=True)}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/unit/test_spider2_dbt_workspace_preflight.py b/tests/unit/test_spider2_dbt_workspace_preflight.py new file mode 100644 index 0000000..73477aa --- /dev/null +++ b/tests/unit/test_spider2_dbt_workspace_preflight.py @@ -0,0 +1,173 @@ +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import duckdb +import pytest + +from razorback.benchmarks.spider2_dbt.preflight import ( + Spider2WorkspacePreflightError, + preflight_spider2_workspace, +) + + +def _write_duckdb(path: Path, tables: set[str]) -> None: + conn = duckdb.connect(str(path)) + try: + for table in sorted(tables): + conn.execute(f'CREATE TABLE "{table}" (id INTEGER)') + finally: + conn.close() + + +def test_present_readable_duckdb_passes(tmp_path: Path) -> None: + _write_duckdb(tmp_path / "spider2-fixture-001.duckdb", {"orders", "customers"}) + + result = preflight_spider2_workspace( + task_id="spider2-fixture-001", + workspace=tmp_path, + db_name="spider2-fixture-001", + ) + + assert result["status"] == "passed" + assert result["task_id"] == "spider2-fixture-001" + assert "orders" in result["observed_tables"] + assert "customers" in result["observed_tables"] + + +def test_present_duckdb_discovered_without_db_name(tmp_path: Path) -> None: + _write_duckdb(tmp_path / "anything.duckdb", {"t1"}) + + result = preflight_spider2_workspace(task_id="t", workspace=tmp_path) + + assert result["status"] == "passed" + assert result["db_path"].endswith("anything.duckdb") + + +def test_missing_duckdb_fails_closed_with_named_error(tmp_path: Path) -> None: + with pytest.raises(Spider2WorkspacePreflightError) as exc_info: + preflight_spider2_workspace( + task_id="spider2-fixture-001", + workspace=tmp_path, + db_name="spider2-fixture-001", + ) + + payload = exc_info.value.payload + assert payload["status"] == "failed" + assert payload["reason"] == "duckdb file missing" + assert "spider2-fixture-001.duckdb" in payload["db_path"] + + +def test_corrupt_duckdb_fails_closed(tmp_path: Path) -> None: + corrupt = tmp_path / "spider2-fixture-001.duckdb" + corrupt.write_bytes(b"this is not a valid duckdb file" * 4) + + with pytest.raises(Spider2WorkspacePreflightError) as exc_info: + preflight_spider2_workspace( + task_id="spider2-fixture-001", + workspace=tmp_path, + db_name="spider2-fixture-001", + ) + + payload = exc_info.value.payload + assert payload["status"] == "failed" + assert payload["reason"] == "duckdb inspection failed" + assert "error" in payload + + +def test_empty_duckdb_with_no_user_tables_fails(tmp_path: Path) -> None: + _write_duckdb(tmp_path / "empty.duckdb", set()) + + with pytest.raises(Spider2WorkspacePreflightError) as exc_info: + preflight_spider2_workspace(task_id="t", workspace=tmp_path) + + assert exc_info.value.payload["reason"] == "no user tables present" + + +def test_dbt_source_metadata_required_tables_enforced(tmp_path: Path) -> None: + models = tmp_path / "models" + models.mkdir() + (models / "sources.yml").write_text( + "\n".join( + [ + "version: 2", + "sources:", + " - name: canonical", + " tables:", + " - name: orders", + " identifier: raw_orders", + " - name: customers", + " identifier: raw_customers", + "", + ] + ) + ) + + # DuckDB missing raw_customers -> fails + _write_duckdb(tmp_path / "db.duckdb", {"raw_orders"}) + with pytest.raises(Spider2WorkspacePreflightError) as exc_info: + preflight_spider2_workspace(task_id="t", workspace=tmp_path) + assert "raw_customers" in exc_info.value.payload["missing_tables"] + + # DuckDB with both -> passes + (tmp_path / "db.duckdb").unlink() + _write_duckdb(tmp_path / "db.duckdb", {"raw_orders", "raw_customers"}) + result = preflight_spider2_workspace(task_id="t", workspace=tmp_path) + assert result["status"] == "passed" + assert result["required_tables_source"] == "dbt_source_metadata" + + +def test_preflight_cli_exits_nonzero_and_emits_json_payload(tmp_path: Path) -> None: + completed = subprocess.run( + [ + sys.executable, + "-m", + "razorback.benchmarks.spider2_dbt.preflight", + "--task-id", + "spider2-fixture-001", + "--workspace", + str(tmp_path), + "--db-name", + "spider2-fixture-001", + ], + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=False, + ) + + assert completed.returncode == 2 + assert "RAZORBACK_SPIDER2_PREFLIGHT" in completed.stderr + payload = json.loads( + completed.stderr.split("RAZORBACK_SPIDER2_PREFLIGHT ", 1)[1] + ) + assert payload["task_id"] == "spider2-fixture-001" + assert payload["reason"] == "duckdb file missing" + + +def test_preflight_cli_passes_on_present_readable_duckdb(tmp_path: Path) -> None: + _write_duckdb(tmp_path / "spider2-fixture-001.duckdb", {"orders"}) + + completed = subprocess.run( + [ + sys.executable, + "-m", + "razorback.benchmarks.spider2_dbt.preflight", + "--task-id", + "spider2-fixture-001", + "--workspace", + str(tmp_path), + "--db-name", + "spider2-fixture-001", + ], + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=False, + ) + + assert completed.returncode == 0 + assert "RAZORBACK_SPIDER2_PREFLIGHT" in completed.stdout From 6f655040df717677e58939796acd4c0da3d3ef8b Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 17:19:49 +0800 Subject: [PATCH 02/19] feat(spider2-dbt): AC-1 dbt-deps layer, AC-2 preflight wiring, AC-3 lock + RIDER build-context - AC-1: _ensure_dbt_deps_image_layer gated on dbt_project/packages.yml (the spider2 divergence from ade-bench's project/) - AC-2 image side: _ensure_workspace_preflight_image_layer copies the preflight script and RUNs it --workspace /app before CMD - AC-3: locking test asserts gold/expected/golden + solution deny-globs strip planted answer files from the materialized view - RIDER (Codex finding 2): _ensure_spider2_build_context_layer stages dbt_project/ (incl. source .duckdb) into the environment/ build context and COPYs it to /app BEFORE the preflight RUN. Proven at build-context level: a test parses the COPY src from the Dockerfile and asserts a real .duckdb is present under it, so --workspace /app cannot fail on a missing project. Layer order: COPY dbt_project -> /app; dbt deps; preflight; CMD. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../benchmarks/spider2_dbt/harbor_view.py | 170 +++++++++++- tests/unit/test_spider2_dbt_harbor_view.py | 250 ++++++++++++++++++ 2 files changed, 419 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_spider2_dbt_harbor_view.py diff --git a/src/razorback/benchmarks/spider2_dbt/harbor_view.py b/src/razorback/benchmarks/spider2_dbt/harbor_view.py index f199d99..035b131 100644 --- a/src/razorback/benchmarks/spider2_dbt/harbor_view.py +++ b/src/razorback/benchmarks/spider2_dbt/harbor_view.py @@ -1,8 +1,11 @@ from __future__ import annotations +import shlex +import shutil from pathlib import Path from typing import Literal +from razorback.benchmarks.spider2_dbt.preflight import preflight_script_text from razorback.harbor_tasks.leakage import DEFAULT_SOLUTION_DENY_GLOBS from razorback.harbor_tasks.materialize import materialize_harbor_task_view @@ -20,6 +23,26 @@ "**/golden/**", ) +# The dbt project root inside the running container. The r5 verifier +# (spider2-dbt-duckdb-match-verifier) and run-wiring entity read this same +# path; do not let later layers drift from it (Task 0 contract). +_APP_ROOT = "/app" + +# Source-side directory (inside the materialized view) holding the dbt +# project. spider2-dbt nests the dbt project under `dbt_project/` — the one +# structural divergence from ade-bench's `project/`. +_DBT_PROJECT_DIRNAME = "dbt_project" + +_BUILD_CONTEXT_MARKER = ( + "# Razorback: land spider2-dbt project + source DuckDB at /app before agent runtime." +) +_DBT_DEPS_LAYER_MARKER = ( + "# Razorback: install declared dbt packages before agent runtime." +) +_SPIDER2_WORKSPACE_PREFLIGHT_MARKER = ( + "# Razorback: validate spider2-dbt source DuckDB before agent runtime." +) + def materialize_spider2_harbor_task_view( *, @@ -29,7 +52,7 @@ def materialize_spider2_harbor_task_view( docker_image: str | None = None, view_mode: Literal["copy", "link"] = "copy", ) -> Path: - return materialize_harbor_task_view( + view = materialize_harbor_task_view( source_task_dir=source_task_dir, view_root=view_root, benchmark_kind="spider2-dbt", @@ -43,3 +66,148 @@ def materialize_spider2_harbor_task_view( exclude_globs=SPIDER2_DBT_DENY_GLOBS, view_mode=view_mode, ) + # RIDER (Codex finding 2): stage dbt_project/ (incl. the source .duckdb) + # into the build context and COPY it to /app BEFORE the preflight RUN, so + # the preflight `--workspace /app` can never fail on a missing project. + _ensure_spider2_build_context_layer(view) + _ensure_dbt_deps_image_layer(view) + _ensure_workspace_preflight_image_layer(view, task_slug=task_slug) + return view + + +def _has_dbt_project(view_dir: Path) -> bool: + """spider2-dbt nests the dbt project under `dbt_project/` (or under + `environment/dbt_project/`).""" + return ( + (view_dir / _DBT_PROJECT_DIRNAME).is_dir() + or (view_dir / "environment" / _DBT_PROJECT_DIRNAME).is_dir() + ) + + +def _has_dbt_packages_manifest(view_dir: Path) -> bool: + return ( + (view_dir / _DBT_PROJECT_DIRNAME / "packages.yml").is_file() + or ( + view_dir / "environment" / _DBT_PROJECT_DIRNAME / "packages.yml" + ).is_file() + ) + + +def _ensure_spider2_build_context_layer(view_dir: Path) -> None: + """Land the dbt project + source DuckDB at /app inside the image build. + + The Docker build context is the view's `environment/` directory, so the + dbt project (which the materializer reflects to `/dbt_project/`) + must be staged *inside* `environment/` for a COPY to reach it. This makes + the entity own the minimal COPY/context the preflight RUN depends on, + rather than assuming run-wiring already placed the project at /app. + """ + if not _has_dbt_project(view_dir): + return + + dockerfile = view_dir / "environment" / "Dockerfile" + if not dockerfile.is_file(): + return + + text = dockerfile.read_text() + if _BUILD_CONTEXT_MARKER in text: + return + + environment_dir = view_dir / "environment" + source_project = view_dir / _DBT_PROJECT_DIRNAME + staged_project = environment_dir / _DBT_PROJECT_DIRNAME + if source_project.is_dir() and not staged_project.exists(): + # Stage into the build context so the COPY source resolves. Copy + # (not move) so the view's own dbt_project/ remains intact for + # downstream consumers/tests. + shutil.copytree(source_project, staged_project) + + block = "\n".join( + [ + _BUILD_CONTEXT_MARKER, + f"COPY {_DBT_PROJECT_DIRNAME}/ {_APP_ROOT}/", + ] + ) + dockerfile.write_text(_insert_before_final_cmd(text, block)) + + +def _ensure_dbt_deps_image_layer(view_dir: Path) -> None: + """Install declared dbt packages during image build for dbt spider2 tasks.""" + if not _has_dbt_packages_manifest(view_dir): + return + + dockerfile = view_dir / "environment" / "Dockerfile" + if not dockerfile.is_file(): + return + + text = dockerfile.read_text() + if _DBT_DEPS_LAYER_MARKER in text: + return + + block = "\n".join( + [ + _DBT_DEPS_LAYER_MARKER, + "RUN if [ -f /app/packages.yml ]; then cd /app && dbt deps; fi", + ] + ) + dockerfile.write_text(_insert_before_final_cmd(text, block)) + + +def _ensure_workspace_preflight_image_layer( + view_dir: Path, *, task_slug: str +) -> None: + """Validate the source DuckDB at build time, before the agent runs. + + Gated on `_has_dbt_project`: spider2-dbt has no task families, so the + preflight is injected whenever the task is a dbt project. By the time this + RUN executes, the build-context layer has already COPY'd dbt_project/ (and + its .duckdb) to /app, so `--workspace /app` cannot fail on a missing + project. + """ + if not _has_dbt_project(view_dir): + return + + environment_dir = view_dir / "environment" + dockerfile = environment_dir / "Dockerfile" + if not dockerfile.is_file(): + return + + script_path = environment_dir / "razorback_spider2_preflight.py" + script_path.write_text(preflight_script_text()) + + text = dockerfile.read_text() + if _SPIDER2_WORKSPACE_PREFLIGHT_MARKER in text: + return + + command = " ".join( + [ + "python", + "/tmp/razorback_spider2_preflight.py", + "--task-id", + shlex.quote(task_slug), + "--workspace", + _APP_ROOT, + ] + ) + block = "\n".join( + [ + _SPIDER2_WORKSPACE_PREFLIGHT_MARKER, + "COPY razorback_spider2_preflight.py /tmp/razorback_spider2_preflight.py", + f"RUN {command}", + ] + ) + dockerfile.write_text(_insert_before_final_cmd(text, block)) + + +def _insert_before_final_cmd(text: str, block: str) -> str: + lines = text.rstrip().splitlines() + insert_at = None + for idx, line in enumerate(lines): + if line.lstrip().startswith("CMD "): + insert_at = idx + block_lines = ["", *block.splitlines()] + if insert_at is None: + lines.extend(block_lines) + else: + lines[insert_at:insert_at] = block_lines + return "\n".join(lines) + "\n" diff --git a/tests/unit/test_spider2_dbt_harbor_view.py b/tests/unit/test_spider2_dbt_harbor_view.py new file mode 100644 index 0000000..c696cac --- /dev/null +++ b/tests/unit/test_spider2_dbt_harbor_view.py @@ -0,0 +1,250 @@ +from __future__ import annotations + +from pathlib import Path + +from razorback.benchmarks.spider2_dbt.harbor_view import ( + SPIDER2_DBT_DENY_GLOBS, + materialize_spider2_harbor_task_view, +) +from razorback.harbor_tasks.leakage import DEFAULT_SOLUTION_DENY_GLOBS + + +_TASK_TOML = "\n".join( + [ + 'schema_version = "1.0"', + "[environment]", + 'os = "linux"', + "cpus = 1", + "memory_mb = 1024", + "storage_mb = 1024", + "", + ] +) + + +def _write_source( + source: Path, + *, + with_packages: bool, + with_duckdb: bool, + dockerfile_lines: list[str] | None = None, +) -> Path: + (source / "environment").mkdir(parents=True) + (source / "dbt_project" / "models").mkdir(parents=True) + (source / "task.toml").write_text(_TASK_TOML) + (source / "instruction.md").write_text("Fix the dbt project.\n") + (source / "dbt_project" / "dbt_project.yml").write_text( + "name: example\nprofile: example\n" + ) + (source / "dbt_project" / "models" / "example.sql").write_text("select 1\n") + if with_packages: + (source / "dbt_project" / "packages.yml").write_text( + "packages:\n - package: dbt-labs/dbt_utils\n version: 1.3.2\n" + ) + if with_duckdb: + import duckdb + + db = source / "dbt_project" / "spider2-fixture-001.duckdb" + conn = duckdb.connect(str(db)) + try: + conn.execute("CREATE TABLE orders (id INTEGER)") + finally: + conn.close() + lines = dockerfile_lines or [ + "FROM python:3.12", + "WORKDIR /app", + 'CMD ["bash"]', + "", + ] + (source / "environment" / "Dockerfile").write_text("\n".join(lines)) + return source + + +# --- AC-1: dbt-deps image layer ------------------------------------------- + + +def test_spider2_view_installs_dbt_packages_when_packages_yml_present(tmp_path): + source = _write_source( + tmp_path / "source", with_packages=True, with_duckdb=True + ) + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + ) + + dockerfile = (view / "environment" / "Dockerfile").read_text() + assert ( + "Razorback: install declared dbt packages before agent runtime." + in dockerfile + ) + assert ( + "RUN if [ -f /app/packages.yml ]; then cd /app && dbt deps; fi" + in dockerfile + ) + assert dockerfile.index("dbt deps") < dockerfile.index('CMD ["bash"]') + + +def test_spider2_view_omits_dbt_deps_layer_when_no_packages_yml(tmp_path): + source = _write_source( + tmp_path / "source", with_packages=False, with_duckdb=True + ) + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + ) + + dockerfile = (view / "environment" / "Dockerfile").read_text() + assert "install declared dbt packages" not in dockerfile + assert "dbt deps" not in dockerfile + + +# --- AC-2 (image side): preflight build layer ------------------------------ + + +def test_spider2_view_injects_workspace_preflight_before_cmd(tmp_path): + source = _write_source( + tmp_path / "source", with_packages=True, with_duckdb=True + ) + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + ) + + preflight_script = view / "environment" / "razorback_spider2_preflight.py" + assert preflight_script.is_file() + assert "def preflight_spider2_workspace" in preflight_script.read_text() + + dockerfile = (view / "environment" / "Dockerfile").read_text() + assert ( + "Razorback: validate spider2-dbt source DuckDB before agent runtime." + in dockerfile + ) + assert ( + "COPY razorback_spider2_preflight.py /tmp/razorback_spider2_preflight.py" + in dockerfile + ) + assert "--task-id spider2-fixture-001" in dockerfile + assert "--workspace /app" in dockerfile + assert dockerfile.index("razorback_spider2_preflight.py") < dockerfile.index( + 'CMD ["bash"]' + ) + + +# --- RIDER (Codex finding 2, mandatory) ------------------------------------ +# The preflight RUN must NOT be able to fail on a missing dbt project: this +# entity owns landing dbt_project/ + the source .duckdb at /app BEFORE the +# preflight RUN, proven at BUILD-CONTEXT level (the files the COPY references +# are actually present in the environment/ build context), not by text +# inspection alone. + + +def test_preflight_build_context_holds_duckdb_before_preflight_run(tmp_path): + source = _write_source( + tmp_path / "source", with_packages=True, with_duckdb=True + ) + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + ) + + environment = view / "environment" + dockerfile_text = (environment / "Dockerfile").read_text() + lines = dockerfile_text.splitlines() + + # Find the COPY that lands the dbt project (incl. the .duckdb) at /app and + # the preflight RUN. The COPY must precede the preflight RUN. + copy_app_idx = next( + i + for i, ln in enumerate(lines) + if ln.strip().startswith("COPY ") and ln.strip().rstrip().endswith("/app/") + ) + preflight_run_idx = next( + i + for i, ln in enumerate(lines) + if "razorback_spider2_preflight.py" in ln and ln.strip().startswith("RUN ") + ) + assert copy_app_idx < preflight_run_idx + + # BUILD-CONTEXT proof: the path the /app COPY references must exist as a + # real entry inside the environment/ build context, and it must contain + # the source .duckdb. Parse the COPY source path from the Dockerfile and + # resolve it against the build context root (environment/). + copy_line = lines[copy_app_idx].strip() + # form: COPY /app/ + parts = copy_line.split() + assert parts[0] == "COPY" + assert parts[-1] == "/app/" + copy_src = parts[1] + staged = environment / copy_src + assert staged.exists(), f"build context missing COPY source: {copy_src}" + duckdbs = list(staged.rglob("*.duckdb")) + assert duckdbs, ( + "no .duckdb staged into the build context under " + f"{copy_src}; preflight RUN --workspace /app could fail on a " + "missing DuckDB" + ) + + +def test_preflight_layer_absent_when_not_a_dbt_project(tmp_path): + # A non-dbt source (no dbt_project/) gets no preflight layer at all, so the + # preflight RUN can never run against an empty /app. + source = tmp_path / "source" + (source / "environment").mkdir(parents=True) + (source / "task.toml").write_text(_TASK_TOML) + (source / "environment" / "Dockerfile").write_text( + "FROM python:3.12\nCMD [\"bash\"]\n" + ) + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="plain-001", + ) + + dockerfile = (view / "environment" / "Dockerfile").read_text() + assert "razorback_spider2_preflight.py" not in dockerfile + assert not (view / "environment" / "razorback_spider2_preflight.py").exists() + + +# --- AC-3: deny-glob lock -------------------------------------------------- + + +def test_spider2_view_excludes_gold_solution_expected_paths(tmp_path): + source = _write_source( + tmp_path / "source", with_packages=False, with_duckdb=False + ) + (source / "gold").mkdir() + (source / "gold" / "answer.sql").write_text("select 'gold';\n") + (source / "golden").mkdir() + (source / "golden" / "result.txt").write_text("golden output\n") + (source / "tests" / "expected").mkdir(parents=True) + (source / "tests" / "expected" / "expected.csv").write_text("id\n1\n") + (source / "expected").mkdir() + (source / "expected" / "answer.txt").write_text("answer\n") + (source / "solution").mkdir() + (source / "solution" / "solve.sh").write_text("#!/bin/bash\n") + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + ) + + assert not (view / "gold" / "answer.sql").exists() + assert not (view / "golden" / "result.txt").exists() + assert not (view / "tests" / "expected" / "expected.csv").exists() + assert not (view / "expected" / "answer.txt").exists() + assert not (view / "solution" / "solve.sh").exists() + + +def test_spider2_deny_globs_cover_required_families(): + assert {"gold/**", "expected/**", "golden/**"} <= set(SPIDER2_DBT_DENY_GLOBS) + assert set(DEFAULT_SOLUTION_DENY_GLOBS) <= set(SPIDER2_DBT_DENY_GLOBS) From 1630ffeee7b84b9c1e7ea061aaf70fd5d5ded3d2 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 17:20:22 +0800 Subject: [PATCH 03/19] docs(spider2-dbt): implementation stage report Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 1a388a7..8bea23b 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -72,3 +72,26 @@ authored tag and leave the digest null when unresolved (per PKG-40). ### Summary Produced a separate plan doc (standard flow, per the FO dispatch) mapping AC-1/AC-2/AC-3 to four code tasks plus a Task-0 written contract. The riskiest surface — the `/app` + `/app/.duckdb` image/workdir contract the r5 verifier depends on — is pinned first as prose; the riskiest mechanism (AC-2 preflight's real DuckDB open / fail-closed) is built first with a real `duckdb.connect` round-trip test. All ade_bench reference cites were verified against the source files. AC-3 is noted as mostly already satisfied (deny-globs present at `spider2_dbt/harbor_view.py:10-21`), so its task is a locking test. Key spider2 divergence flagged: `dbt_project/` layout vs ade-bench `project/`. + +## Implementation summary + +Modules added/touched (all under `src/razorback/benchmarks/spider2_dbt/`; generic `harbor_tasks/materialize.py` + `harbor_tasks/leakage.py` byte-for-byte unchanged): +- new `preflight.py` — `Spider2WorkspacePreflightError`, `preflight_spider2_workspace`, `preflight_script_text`, `main` CLI (`RAZORBACK_SPIDER2_PREFLIGHT`). +- `harbor_view.py` — added `_ensure_spider2_build_context_layer` (RIDER), `_ensure_dbt_deps_image_layer`, `_ensure_workspace_preflight_image_layer`, `_has_dbt_project`, `_has_dbt_packages_manifest`, `_insert_before_final_cmd`; wired into `materialize_spider2_harbor_task_view`. + +Harbor surfaces touched: the spider2 view's `environment/Dockerfile` now gains (in order before the final `CMD`) a build-context COPY landing `dbt_project/` at `/app`, the dbt-deps RUN, and the preflight COPY+RUN. + +## Stage Report: implementation + +- DONE: Implement the approved plan TDD-first so all 3 ACs pass: new `spider2_dbt/preflight.py`; `_ensure_dbt_deps_image_layer` adding a dbt-deps layer when `dbt_project/packages.yml` is present; deny-glob regression lock. `uv run pytest` green. + Red→green per task; `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` → 28 passed. Commits: preflight + tests, then harbor_view + tests. +- DONE: RIDER (Codex finding 2): build-time preflight `RUN ... --workspace /app` must NOT fail on a missing dbt project; this entity owns the COPY/context landing `dbt_project/` + source `.duckdb` at `/app` BEFORE the preflight RUN; verify at build-context level. + `_ensure_spider2_build_context_layer` `shutil.copytree`s `dbt_project/` (incl. its `.duckdb`) into the `environment/` build context and emits `COPY dbt_project/ /app/` before the preflight RUN. `test_preflight_build_context_holds_duckdb_before_preflight_run` parses the COPY src from the Dockerfile and asserts a real `*.duckdb` exists under it in the build context — build-context proof, not text inspection. +- DONE: Pin the image/workdir contract (`/app` dbt root, agent DB at `/app/.duckdb`, preflight at `/tmp/...`) as the stable r5-facing invariant; keep generic materializer + non-spider2 harbor behavior byte-for-byte unchanged. + `_APP_ROOT="/app"` and preflight at `/tmp/razorback_spider2_preflight.py` constant-pinned in `harbor_view.py`. `materialize.py`/`leakage.py` untouched (git diff empty); ade_bench + translate regression suites 25 passed. +- SKIPPED: verifier-time `test-setup.sh` dbt-deps reuse helper (`_ensure_dbt_deps_test_setup_uses_preinstalled_packages`). + Plan Task-2 note: r5 verifier out of scope; spider2 fixture has `tests/test.sh` not `tests/test-setup.sh`. Deferred to r5. + +### Summary + +Ported the three ade_bench harness patterns into a new `spider2_dbt/preflight.py` (real DuckDB round-trip, fails closed with a named error) and three Dockerfile-layer helpers in `harbor_view.py`, all TDD-first. The mandatory RIDER is satisfied structurally: the entity now stages `dbt_project/` (carrying the source `.duckdb`) into the `environment/` build context and COPYs it to `/app` before the preflight RUN, proven by a build-context-level test that resolves the COPY source and asserts a real `.duckdb` is present. The one structural divergence — `dbt_project/` (not ade-bench `project/`) for the packages-manifest and build-context lookups — is implemented and tested. Note: `uv run pytest -k spider2_dbt` collection trips a PRE-EXISTING unrelated broken module (`tests/unit/test_task_identity_scoring.py` imports the nonexistent `razorback.score.load`, present verbatim in base commit `996d42b`); excluding that module the spider2 acceptance is 28 passed and ade_bench/translate regression is 25 passed. From a61711cadb420b2fe37075478cf4a8c593aa4a50 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 17:31:37 +0800 Subject: [PATCH 04/19] =?UTF-8?q?validation(spider2-dbt):=20REJECTED=20?= =?UTF-8?q?=E2=80=94=20link-mode=20Dockerfile=20write-through=20corrupts?= =?UTF-8?q?=20source=20fixtures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ACs 1-3 + RIDER verified from a clean checkout (28 passed; build-context proof; /app contract pinned; generic materializer unchanged; 72-passed non-spider2 regression; pre-existing score.load error confirmed not a regression). Code review + independent repro found a Critical defect: the three Dockerfile helpers write through the link-mode symlinked Dockerfile and corrupt the committed source fixtures (and the user's source Dockerfile in production). Gate: REJECTED -> implementation. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 15 ++ .../spider2-dbt-harbor-view-ade-parity.md | 176 ++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 8bea23b..192336f 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -95,3 +95,18 @@ Harbor surfaces touched: the spider2 view's `environment/Dockerfile` now gains ( ### Summary Ported the three ade_bench harness patterns into a new `spider2_dbt/preflight.py` (real DuckDB round-trip, fails closed with a named error) and three Dockerfile-layer helpers in `harbor_view.py`, all TDD-first. The mandatory RIDER is satisfied structurally: the entity now stages `dbt_project/` (carrying the source `.duckdb`) into the `environment/` build context and COPYs it to `/app` before the preflight RUN, proven by a build-context-level test that resolves the COPY source and asserts a real `.duckdb` is present. The one structural divergence — `dbt_project/` (not ade-bench `project/`) for the packages-manifest and build-context lookups — is implemented and tested. Note: `uv run pytest -k spider2_dbt` collection trips a PRE-EXISTING unrelated broken module (`tests/unit/test_task_identity_scoring.py` imports the nonexistent `razorback.score.load`, present verbatim in base commit `996d42b`); excluding that module the spider2 acceptance is 28 passed and ade_bench/translate regression is 25 passed. + +## Stage Report: validation + +- DONE: Independently reproduce the 3 ACs from a clean worktree checkout (AC-2 preflight fail-closed on missing/corrupt/empty/no-sources + pass; AC-1 dbt-deps layer present iff `dbt_project/packages.yml`; AC-3 deny-glob lock). Run `uv run pytest -k spider2_dbt` (excluding pre-existing-broken `test_task_identity_scoring.py`). + Fresh `git clone --single-branch` to /tmp; `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` → 28 passed. AC-2 8/8 with every named `reason`; AC-1/AC-3 7/7. Detail in validation/spider2-dbt-harbor-view-ade-parity.md. +- DONE: Scrutinize the mandatory RIDER (build-context COPY before preflight RUN; real `*.duckdb` under the COPY source at build-context level; `/app` + `/app/.duckdb` contract pinned for r5). + Exercised the materializer directly: COPY `dbt_project/`→`/app/` (idx 4) precedes preflight RUN (idx 11); COPY source resolves under `environment/` and holds a real `demo.duckdb`; `_APP_ROOT="/app"` + `/tmp/razorback_spider2_preflight.py` pinned. Build-context proof, not text. +- DONE: Run superpowers:requesting-code-review; confirm generic `materialize.py`/`leakage.py` + non-spider2 harbor behavior unchanged; confirm the only failing item is the pre-existing `razorback.score.load` collection error (base `996d42b`), not a regression. Give a gate verdict. + Generic surfaces byte-for-byte unchanged (empty diff 996d42b..HEAD); ade_bench+translate regression 72 passed/1 skipped; broken module confirmed pre-existing on base. Code review found a Critical defect (B1). Verdict: REJECTED → implementation. +- FAILED: Gate PASSED. + B1 (Critical, reproduced from clean checkout, NEW in this entity): the three Dockerfile-writing helpers in `harbor_view.py` write through the link-mode symlinked `environment/Dockerfile`, corrupting the version-controlled source fixtures (`spider2-fixture-00{1,2}/environment/Dockerfile`). Default production path (translate bind→link) rewrites the user's source Dockerfile and can leak the idempotency marker, suppressing layer injection on later runs. Fix: unlink-then-write guard mirroring `materialize.py:140-146`. + +### Summary + +Verified all 3 ACs and the mandatory RIDER from a clean clone of the worktree branch: `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` → 28 passed; the RIDER's COPY-before-preflight ordering and real-`.duckdb`-in-build-context were confirmed by independently exercising the materializer; the `/app` contract is pinned; generic `materialize.py`/`leakage.py` are unchanged and non-spider2 regression is 72 passed; the `razorback.score.load` collection error is pre-existing on base `996d42b`, not a regression. However the code review surfaced — and I independently reproduced — a Critical defect: in the default link materialize mode the new Dockerfile helpers write through the symlinked Dockerfile and corrupt the committed source fixtures (and in production rewrite the user's source task Dockerfile, risking marker-leak that skips layer injection). The generic materializer already guards this exact hazard for `task.toml`. **Gate: REJECTED → implementation** (fix B1 + restore corrupted fixtures + re-run for a clean tree). The corrupted fixtures are intentionally left dirty in the worktree as the reproduction artifact. diff --git a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md new file mode 100644 index 0000000..dee51d6 --- /dev/null +++ b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md @@ -0,0 +1,176 @@ +# Validation: spider2-dbt — harbor_view dbt+DuckDB parity with ade-bench + +**Entity:** `spider2-dbt-harbor-view-ade-parity` +**Branch:** `spacedock-ensign/spider2-dbt-harbor-view-ade-parity` +**Range reviewed:** `996d42b` (base / merge-base with main) .. `185c89d` (HEAD) +**Method:** independent reproduction from a fresh `git clone --single-branch` of the worktree branch into `/tmp/spider2-validation-clean` (no production code written). + +**Gate verdict: REJECTED → back to `implementation`.** + +The 3 ACs and the RIDER are functionally implemented and well-tested, but +running the acceptance command corrupts version-controlled fixture files in +the default production materialize path. This is a Critical, blocking defect +with a narrow root cause and a known fix pattern already present in the +generic materializer. + +--- + +## Acceptance command + +`uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` +→ **28 passed, 751 deselected** (clean checkout, `EXIT 0`). + +The `--ignore` is required: bare `uv run pytest -k spider2_dbt` trips a +collection error in `tests/unit/test_task_identity_scoring.py` +(`ModuleNotFoundError: No module named 'razorback.score.load'`). Confirmed +**pre-existing on base `996d42b`** (`git show 996d42b:tests/unit/test_task_identity_scoring.py` +line 5 imports `razorback.score.load`; `src/razorback/score/load.py` does not +exist at HEAD). **Not a regression.** + +--- + +## AC results + +### AC-1 — dbt-deps image layer when `packages.yml` present — PASS +`uv run pytest tests/unit/test_spider2_dbt_harbor_view.py -v` → +- `test_spider2_view_installs_dbt_packages_when_packages_yml_present` PASS: + `RUN if [ -f /app/packages.yml ]; then cd /app && dbt deps; fi` present, before `CMD`. +- `test_spider2_view_omits_dbt_deps_layer_when_no_packages_yml` PASS: + no `dbt deps` when manifest absent. + +### AC-2 — preflight validates source DuckDB, fails closed — PASS +`uv run pytest tests/unit/test_spider2_dbt_workspace_preflight.py -v` → **8 passed.** +Reproduced every named failure mode with real DuckDB round-trips: +- missing → `reason: "duckdb file missing"` +- corrupt → `reason: "duckdb inspection failed"` (+ `error`) +- empty / no user tables → `reason: "no user tables present"` +- declared dbt `sources:` table missing → `reason: "required dbt source tables missing"` +- valid DuckDB → `status: "passed"`; CLI exits `2` on failure with JSON payload, `0` on pass. +Named error type: `Spider2WorkspacePreflightError` (preflight.py:13). + +### AC-3 — view excludes gold/golden/expected/solution — PASS +- `test_spider2_view_excludes_gold_solution_expected_paths` PASS: planted + `gold/`, `golden/`, `tests/expected/`, `expected/`, `solution/` files do not + survive into the materialized view. +- `test_spider2_deny_globs_cover_required_families` PASS: + `{gold/**, expected/**, golden/**} ⊆ SPIDER2_DBT_DENY_GLOBS` and + `DEFAULT_SOLUTION_DENY_GLOBS ⊆ SPIDER2_DBT_DENY_GLOBS`. Both top-level + (`gold/**`) and nested (`**/gold/**`) forms present (harbor_view.py:13-24). + +### RIDER (mandatory) — build-context preflight ordering — PASS (functionally) +Independently exercised `materialize_spider2_harbor_task_view` (copy mode) and +inspected the on-disk build context: +- Layer order in the emitted Dockerfile: `COPY dbt_project/ /app/` (idx 4) → + dbt-deps RUN (idx 7) → preflight `COPY`+`RUN` (idx 11) → `CMD` last. **COPY-to-`/app` + precedes the preflight RUN.** +- **Build-context proof (not text inspection):** the COPY source `dbt_project/` + resolves under `environment/` and contains a real `demo.duckdb` + (`staged.rglob("*.duckdb")` non-empty). So `--workspace /app` cannot fail on a + missing project/DB. +- Contract pinned for r5: `_APP_ROOT = "/app"` (single constant, harbor_view.py:29), + preflight at `/tmp/razorback_spider2_preflight.py`, `--workspace /app`. + Explicit and stable. + +--- + +## Regression sweep + +- `uv run pytest -k "ade_bench or translate" --ignore=tests/unit/test_task_identity_scoring.py` + → **72 passed, 1 skipped, 706 deselected.** No regressions in ade_bench or + non-spider2 translate/harbor behavior. +- Generic surfaces **byte-for-byte unchanged** vs base: + `git diff 996d42b..HEAD --stat src/razorback/harbor_tasks/materialize.py src/razorback/harbor_tasks/leakage.py` + → empty. + +--- + +## Code review findings + +Dispatched `superpowers:requesting-code-review` (general-purpose reviewer, +read-only on the worktree). Findings classified: + +### BLOCKING (Critical) + +**B1 — link-mode symlink write-through corrupts version-controlled fixtures.** +`src/razorback/benchmarks/spider2_dbt/harbor_view.py` — the three Dockerfile +helpers (`_ensure_spider2_build_context_layer` :112/131, +`_ensure_dbt_deps_image_layer` :143/153, +`_ensure_workspace_preflight_image_layer` :178/199) do +`dockerfile.read_text()` / `dockerfile.write_text(...)` on +`view_dir/"environment"/"Dockerfile"`. In the **default production path**, +`translate.py:376` maps `materialize_mode="bind"` → `view_mode="link"`, so that +Dockerfile is a **symlink back into the source task tree**; `write_text` +follows the link and overwrites the source file. + +- Independently reproduced from a clean checkout: running + `tests/unit/test_translate_spider2_dbt.py` (12 passed) OR + `tests/integration/test_rk_run_spider2_dbt_explain.py` (1 passed) leaves + `tests/fixtures/spider2_dbt/harbor_task_minimal/spider2-fixture-00{1,2}/environment/Dockerfile` + modified in git (the injected `COPY dbt_project/ /app/` + preflight block is + written into the **committed source** Dockerfile, which is a 1-line + `FROM python:3.12` at HEAD). +- Proven NEW in this entity: base `996d42b` `harbor_view.py` has no Dockerfile + writes; swapping the base file in and re-running the same test leaves fixtures + CLEAN (12 passed), while the HEAD file dirties them. +- Production severity (reviewer's independent escalation): beyond a dirty tree, + this (a) rewrites the user's real source task Dockerfile on disk on every + materialize, and (b) leaks the idempotency marker into the source — so a later + `copy`-mode materialize sees the marker already present and **skips layer + injection**, producing a view without the dbt-deps/preflight layers. +- The generic materializer already guards this exact hazard for `task.toml` + (`materialize.py:140-146`: unlink the symlink, then write a real view-owned + file). The new spider2 helpers do not replicate it. +- **Fix:** before each `write_text`, if the Dockerfile is a symlink, read its + contents, `unlink()`, then write a real file — a small shared helper + (`_materialize_real_file`) called at the top of each of the three helpers. + Scope is narrow: only the `Dockerfile` is symlinked; the preflight-script + `write_text` (:176) and the `shutil.copytree` (:123) land in real view-owned + dirs and are safe (copytree is additionally guarded by `not staged.exists()`). + +**Live evidence:** the worktree currently carries the corrupted fixtures +(`git status`: modified `spider2-fixture-00{1,2}/environment/Dockerfile`) from a +prior test run — left in place intentionally as the reproduction artifact for +the fix. (The `uv.lock` modification is unrelated: `uv sync`/`uv run` drops the +`exclude-newer` pin; benign, not this entity's work.) + +### NON-BLOCKING (Important / Minor — fold into B1's fix or note for follow-up) + +- **N1 (Important):** the six translate/integration tests pass the shared + `FIXTURE_ROOT` as source under the default link mode and are the corruption + vector; even after B1 they should adopt the isolated-`copytree` pattern that + `test_spider2_dbt_harbor_view.py::_materialize_spider2` already uses. Add a CI + guard that fails if the working tree is dirty after the suite — would have + caught this automatically. Recommend a regression test: after a `link`-mode + materialize, the **source** Dockerfile is unchanged and the **view** Dockerfile + is a real file carrying the layers. +- **N2 (Minor):** `dbt deps` RUN (harbor_view.py:150) assumes `dbt` is on PATH at + that build step; document the assumption or gate on `command -v dbt`. +- **N3 (Minor):** injected preflight RUN passes no `--db-name`, so it relies on + `glob("*.duckdb")[0]` rather than the pinned `.duckdb` — fine at build + time (only the COPY'd source is present) but softer than the "pinned contract" + comment claims. +- **N4 (Minor):** if `pyyaml` is absent in-container, dbt source-table + enforcement silently degrades (preflight.py:108-109); worth a comment. + +--- + +## Gate decision + +**REJECTED → `implementation`.** Concrete required fix: + +1. **(B1, blocking)** Guard the three Dockerfile-writing helpers in + `harbor_view.py` against link-mode symlink write-through: replace a symlinked + `environment/Dockerfile` with a real, view-owned file before patching + (mirror `materialize.py:140-146`). Add a regression test proving a `link`-mode + materialize leaves the source Dockerfile byte-for-byte unchanged and the view + Dockerfile a real file with the injected layers. +2. Restore the corrupted committed fixtures + (`spider2-fixture-00{1,2}/environment/Dockerfile` back to `FROM python:3.12`) + and re-confirm `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` + leaves the working tree clean. +3. (Recommended, N1) make the translate/integration tests hermetic and add the + dirty-tree CI guard. + +Everything else — all 3 ACs, the RIDER build-context proof, the pinned `/app` +contract, unchanged generic materializer/leakage, no non-spider2 regressions — +is verified and sound. Re-run this validation after B1 lands. From 93b2c939aca176478c0c1c5456bf77cf4445a213 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 17:33:01 +0800 Subject: [PATCH 05/19] =?UTF-8?q?feedback:=20validation=20gate=20rejected?= =?UTF-8?q?=20(cycle=201)=20=E2=80=94=20Dockerfile=20symlink-write-through?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../spider2-dbt-harbor-view-ade-parity.md | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 192336f..a0877b8 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -110,3 +110,27 @@ Ported the three ade_bench harness patterns into a new `spider2_dbt/preflight.py ### Summary Verified all 3 ACs and the mandatory RIDER from a clean clone of the worktree branch: `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` → 28 passed; the RIDER's COPY-before-preflight ordering and real-`.duckdb`-in-build-context were confirmed by independently exercising the materializer; the `/app` contract is pinned; generic `materialize.py`/`leakage.py` are unchanged and non-spider2 regression is 72 passed; the `razorback.score.load` collection error is pre-existing on base `996d42b`, not a regression. However the code review surfaced — and I independently reproduced — a Critical defect: in the default link materialize mode the new Dockerfile helpers write through the symlinked Dockerfile and corrupt the committed source fixtures (and in production rewrite the user's source task Dockerfile, risking marker-leak that skips layer injection). The generic materializer already guards this exact hazard for `task.toml`. **Gate: REJECTED → implementation** (fix B1 + restore corrupted fixtures + re-run for a clean tree). The corrupted fixtures are intentionally left dirty in the worktree as the reproduction artifact. + +## Feedback Cycles + +### Cycle 1 — validation gate REJECTED (2026-06-18) + +Validation (our own reviewer, no Codex needed) found a Critical defect. +Routing back to `implementation`: + +1. **[Critical] B1 — Dockerfile-writing helpers corrupt the source fixture under link mode.** + The three new helpers in `harbor_view.py` (`_ensure_spider2_build_context_layer`, + `_ensure_dbt_deps_image_layer`, `_ensure_workspace_preflight_image_layer`) + call `dockerfile.write_text(...)` on `environment/Dockerfile`. In the default + production path `translate.py:376` maps `bind`->`view_mode="link"`, so that + Dockerfile is a SYMLINK back into the source tree — the write follows the link + and mutates the version-controlled `spider2-fixture-00{1,2}/environment/Dockerfile` + (and can leak the idempotency marker, suppressing layer injection on later runs). + This is the same symlink-write-through class fixed for `task.toml` at + `materialize.py:140-146`. + **Fix:** apply the unlink-then-write pattern (unlink the symlink before + `write_text`, so the view owns a real file) in ALL three helpers; restore the + two corrupted fixture Dockerfiles to their committed content; add a test + proving link mode never mutates the source Dockerfile (mirror + `test_link_mode_symlinks_files_but_never_mutates_source_task_toml`). Keep AC-1/2/3 + + the build-context rider green. From 12de5389ac8eb11957aaf7307d08ee31e57f4d2e Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 17:36:47 +0800 Subject: [PATCH 06/19] fix(spider2-dbt): unlink Dockerfile symlink before layer-injection write Under view_mode="link" the reflected environment/Dockerfile is a symlink into the source tree; the three layer-injection helpers' write_text calls followed the link and corrupted the version-controlled fixtures. Guard all three with the existing unlink-then-write pattern (materialize.py:140-146) so the view owns a real file. Add a regression test proving link mode never mutates the source Dockerfile. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 15 +++++++ .../benchmarks/spider2_dbt/harbor_view.py | 18 ++++++++ tests/unit/test_spider2_dbt_harbor_view.py | 41 +++++++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index a0877b8..0cca5a4 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -134,3 +134,18 @@ Routing back to `implementation`: proving link mode never mutates the source Dockerfile (mirror `test_link_mode_symlinks_files_but_never_mutates_source_task_toml`). Keep AC-1/2/3 + the build-context rider green. + +## Stage Report: implementation (cycle 2) + +- DONE: Apply the unlink-then-write pattern to ALL THREE Dockerfile-writing helpers in `harbor_view.py` + Added `if dockerfile.is_symlink(): dockerfile.unlink()` before each `dockerfile.write_text(...)` in `_ensure_spider2_build_context_layer`, `_ensure_dbt_deps_image_layer`, `_ensure_workspace_preflight_image_layer`, mirroring `materialize.py:140-146`. +- DONE: Restore the two corrupted source fixture Dockerfiles and confirm a clean `git status` + `git checkout` on `spider2-fixture-00{1,2}/environment/Dockerfile`; `git diff tests/fixtures/` is now empty. +- DONE: Add a regression test proving link mode never mutates the source Dockerfile; FAILS without guard, passes with it + `test_link_mode_injects_layers_but_never_mutates_source_dockerfile`: confirmed FAILED on stashed (un-guarded) helper (`assert not is_symlink` -> True), PASSES with the fix. +- DONE: Keep AC-1/AC-2/AC-3 + the build-context rider green + `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` -> 29 passed. + +### Summary + +Fixed the Critical symlink-write-through defect: under `view_mode="link"` the reflected `environment/Dockerfile` is a symlink into the source tree, so the three layer-injection helpers' `write_text` calls followed the link and corrupted the version-controlled fixtures. Guarded all three with the existing unlink-then-write pattern, restored the two dirty fixtures, and added a regression test (proven red-then-green). The preflight helper's `script_path.write_text` needed no guard since that file is view-owned, not symlinked. Pre-existing unrelated `uv.lock` change left unstaged. diff --git a/src/razorback/benchmarks/spider2_dbt/harbor_view.py b/src/razorback/benchmarks/spider2_dbt/harbor_view.py index 035b131..ad5531f 100644 --- a/src/razorback/benchmarks/spider2_dbt/harbor_view.py +++ b/src/razorback/benchmarks/spider2_dbt/harbor_view.py @@ -128,6 +128,12 @@ def _ensure_spider2_build_context_layer(view_dir: Path) -> None: f"COPY {_DBT_PROJECT_DIRNAME}/ {_APP_ROOT}/", ] ) + # In `view_mode="link"` the reflected Dockerfile is a symlink back into the + # shared source tree; writing through it would follow the link and corrupt + # the version-controlled source. Replace the symlink with a real, + # view-owned file so the layer injection stays inside the view. + if dockerfile.is_symlink(): + dockerfile.unlink() dockerfile.write_text(_insert_before_final_cmd(text, block)) @@ -150,6 +156,12 @@ def _ensure_dbt_deps_image_layer(view_dir: Path) -> None: "RUN if [ -f /app/packages.yml ]; then cd /app && dbt deps; fi", ] ) + # In `view_mode="link"` the reflected Dockerfile is a symlink back into the + # shared source tree; writing through it would follow the link and corrupt + # the version-controlled source. Replace the symlink with a real, + # view-owned file so the layer injection stays inside the view. + if dockerfile.is_symlink(): + dockerfile.unlink() dockerfile.write_text(_insert_before_final_cmd(text, block)) @@ -196,6 +208,12 @@ def _ensure_workspace_preflight_image_layer( f"RUN {command}", ] ) + # In `view_mode="link"` the reflected Dockerfile is a symlink back into the + # shared source tree; writing through it would follow the link and corrupt + # the version-controlled source. Replace the symlink with a real, + # view-owned file so the layer injection stays inside the view. + if dockerfile.is_symlink(): + dockerfile.unlink() dockerfile.write_text(_insert_before_final_cmd(text, block)) diff --git a/tests/unit/test_spider2_dbt_harbor_view.py b/tests/unit/test_spider2_dbt_harbor_view.py index c696cac..3b3a904 100644 --- a/tests/unit/test_spider2_dbt_harbor_view.py +++ b/tests/unit/test_spider2_dbt_harbor_view.py @@ -3,6 +3,7 @@ from pathlib import Path from razorback.benchmarks.spider2_dbt.harbor_view import ( + _DBT_PROJECT_DIRNAME, SPIDER2_DBT_DENY_GLOBS, materialize_spider2_harbor_task_view, ) @@ -248,3 +249,43 @@ def test_spider2_view_excludes_gold_solution_expected_paths(tmp_path): def test_spider2_deny_globs_cover_required_families(): assert {"gold/**", "expected/**", "golden/**"} <= set(SPIDER2_DBT_DENY_GLOBS) assert set(DEFAULT_SOLUTION_DENY_GLOBS) <= set(SPIDER2_DBT_DENY_GLOBS) + + +# --- REGRESSION: link mode must never mutate the source Dockerfile --------- +# Mirrors test_link_mode_symlinks_files_but_never_mutates_source_task_toml. +# Under `view_mode="link"` the reflected environment/Dockerfile is a symlink +# back into the shared source tree; the three image-layer helpers each call +# `dockerfile.write_text(...)`, which would FOLLOW the symlink and corrupt the +# version-controlled source (and leak idempotency markers, suppressing layer +# injection on later runs). Each helper must unlink the symlink before writing +# so the view owns a real file. + + +def test_link_mode_injects_layers_but_never_mutates_source_dockerfile(tmp_path): + source = _write_source( + tmp_path / "source", with_packages=True, with_duckdb=True + ) + source_dockerfile = source / "environment" / "Dockerfile" + source_dockerfile_before = source_dockerfile.read_text() + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + view_mode="link", + ) + + # The view's Dockerfile is a real, view-owned file (not a symlink) and + # carries all three injected layers. + view_dockerfile = view / "environment" / "Dockerfile" + assert view_dockerfile.is_file() + assert not view_dockerfile.is_symlink() + view_text = view_dockerfile.read_text() + assert "Razorback: install declared dbt packages before agent runtime." in view_text + assert "Razorback: validate spider2-dbt source DuckDB before agent runtime." in view_text + assert f"COPY {_DBT_PROJECT_DIRNAME}/ /app/" in view_text + + # The SOURCE Dockerfile is byte-for-byte unchanged — no write followed the + # symlink, and no idempotency marker leaked back into the source. + assert source_dockerfile.read_text() == source_dockerfile_before + assert "Razorback:" not in source_dockerfile.read_text() From e78b809494c4ba9586ad050a00d3f5bbd646fa80 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 17:45:03 +0800 Subject: [PATCH 07/19] =?UTF-8?q?validation(spider2-dbt):=20cycle=202=20PA?= =?UTF-8?q?SSED=20=E2=80=94=20B1=20fix=20verified=20load-bearing=20from=20?= =?UTF-8?q?clean=20checkout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-verified the unlink-then-write guard in all three Dockerfile helpers from a fresh clone of HEAD 7f31b7b: source fixtures unchanged (git diff tests/fixtures empty), view Dockerfile a real file with injected layers, regression test proven load-bearing (fails without the guard). 29 passed; generic materializer/leakage unchanged; /app contract pinned for r5. Gate: PASSED -> done. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 15 ++++ .../spider2-dbt-harbor-view-ade-parity.md | 83 +++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 0cca5a4..8f61c36 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -149,3 +149,18 @@ Routing back to `implementation`: ### Summary Fixed the Critical symlink-write-through defect: under `view_mode="link"` the reflected `environment/Dockerfile` is a symlink into the source tree, so the three layer-injection helpers' `write_text` calls followed the link and corrupted the version-controlled fixtures. Guarded all three with the existing unlink-then-write pattern, restored the two dirty fixtures, and added a regression test (proven red-then-green). The preflight helper's `script_path.write_text` needed no guard since that file is view-owned, not symlinked. Pre-existing unrelated `uv.lock` change left unstaged. + +## Stage Report: validation (cycle 2) + +- DONE: Confirm the Critical defect B1 is fixed: from a clean checkout, exercise the materializer in link mode and assert the SOURCE fixture Dockerfiles are NOT mutated (git diff tests/fixtures empty) while the view Dockerfile is a real file carrying the injected layers. Confirm the regression test `test_link_mode_injects_layers_but_never_mutates_source_dockerfile` is load-bearing (fails without the unlink guard). + Fresh clone of branch HEAD `7f31b7b`. Direct link-mode materialize: source Dockerfile sha unchanged + no marker leak; view Dockerfile `is_file() and not is_symlink()` with all 3 layers. `git diff tests/fixtures` empty (vs HEAD and vs base). Stripped all 3 guards → regression test FAILS at `assert not is_symlink()`; restored → passes. Guard mirrors `materialize.py:144-145`. +- DONE: Confirm no regression to AC-1/AC-2/AC-3 + the build-context rider: `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` (expect 29 passed). Generic `materialize.py`/`leakage.py` and non-spider2 harbor behavior unchanged. + 29 passed, 751 deselected (EXIT 0). `materialize.py`/`leakage.py` byte-for-byte unchanged vs `996d42b`. ade_bench harbor_view + preflight → 13 passed. +- DONE: Confirm the `/app` + `/app/.duckdb` image/workdir contract is still pinned and stable for r5 to consume. Give a gate verdict. + `_APP_ROOT="/app"` (harbor_view.py:29); preflight at `/tmp/razorback_spider2_preflight.py --workspace /app`; agent DB at `/app/.duckdb` via `COPY dbt_project/ /app/`. Pinned and stable. +- DONE: Gate verdict. + PASSED → done. Report appended to validation/spider2-dbt-harbor-view-ade-parity.md (cycle-2 section). + +### Summary + +Independently re-verified the cycle-1 fix from a fresh clone of branch HEAD `7f31b7b` (no production code written). B1 is genuinely fixed: the unlink-then-write guard is present in all three Dockerfile-writing helpers (mirroring `materialize.py:144-145`), the two source fixtures are restored and clean (`git diff tests/fixtures` empty), and the regression test is proven load-bearing — stripping the three guards makes `test_link_mode_injects_layers_but_never_mutates_source_dockerfile` fail at `assert not is_symlink()`. A direct link-mode materialize leaves the source Dockerfile byte-for-byte unchanged (no marker leak) while the view Dockerfile is a real owned file carrying all three layers. Acceptance is `29 passed`; generic materializer/leakage unchanged; the `/app` contract is pinned for r5. The pre-existing `razorback.score.load` collection error (and two further base-failing tests in `test_generate_matrix_specs.py` / `test_rk_research_new.py`, both failing on clean `996d42b`) are unrelated and out of scope. **Gate: PASSED → done.** diff --git a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md index dee51d6..443d0cd 100644 --- a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md @@ -174,3 +174,86 @@ the fix. (The `uv.lock` modification is unrelated: `uv sync`/`uv run` drops the Everything else — all 3 ACs, the RIDER build-context proof, the pinned `/app` contract, unchanged generic materializer/leakage, no non-spider2 regressions — is verified and sound. Re-run this validation after B1 lands. + +--- + +## Re-validation — cycle 2 (2026-06-18) + +**Range reviewed:** `996d42b` (base) .. `7f31b7b` (HEAD, includes cycle-1 fix). +**Method:** fresh `git clone --single-branch` of the worktree branch into +`/tmp/spider2-dbt-validate-c2` (since torn down); no production code written. +Pre-existing dirty `uv.lock` in the worktree is a harness artifact — ignored. + +**Gate verdict: PASSED → `done`.** + +### B1 fix is real and load-bearing + +- **Fix present in all three helpers.** `git show 7f31b7b` adds the + `if dockerfile.is_symlink(): dockerfile.unlink()` guard immediately before + each `dockerfile.write_text(...)` in `_ensure_spider2_build_context_layer` + (harbor_view.py:135), `_ensure_dbt_deps_image_layer` (:163), and + `_ensure_workspace_preflight_image_layer` (:215). Byte-identical pattern to + the reference at `materialize.py:144-145`. The fourth `write_text` (:188, + `razorback_spider2_preflight.py`) correctly needs no guard — that file is + freshly created and view-owned, never symlinked into source. +- **Fixtures restored / clean.** In the fresh clone `git status --short -- + tests/fixtures` is empty; `git diff 996d42b HEAD -- tests/fixtures` is empty; + both `spider2-fixture-00{1,2}/environment/Dockerfile` are `FROM python:3.12` + with zero `Razorback:` markers. +- **Regression test is load-bearing.** Stripped all three guards from a clean + checkout (left `write_text` intact); `test_link_mode_injects_layers_but_never_mutates_source_dockerfile` + then FAILS at `assert not view_dockerfile.is_symlink()` (the view Dockerfile + is still a symlink, so the write would follow it). Restored the guard → test + passes. Confirms the test guards the exact B1 hazard. +- **Independent end-to-end proof.** Exercised `materialize_spider2_harbor_task_view(..., view_mode="link")` + directly (outside the test) against a source with `packages.yml` + a real + `.duckdb`: the source `environment/Dockerfile` is byte-for-byte unchanged + (sha unchanged), no `Razorback` marker leaked into source, and the view + Dockerfile `is_file() and not is_symlink()` carrying all three injected + layers (build-context COPY, `dbt deps` RUN, preflight COPY+RUN). The working + tree stayed clean after the run. + +### No regression to AC-1/AC-2/AC-3 + build-context rider + +`uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` +→ **29 passed, 751 deselected** (clean checkout, `EXIT 0`) — matches the +expected 29 (28 from cycle 1 + the new regression test). Generic +`materialize.py`/`leakage.py` byte-for-byte unchanged vs base +(`git diff --stat 996d42b HEAD` empty for both). Non-spider2 harbor behavior +unchanged: `test_ade_bench_harbor_view.py` + `test_ade_bench_workspace_preflight.py` +→ 13 passed. + +### `/app` contract still pinned for r5 + +`_APP_ROOT = "/app"` (harbor_view.py:29); preflight script COPY'd to +`/tmp/razorback_spider2_preflight.py` and invoked `--workspace /app`; agent DB +lands at `/app/.duckdb` via `COPY dbt_project/ /app/`. Stable invariant +for the r5 `duckdb_match` verifier. + +### Code review (focused on the fix diff) + +No blocking findings. The `is_file()` precheck (:109/:146/:184) follows +symlinks so link-mode Dockerfiles are still processed (reach the unlink guard); +`text = dockerfile.read_text()` reads the source content as the injection base +before the unlink, then the patched content is written to the fresh view-owned +file. Non-blocking nit (N2): the identical 4-line guard comment is duplicated +verbatim across the three helpers — a one-line `# see materialize.py:144` would +suffice. Not a correctness issue; not gating. + +### Pre-existing, unrelated failures (NOT regressions) + +`tests/unit/test_task_identity_scoring.py` (`ModuleNotFoundError: razorback.score.load`) +— confirmed present verbatim on base `996d42b`. While running the full unit +suite I also observed `test_generate_matrix_specs.py::test_matrix_specs_carry_query_mode_batch` +and `test_rk_research_new.py::test_rk_research_new_creates_scaffold_tree` failing; +both ALSO fail on a clean `996d42b` worktree and this entity touches only +`benchmarks/spider2_dbt/{harbor_view,preflight}.py` (the sole two source files +in `git diff --name-only 996d42b HEAD`), so neither can be a regression from +this work. Surfaced here for the captain's awareness, out of this entity's scope. + +### Decision + +All three ACs, the build-context rider, the pinned `/app` contract, and the +cycle-1 Critical defect B1 are verified from a clean checkout. The fix is real, +mirrors the established pattern, and its regression test is proven load-bearing. +**PASSED → `done`.** From 22002865ccffceca2c054de66dc6c08ee44b15df Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 17:52:04 +0800 Subject: [PATCH 08/19] =?UTF-8?q?feedback:=20validation=20gate=20rejected?= =?UTF-8?q?=20(cycle=202)=20=E2=80=94=20db=5Fname=20pin=20+=20schema-aware?= =?UTF-8?q?=20preflight?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../spider2-dbt-harbor-view-ade-parity.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 8f61c36..09d9b75 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -164,3 +164,18 @@ Fixed the Critical symlink-write-through defect: under `view_mode="link"` the re ### Summary Independently re-verified the cycle-1 fix from a fresh clone of branch HEAD `7f31b7b` (no production code written). B1 is genuinely fixed: the unlink-then-write guard is present in all three Dockerfile-writing helpers (mirroring `materialize.py:144-145`), the two source fixtures are restored and clean (`git diff tests/fixtures` empty), and the regression test is proven load-bearing — stripping the three guards makes `test_link_mode_injects_layers_but_never_mutates_source_dockerfile` fail at `assert not is_symlink()`. A direct link-mode materialize leaves the source Dockerfile byte-for-byte unchanged (no marker leak) while the view Dockerfile is a real owned file carrying all three layers. Acceptance is `29 passed`; generic materializer/leakage unchanged; the `/app` contract is pinned for r5. The pre-existing `razorback.score.load` collection error (and two further base-failing tests in `test_generate_matrix_specs.py` / `test_rk_research_new.py`, both failing on clean `996d42b`) are unrelated and out of scope. **Gate: PASSED → done.** + +### Cycle 2 — validation gate REJECTED (2026-06-18, captain via Codex review) + +A Codex adversarial review of the implementation surfaced two valid preflight +robustness gaps; captain chose fix-both. Both confirmed live. Routing back to +`implementation`: + +1. **[high] db_name not pinned — preflight glob-firsts; the `/app/.duckdb` contract is prose-only.** + `preflight.py` supports `--db-name`/`--db-path` (resolves `workspace/f"{db_name}.duckdb"`, else `sorted(glob("*.duckdb"))[0]`), but the injected RUN (`harbor_view.py:194-202`) passes only `--task-id` + `--workspace`, so the build-time guard always glob-firsts and can validate the wrong DB under multi/stale-DB drift. + **Fix:** IMPLEMENT db_name resolution (parse the dbt `profiles.yml` `path:` / source `.duckdb` filename; slug fallback) and pass `--db-name` into the injected preflight RUN; OR fail closed when >1 `*.duckdb` and none specified. Expose the resolver as an importable function so the **r5 verifier can reuse it** (this IS the shared contract r5 consumes — don't leave it prose-only). Add a test: a multi-DB workspace pins the right DB (or fails closed), and the injected RUN carries `--db-name`. +2. **[medium] Source-table check ignores schema.** + The dbt `sources:` parser records bare table names and the DuckDB query selects `DISTINCT table_name` (`preflight.py:161-163`), so `other.raw_orders` satisfies a source expecting `main.raw_orders`. + **Fix:** track required + observed relations as `(schema, table)` pairs (honor dbt source/table `schema` fields; compare against `information_schema.tables.table_schema` + `table_name`). Add a test: a table in the wrong schema does NOT satisfy the source. + +Keep AC-1/2/3, the build-context rider, and the cycle-1 unlink-then-write fix green. From d58d1f85916df43918cd34a6574252279d70c866 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:10:02 +0800 Subject: [PATCH 09/19] fix(spider2-dbt): pin --db-name into preflight RUN + schema-aware source check Validation gate cycle-2 feedback (two valid Codex findings): Finding 1 (high): the /app/.duckdb contract was prose-only. The injected build-time preflight RUN passed only --task-id/--workspace, so it glob-firsted and could validate the wrong DB under multi/stale-DB drift. Wire the importable resolve_spider2_db_name (profiles.yml path: -> single *.duckdb -> task-slug; fails closed on >1 *.duckdb with none pinned) into _ensure_workspace_preflight_image_layer and thread --db-name into the RUN. The resolver is exposed for the r5 verifier to reuse the SAME resolution. Finding 2 (medium): the source-table check ignored schema. Track required and observed relations as (schema, table) pairs (honor dbt source/table schema fields, default to source name; compare against information_schema table_schema + table_name) so a table in the wrong schema no longer satisfies a source. AC-1/2/3, the build-context rider, and the cycle-1 unlink-then-write fix stay green: uv run pytest -k spider2_dbt --ignore=...test_task_identity_scoring.py -> 38 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../benchmarks/spider2_dbt/harbor_view.py | 39 ++++- .../benchmarks/spider2_dbt/preflight.py | 106 ++++++++++-- tests/unit/test_spider2_dbt_harbor_view.py | 61 +++++++ .../test_spider2_dbt_workspace_preflight.py | 153 +++++++++++++++++- 4 files changed, 337 insertions(+), 22 deletions(-) diff --git a/src/razorback/benchmarks/spider2_dbt/harbor_view.py b/src/razorback/benchmarks/spider2_dbt/harbor_view.py index ad5531f..3f8fa3d 100644 --- a/src/razorback/benchmarks/spider2_dbt/harbor_view.py +++ b/src/razorback/benchmarks/spider2_dbt/harbor_view.py @@ -5,7 +5,10 @@ from pathlib import Path from typing import Literal -from razorback.benchmarks.spider2_dbt.preflight import preflight_script_text +from razorback.benchmarks.spider2_dbt.preflight import ( + preflight_script_text, + resolve_spider2_db_name, +) from razorback.harbor_tasks.leakage import DEFAULT_SOLUTION_DENY_GLOBS from razorback.harbor_tasks.materialize import materialize_harbor_task_view @@ -78,10 +81,24 @@ def materialize_spider2_harbor_task_view( def _has_dbt_project(view_dir: Path) -> bool: """spider2-dbt nests the dbt project under `dbt_project/` (or under `environment/dbt_project/`).""" - return ( - (view_dir / _DBT_PROJECT_DIRNAME).is_dir() - or (view_dir / "environment" / _DBT_PROJECT_DIRNAME).is_dir() - ) + return _dbt_project_dir(view_dir) is not None + + +def _dbt_project_dir(view_dir: Path) -> Path | None: + """The dbt project root inside the view (`dbt_project/` or + `environment/dbt_project/`), if present. + + This is the on-disk stand-in for the container's `/app` dbt root: the + source `.duckdb` and any `profiles.yml` live here, so it is the workspace + `resolve_spider2_db_name` reads to pin `/app/.duckdb`. + """ + direct = view_dir / _DBT_PROJECT_DIRNAME + if direct.is_dir(): + return direct + nested = view_dir / "environment" / _DBT_PROJECT_DIRNAME + if nested.is_dir(): + return nested + return None def _has_dbt_packages_manifest(view_dir: Path) -> bool: @@ -191,6 +208,16 @@ def _ensure_workspace_preflight_image_layer( if _SPIDER2_WORKSPACE_PREFLIGHT_MARKER in text: return + # Pin the agent-facing DuckDB via the SHARED resolver so the build-time + # preflight validates the SAME `/app/.duckdb` the agent (and the + # r5 verifier) operate against — never a glob-first under multi/stale-DB + # drift. Resolution fails CLOSED (raises) when >1 *.duckdb exists and none + # is pinned; that aborts the materialize, the correct fail-closed point. + project_dir = _dbt_project_dir(view_dir) + db_name = resolve_spider2_db_name( + project_dir if project_dir is not None else view_dir, + task_slug=task_slug, + ) command = " ".join( [ "python", @@ -199,6 +226,8 @@ def _ensure_workspace_preflight_image_layer( shlex.quote(task_slug), "--workspace", _APP_ROOT, + "--db-name", + shlex.quote(db_name), ] ) block = "\n".join( diff --git a/src/razorback/benchmarks/spider2_dbt/preflight.py b/src/razorback/benchmarks/spider2_dbt/preflight.py index 649a558..bfa552d 100644 --- a/src/razorback/benchmarks/spider2_dbt/preflight.py +++ b/src/razorback/benchmarks/spider2_dbt/preflight.py @@ -66,13 +66,13 @@ def preflight_spider2_workspace( payload["error"] = repr(exc) raise Spider2WorkspacePreflightError(payload) from exc - payload["observed_tables"] = sorted(observed_tables) + payload["observed_tables"] = sorted(_format_relations(observed_tables)) required_tables = _read_dbt_source_tables(workspace) if required_tables: - payload["required_tables"] = sorted(required_tables) + payload["required_tables"] = sorted(_format_relations(required_tables)) payload["required_tables_source"] = "dbt_source_metadata" - missing = sorted(required_tables - observed_tables) + missing = sorted(_format_relations(required_tables - observed_tables)) payload["missing_tables"] = missing if missing: payload["status"] = "failed" @@ -87,6 +87,69 @@ def preflight_spider2_workspace( return payload +def resolve_spider2_db_name(workspace: Path, *, task_slug: str) -> str: + """Resolve the agent-facing DuckDB stem for `/app/.duckdb`. + + This is the SHARED `/app/.duckdb` contract: the build-time + preflight (`harbor_view.py`) and the r5 verifier + (`spider2-dbt-duckdb-match-verifier`) MUST import and reuse this exact + resolution so all three agree on which DuckDB the agent operates against. + + Resolution order: + 1. The dbt `profiles.yml` `path:` value — strip directories and the + `.duckdb` suffix to get the stem. + 2. Exactly one `*.duckdb` already present in the workspace — use its stem. + 3. The task slug (used as the DB name when the project ships nothing). + + Fails CLOSED (`Spider2WorkspacePreflightError`, reason `ambiguous duckdb + file`) when >1 `*.duckdb` exists and no `profiles.yml` `path:` pins one, so + a multi/stale-DB workspace never silently validates the wrong DB. + """ + workspace = Path(workspace) + + profile_path = _read_profiles_db_path(workspace) + if profile_path: + return Path(profile_path).name.removesuffix(".duckdb") + + if workspace.is_dir(): + candidates = sorted(workspace.glob("*.duckdb")) + if len(candidates) == 1: + return candidates[0].name.removesuffix(".duckdb") + if len(candidates) > 1: + raise Spider2WorkspacePreflightError( + { + "status": "failed", + "reason": "ambiguous duckdb file", + "task_id": task_slug, + "candidates": sorted(c.name for c in candidates), + } + ) + + return task_slug + + +def _read_profiles_db_path(workspace: Path) -> str | None: + """Return the first dbt `profiles.yml` DuckDB output `path:`, if any.""" + try: + import yaml + except Exception: + return None + if not workspace.is_dir(): + return None + for profiles_path in sorted(workspace.rglob("profiles.yml")): + try: + document = yaml.safe_load(profiles_path.read_text()) + except Exception: + continue + for profile in _iter_dicts(list(_as_dict(document).values())): + outputs = _as_dict(profile.get("outputs")) + for output in _iter_dicts(list(outputs.values())): + path = output.get("path") + if isinstance(path, str) and path.strip().endswith(".duckdb"): + return path.strip() + return None + + def _resolve_db_path( *, workspace: Path, db_name: str | None, db_path: Path | None ) -> Path | None: @@ -101,25 +164,44 @@ def _resolve_db_path( return None -def _read_dbt_source_tables(workspace: Path) -> set[str]: - """Read dbt `sources:` table names when the task ships source metadata.""" +def _read_dbt_source_tables(workspace: Path) -> set[tuple[str, str]]: + """Read dbt `sources:` as `(schema, table)` relations. + + The relation schema is resolved with dbt's precedence: a table-level + `schema` overrides a source-level `schema`, which in turn defaults to the + source `name` (dbt's documented default when a source omits `schema`). The + table identifier follows the same `identifier`-over-`name` precedence. Both + parts are lowercased to match `_read_duckdb_tables`. + """ try: import yaml except Exception: return set() - tables: set[str] = set() + relations: set[tuple[str, str]] = set() for yaml_path in _iter_candidate_dbt_yaml_files(workspace): try: document = yaml.safe_load(yaml_path.read_text()) except Exception: continue for source in _iter_dicts(_as_list(_as_dict(document).get("sources"))): + source_name = source.get("name") + source_schema = source.get("schema") or source_name for table in _iter_dicts(_as_list(source.get("tables"))): name = table.get("identifier") or table.get("name") - if isinstance(name, str) and name.strip(): - tables.add(name.strip().lower()) - return tables + if not (isinstance(name, str) and name.strip()): + continue + schema = table.get("schema") or source_schema + if not (isinstance(schema, str) and schema.strip()): + continue + relations.add( + (schema.strip().lower(), name.strip().lower()) + ) + return relations + + +def _format_relations(relations: set[tuple[str, str]]) -> list[str]: + return [f"{schema}.{table}" for schema, table in relations] def _iter_candidate_dbt_yaml_files(workspace: Path): @@ -151,21 +233,21 @@ def _iter_dicts(values: list[Any]): yield value -def _read_duckdb_tables(db_path: Path) -> set[str]: +def _read_duckdb_tables(db_path: Path) -> set[tuple[str, str]]: import duckdb conn = duckdb.connect(str(db_path), read_only=True) try: rows = conn.execute( """ - SELECT DISTINCT table_name + SELECT DISTINCT table_schema, table_name FROM information_schema.tables WHERE table_schema NOT IN ('information_schema', 'pg_catalog') """ ).fetchall() finally: conn.close() - return {str(row[0]).lower() for row in rows} + return {(str(row[0]).lower(), str(row[1]).lower()) for row in rows} def main(argv: list[str] | None = None) -> int: diff --git a/tests/unit/test_spider2_dbt_harbor_view.py b/tests/unit/test_spider2_dbt_harbor_view.py index 3b3a904..6ef417c 100644 --- a/tests/unit/test_spider2_dbt_harbor_view.py +++ b/tests/unit/test_spider2_dbt_harbor_view.py @@ -194,6 +194,67 @@ def test_preflight_build_context_holds_duckdb_before_preflight_run(tmp_path): ) +# --- Finding 1 (cycle 2): injected RUN must pin --db-name ------------------ +# The build-time preflight must validate the SAME DB the agent runs against +# (`/app/.duckdb`), not glob-first. The materializer resolves the +# db_name and threads it into the injected RUN. + + +def test_injected_preflight_run_carries_db_name(tmp_path): + source = _write_source( + tmp_path / "source", with_packages=True, with_duckdb=True + ) + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + ) + + dockerfile = (view / "environment" / "Dockerfile").read_text() + # The single staged DB is spider2-fixture-001.duckdb, so the resolver pins + # that name and the injected RUN passes it explicitly. + assert "--db-name spider2-fixture-001" in dockerfile + + +def test_injected_preflight_run_pins_db_among_many(tmp_path): + # A multi-DB workspace with a profiles.yml `path:` pins the right DB into + # the injected RUN (not a glob-first of whichever sorts first). + source = _write_source( + tmp_path / "source", with_packages=True, with_duckdb=True + ) + import duckdb as _duckdb + + stale = source / "dbt_project" / "aaa_stale.duckdb" + conn = _duckdb.connect(str(stale)) + try: + conn.execute("CREATE TABLE t (id INTEGER)") + finally: + conn.close() + (source / "dbt_project" / "profiles.yml").write_text( + "\n".join( + [ + "example:", + " outputs:", + " dev:", + " type: duckdb", + " path: spider2-fixture-001.duckdb", + " target: dev", + "", + ] + ) + ) + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + ) + + dockerfile = (view / "environment" / "Dockerfile").read_text() + assert "--db-name spider2-fixture-001" in dockerfile + + def test_preflight_layer_absent_when_not_a_dbt_project(tmp_path): # A non-dbt source (no dbt_project/) gets no preflight layer at all, so the # preflight RUN can never run against an empty /app. diff --git a/tests/unit/test_spider2_dbt_workspace_preflight.py b/tests/unit/test_spider2_dbt_workspace_preflight.py index 73477aa..e3751b6 100644 --- a/tests/unit/test_spider2_dbt_workspace_preflight.py +++ b/tests/unit/test_spider2_dbt_workspace_preflight.py @@ -11,14 +11,22 @@ from razorback.benchmarks.spider2_dbt.preflight import ( Spider2WorkspacePreflightError, preflight_spider2_workspace, + resolve_spider2_db_name, ) def _write_duckdb(path: Path, tables: set[str]) -> None: + """Create user tables in the default (`main`) schema.""" + _write_duckdb_relations(path, {("main", table) for table in tables}) + + +def _write_duckdb_relations(path: Path, relations: set[tuple[str, str]]) -> None: conn = duckdb.connect(str(path)) try: - for table in sorted(tables): - conn.execute(f'CREATE TABLE "{table}" (id INTEGER)') + for schema, table in sorted(relations): + if schema != "main": + conn.execute(f'CREATE SCHEMA IF NOT EXISTS "{schema}"') + conn.execute(f'CREATE TABLE "{schema}"."{table}" (id INTEGER)') finally: conn.close() @@ -34,8 +42,8 @@ def test_present_readable_duckdb_passes(tmp_path: Path) -> None: assert result["status"] == "passed" assert result["task_id"] == "spider2-fixture-001" - assert "orders" in result["observed_tables"] - assert "customers" in result["observed_tables"] + assert "main.orders" in result["observed_tables"] + assert "main.customers" in result["observed_tables"] def test_present_duckdb_discovered_without_db_name(tmp_path: Path) -> None: @@ -96,6 +104,7 @@ def test_dbt_source_metadata_required_tables_enforced(tmp_path: Path) -> None: "version: 2", "sources:", " - name: canonical", + " schema: main", " tables:", " - name: orders", " identifier: raw_orders", @@ -110,7 +119,7 @@ def test_dbt_source_metadata_required_tables_enforced(tmp_path: Path) -> None: _write_duckdb(tmp_path / "db.duckdb", {"raw_orders"}) with pytest.raises(Spider2WorkspacePreflightError) as exc_info: preflight_spider2_workspace(task_id="t", workspace=tmp_path) - assert "raw_customers" in exc_info.value.payload["missing_tables"] + assert "main.raw_customers" in exc_info.value.payload["missing_tables"] # DuckDB with both -> passes (tmp_path / "db.duckdb").unlink() @@ -171,3 +180,137 @@ def test_preflight_cli_passes_on_present_readable_duckdb(tmp_path: Path) -> None assert completed.returncode == 0 assert "RAZORBACK_SPIDER2_PREFLIGHT" in completed.stdout + + +# --- Finding 2 (cycle 2): source-table check must be schema-aware ---------- + + +def test_source_table_in_wrong_schema_does_not_satisfy_source(tmp_path: Path) -> None: + # A dbt source explicitly scoped to schema `main` must NOT be satisfied by + # the same table name living in a different schema. + models = tmp_path / "models" + models.mkdir() + (models / "sources.yml").write_text( + "\n".join( + [ + "version: 2", + "sources:", + " - name: canonical", + " schema: main", + " tables:", + " - name: raw_orders", + "", + ] + ) + ) + + # raw_orders exists only under `other`, never under `main`. + _write_duckdb_relations(tmp_path / "db.duckdb", {("other", "raw_orders")}) + + with pytest.raises(Spider2WorkspacePreflightError) as exc_info: + preflight_spider2_workspace(task_id="t", workspace=tmp_path) + payload = exc_info.value.payload + assert payload["status"] == "failed" + assert payload["reason"] == "required dbt source tables missing" + assert "main.raw_orders" in payload["missing_tables"] + + # Now place raw_orders in the expected `main` schema -> passes. + (tmp_path / "db.duckdb").unlink() + _write_duckdb_relations(tmp_path / "db.duckdb", {("main", "raw_orders")}) + result = preflight_spider2_workspace(task_id="t", workspace=tmp_path) + assert result["status"] == "passed" + assert result["required_tables_source"] == "dbt_source_metadata" + + +def test_source_schema_falls_back_to_source_level_schema(tmp_path: Path) -> None: + # The table-level `schema`/`identifier` overrides win, but otherwise the + # source-level `schema` (or source `name`) supplies the relation schema. + models = tmp_path / "models" + models.mkdir() + (models / "sources.yml").write_text( + "\n".join( + [ + "version: 2", + "sources:", + " - name: raw", + " schema: staging", + " tables:", + " - name: orders", + " - name: customers", + " schema: warehouse", + "", + ] + ) + ) + + _write_duckdb_relations( + tmp_path / "db.duckdb", + {("staging", "orders"), ("warehouse", "customers")}, + ) + result = preflight_spider2_workspace(task_id="t", workspace=tmp_path) + assert result["status"] == "passed" + assert "staging.orders" in result["required_tables"] + assert "warehouse.customers" in result["required_tables"] + + +# --- Finding 1 (cycle 2): db_name resolution is an importable function ------ + + +def test_resolve_db_name_from_profiles_path(tmp_path: Path) -> None: + (tmp_path / "profiles.yml").write_text( + "\n".join( + [ + "example:", + " target: dev", + " outputs:", + " dev:", + " type: duckdb", + " path: warehouse.duckdb", + "", + ] + ) + ) + assert resolve_spider2_db_name(tmp_path, task_slug="spider2-fixture-001") == "warehouse" + + +def test_resolve_db_name_from_single_duckdb_file(tmp_path: Path) -> None: + _write_duckdb(tmp_path / "anything.duckdb", {"t1"}) + assert resolve_spider2_db_name(tmp_path, task_slug="spider2-fixture-001") == "anything" + + +def test_resolve_db_name_falls_back_to_task_slug(tmp_path: Path) -> None: + assert ( + resolve_spider2_db_name(tmp_path, task_slug="spider2-fixture-001") + == "spider2-fixture-001" + ) + + +def test_resolve_db_name_fails_closed_on_multi_db_with_no_profile(tmp_path: Path) -> None: + _write_duckdb(tmp_path / "a.duckdb", {"t1"}) + _write_duckdb(tmp_path / "b.duckdb", {"t2"}) + with pytest.raises(Spider2WorkspacePreflightError) as exc_info: + resolve_spider2_db_name(tmp_path, task_slug="spider2-fixture-001") + payload = exc_info.value.payload + assert payload["status"] == "failed" + assert payload["reason"] == "ambiguous duckdb file" + assert sorted(payload["candidates"]) == ["a.duckdb", "b.duckdb"] + + +def test_resolve_db_name_profile_pins_db_among_many(tmp_path: Path) -> None: + # profiles.yml `path:` pins the DB even when several *.duckdb exist. + _write_duckdb(tmp_path / "stale.duckdb", {"t1"}) + _write_duckdb(tmp_path / "warehouse.duckdb", {"t2"}) + (tmp_path / "profiles.yml").write_text( + "\n".join( + [ + "example:", + " outputs:", + " dev:", + " type: duckdb", + " path: /app/warehouse.duckdb", + " target: dev", + "", + ] + ) + ) + assert resolve_spider2_db_name(tmp_path, task_slug="x") == "warehouse" From b1d52b497b2539f7a538df2bd49cb97dcc164929 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:10:27 +0800 Subject: [PATCH 10/19] docs(spider2-dbt): implementation stage report (cycle 3) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 09d9b75..027b653 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -179,3 +179,16 @@ robustness gaps; captain chose fix-both. Both confirmed live. Routing back to **Fix:** track required + observed relations as `(schema, table)` pairs (honor dbt source/table `schema` fields; compare against `information_schema.tables.table_schema` + `table_name`). Add a test: a table in the wrong schema does NOT satisfy the source. Keep AC-1/2/3, the build-context rider, and the cycle-1 unlink-then-write fix green. + +## Stage Report: implementation (cycle 3) + +- DONE: Finding 1 (high): implement db_name resolution as an importable function (parse the dbt `profiles.yml` `path:` / source `.duckdb` filename; task-slug fallback) and pass `--db-name` into the injected preflight RUN in `harbor_view.py`; OR fail closed when >1 `*.duckdb` exists and none is specified. Expose it so the r5 verifier can import and reuse it. Add a test: a multi-DB workspace pins the right DB (or fails closed) and the injected RUN carries `--db-name`. + Resolver `resolve_spider2_db_name` already existed (importable, profiles->single-glob->slug, fails closed on >1) but was UNWIRED — the injected RUN still glob-firsted (committed `b005412`). Wired it into `_ensure_workspace_preflight_image_layer` via new `_dbt_project_dir` (resolve against the view's dbt root); threaded `--db-name`. `test_injected_preflight_run_carries_db_name` + `_pins_db_among_many` were RED on entry (committed working-tree), now green. Materializer fail-closed proven end-to-end: ambiguous multi-DB -> `Spider2WorkspacePreflightError(reason="ambiguous duckdb file")`. Commit e60795e. +- DONE: Finding 2 (medium): make the preflight source-table check schema-aware — track required + observed relations as `(schema, table)` pairs (honor dbt source/table `schema` fields; compare against `information_schema.tables.table_schema` + `table_name`). Add a test: a table in the wrong schema does NOT satisfy a source expecting a specific schema. + `_read_dbt_source_tables` returns `(schema, table)` (table-schema > source-schema > source-name precedence); `_read_duckdb_tables` selects `table_schema, table_name`. `test_source_table_in_wrong_schema_does_not_satisfy_source` + `_schema_falls_back_to_source_level_schema` green. (Source from prior uncommitted working-tree; committed in e60795e.) +- DONE: Keep AC-1/AC-2/AC-3, the build-context rider, and the cycle-1 unlink-then-write fix all green. Leave the worktree clean (restore the dirty `uv.lock`). + `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` -> 38 passed. Cycle-1 `test_link_mode_injects_layers_but_never_mutates_source_dockerfile` passes. Generic `materialize.py`/`leakage.py` byte-for-byte unchanged vs base `996d42b`; ade_bench harbor_view+preflight 13 passed. `uv.lock` restored; `git status` clean. + +### Summary + +Both cycle-2 findings resolved and committed (e60795e). The prior session had left the deliverable uncommitted and INCOMPLETE: Finding 2 (schema-aware check) and the `resolve_spider2_db_name` resolver + its tests were written, but the resolver was never wired into the injected preflight RUN — the two `test_injected_preflight_run_*` tests were failing on entry. Completed Finding 1 by threading `--db-name` (resolved against the view's dbt project dir) into `_ensure_workspace_preflight_image_layer`; the resolver fails closed at materialize time on an ambiguous multi-DB workspace, proven end-to-end. The resolver stays importable for the r5 verifier to reuse the SAME `/app/.duckdb` resolution. Acceptance is 38 passed; generic surfaces and the cycle-1 unlink fix are unregressed; worktree clean. From 8d51dd46ec3611ec6620b5c12856b4746b7928a0 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:15:24 +0800 Subject: [PATCH 11/19] =?UTF-8?q?validation(spider2-dbt):=20cycle=203=20PA?= =?UTF-8?q?SSED=20=E2=80=94=20db-name=20pin=20+=20schema-aware=20check=20v?= =?UTF-8?q?erified=20load-bearing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 15 ++++ .../spider2-dbt-harbor-view-ade-parity.md | 75 +++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 027b653..6ababfa 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -192,3 +192,18 @@ Keep AC-1/2/3, the build-context rider, and the cycle-1 unlink-then-write fix gr ### Summary Both cycle-2 findings resolved and committed (e60795e). The prior session had left the deliverable uncommitted and INCOMPLETE: Finding 2 (schema-aware check) and the `resolve_spider2_db_name` resolver + its tests were written, but the resolver was never wired into the injected preflight RUN — the two `test_injected_preflight_run_*` tests were failing on entry. Completed Finding 1 by threading `--db-name` (resolved against the view's dbt project dir) into `_ensure_workspace_preflight_image_layer`; the resolver fails closed at materialize time on an ambiguous multi-DB workspace, proven end-to-end. The resolver stays importable for the r5 verifier to reuse the SAME `/app/.duckdb` resolution. Acceptance is 38 passed; generic surfaces and the cycle-1 unlink fix are unregressed; worktree clean. + +## Stage Report: validation (cycle 3) + +- DONE: Confirm Finding 1 fixed: the injected preflight RUN now carries `--db-name` (db_name resolved from dbt profiles.yml path / source .duckdb filename, slug fallback), and the resolver fails closed on an ambiguous multi-DB workspace (`Spider2WorkspacePreflightError(reason="ambiguous duckdb file")`). Confirm `resolve_spider2_db_name` is importable for the r5 verifier to reuse. Exercise it; do not just read it. + Exercised end-to-end: real materialize of a single-DB task emits `RUN ... --db-name spider2-fixture-001` (resolved, not glob-firsted). Resolver exercised directly: ambiguous 2-DB → raises `reason="ambiguous duckdb file"`; single→stem; profiles.yml `path:` wins over a 2-DB dir; empty→slug. Importable: `from razorback.benchmarks.spider2_dbt.preflight import resolve_spider2_db_name` OK. Materializer fails closed on a 2-DB dbt_project with no profiles. +- DONE: Confirm Finding 2 fixed: the preflight source-table check is schema-aware — a table in the wrong schema does NOT satisfy a source expecting a specific schema (`(schema, table)` pairs). + Exercised against a real DuckDB: source expects `main.raw_orders`, DuckDB has `other.raw_orders` → raises `missing_tables=['main.raw_orders']`; after creating `main.raw_orders` → `status=passed`. Set difference is over `(schema, table)` tuples (preflight.py:75), both sides lowercased. +- DONE: Confirm no regression: AC-1/2/3, the build-context rider, and the cycle-1 unlink-then-write fix all green. `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` (expect ~38 passed). Generic materialize.py/leakage.py unchanged. Give a gate verdict. + 38 passed, 751 deselected. Generic `materialize.py`/`leakage.py` byte-for-byte unchanged vs `996d42b` (empty diff); ade_bench regression 13 passed; 3 unlink guards + cycle-1 test intact. Pre-existing failures confirmed on base (`razorback.score.load` missing in `996d42b`; the 3 named test files untouched by this branch). `uv.lock` ignored per dispatch; `git status` clean. +- DONE: Gate verdict. + PASSED → done. Cycle-3 section appended to validation/spider2-dbt-harbor-view-ade-parity.md. + +### Summary + +Independently re-verified the cycle-3 fix (commit `e60795e`, branch HEAD `73fd832`) from the worktree with no production code written. Both cycle-2 Codex findings are real and load-bearing: (1) `resolve_spider2_db_name` is importable for r5, fails closed on an ambiguous multi-DB workspace, and is now actually wired into the injected preflight RUN (`--db-name` resolved against the view's dbt project dir, proven by exercising the materializer end-to-end — including the fail-closed abort on a 2-DB project); (2) the source-table check is schema-aware — `other.raw_orders` does not satisfy a source expecting `main.raw_orders`, proven against a real DuckDB. Acceptance is 38 passed; generic materializer/leakage unchanged; ade_bench regression 13 passed; the cycle-1 unlink-then-write fix (3 guards + its regression test) is intact; the `/app` contract is pinned. The pre-existing unrelated failures predate this branch (verified on base `996d42b`) and the harness `uv.lock` is ignored per dispatch. **Gate: PASSED → done.** diff --git a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md index 443d0cd..69548b0 100644 --- a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md @@ -257,3 +257,78 @@ All three ACs, the build-context rider, the pinned `/app` contract, and the cycle-1 Critical defect B1 are verified from a clean checkout. The fix is real, mirrors the established pattern, and its regression test is proven load-bearing. **PASSED → `done`.** + +--- + +## Cycle 3 — re-review of the cycle-2 Codex findings fix (2026-06-18) + +Re-validated the cycle-3 fix commit `e60795e` from the worktree branch HEAD +`73fd832`. No production code written; verification by exercising behavior. + +### Finding 1 (high) — db_name pinned into the injected preflight RUN; resolver importable + fails closed — VERIFIED FIXED + +- **Importable for r5:** `from razorback.benchmarks.spider2_dbt.preflight import resolve_spider2_db_name` + imports cleanly. r5 (`spider2-dbt-duckdb-match-verifier`) can reuse the same + `/app/.duckdb` resolution. +- **Resolver behavior (exercised directly):** + - ambiguous multi-DB (`a.duckdb`, `b.duckdb`, no profiles) → raises + `Spider2WorkspacePreflightError(reason="ambiguous duckdb file", candidates=['a.duckdb','b.duckdb'])` + - single `demo.duckdb` → `"demo"` + - `profiles.yml` `path: /some/dir/pinned.duckdb` overrides even a 2-DB dir → `"pinned"` + - empty workspace → slug fallback `"myslug"` +- **Wired into the injected RUN (exercised via the materializer):** a real + end-to-end materialize of a single-DB task emits + `RUN python /tmp/razorback_spider2_preflight.py --task-id spider2-fixture-001 --workspace /app --db-name spider2-fixture-001` + — `--db-name` is present and resolved against the view's dbt project dir + (`harbor_view.py:216-220`), not glob-firsted. +- **Fail-closed is load-bearing at materialize time:** materializing a task whose + `dbt_project/` carries two `*.duckdb` and no `profiles.yml` raises + `Spider2WorkspacePreflightError(reason="ambiguous duckdb file")` — the build + aborts rather than validating the wrong DB. + +### Finding 2 (medium) — schema-aware source-table check — VERIFIED FIXED + +Exercised `preflight_spider2_workspace` against a real DuckDB: + +- dbt source expects `main.raw_orders`; DuckDB has `other.raw_orders` (wrong schema) + → raises `reason="required dbt source tables missing"`, `missing_tables=['main.raw_orders']`. + The wrong-schema table does NOT satisfy the source. +- After creating `main.raw_orders` → `status="passed"`, `missing_tables=[]`. + +Confirmed in code: `_read_dbt_source_tables` returns `(schema, table)` with dbt +precedence (table.schema > source.schema > source.name; identifier > name), and +`_read_duckdb_tables` selects `table_schema, table_name`; the comparison at +`preflight.py:75` is a `(schema, table)` tuple set difference. Both sides +lowercased consistently. + +### No regression + +- `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` + → **38 passed, 751 deselected** (matches expected ~38). +- Generic `materialize.py` / `leakage.py`: `git diff 996d42b..HEAD` empty — + byte-for-byte unchanged. +- ade_bench harbor_view + workspace_preflight regression → 13 passed. +- Cycle-1 unlink-then-write fix intact: 3 `if dockerfile.is_symlink():` guards in + `harbor_view.py`; `test_link_mode_injects_layers_but_never_mutates_source_dockerfile` + present and green. +- `/app` + `/app/.duckdb` contract still pinned (`_APP_ROOT="/app"`, + preflight `--workspace /app`, COPY `dbt_project/ /app/`). +- Pre-existing unrelated failures confirmed on base: `razorback.score.load` is + MISSING in `996d42b` (the `test_task_identity_scoring` import error); the three + named files (`test_task_identity_scoring`, `test_generate_matrix_specs`, + `test_rk_research_new`) are untouched by this branch (empty diff vs base) — not + regressions. Harness `uv.lock` ignored per dispatch. `git status` clean. + +### Code review + +Focused adversarial review of the fix diff (`e60795e`). No blocking findings. +Non-blocking observation: the resolver is invoked against the view's on-disk dbt +project dir as the stand-in for the container `/app` root — coherent with the +runtime contract. Schema precedence and consistent lowercasing make the set +difference correct. + +### Gate + +Both cycle-2 findings are genuinely fixed and load-bearing; the resolver is +importable for r5; no regression to AC-1/2/3, the build-context rider, or the +cycle-1 unlink fix. **PASSED → `done`.** From d17e1514bd2e1b054e99643aaacabfa61f802a6b Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:21:17 +0800 Subject: [PATCH 12/19] =?UTF-8?q?feedback:=20validation=20gate=20rejected?= =?UTF-8?q?=20(cycle=203)=20=E2=80=94=20preflight-helper=20symlink=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../spider2-dbt-harbor-view-ade-parity.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 6ababfa..0c96c95 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -207,3 +207,12 @@ Both cycle-2 findings resolved and committed (e60795e). The prior session had le ### Summary Independently re-verified the cycle-3 fix (commit `e60795e`, branch HEAD `73fd832`) from the worktree with no production code written. Both cycle-2 Codex findings are real and load-bearing: (1) `resolve_spider2_db_name` is importable for r5, fails closed on an ambiguous multi-DB workspace, and is now actually wired into the injected preflight RUN (`--db-name` resolved against the view's dbt project dir, proven by exercising the materializer end-to-end — including the fail-closed abort on a 2-DB project); (2) the source-table check is schema-aware — `other.raw_orders` does not satisfy a source expecting `main.raw_orders`, proven against a real DuckDB. Acceptance is 38 passed; generic materializer/leakage unchanged; ade_bench regression 13 passed; the cycle-1 unlink-then-write fix (3 guards + its regression test) is intact; the `/app` contract is pinned. The pre-existing unrelated failures predate this branch (verified on base `996d42b`) and the harness `uv.lock` is ignored per dispatch. **Gate: PASSED → done.** + +### Cycle 3 — validation gate REJECTED (2026-06-18, captain via Codex review) + +Codex re-review found the symlink-write-through class is not fully closed. +Captain chose fix-now. Routing back to `implementation`: + +1. **[medium] Preflight helper write can still follow a source symlink in link mode (`harbor_view.py:187-188`).** + `_ensure_workspace_preflight_image_layer` does `script_path.write_text(preflight_script_text())` on `environment/razorback_spider2_preflight.py` with NO `is_symlink()` guard. Under `view_mode="link"`, if a source task ships a file with that exact name, the view path is a symlink into the source and the write corrupts the user's source file — the same class as the cycle-1 Dockerfile fix. + **Fix:** add the same `if script_path.is_symlink(): script_path.unlink()` guard before the helper `write_text` (mirror the Dockerfile/`task.toml` guards). Add a link-mode regression test that SEEDS `environment/razorback_spider2_preflight.py` in the source fixture and proves the source file is unchanged after materialization (fails without the guard). Keep all prior ACs, the rider, and cycle-1/cycle-2 fixes green. From 51e725f8037097e934183aa1199ced5aa44b5a51 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:23:35 +0800 Subject: [PATCH 13/19] fix(spider2-dbt): guard preflight-script write against source symlink in link mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The preflight helper wrote environment/razorback_spider2_preflight.py with no is_symlink() guard. Under view_mode="link", a source task shipping a file with that exact name is reflected as a symlink into the source tree, so the write followed the link and corrupted the version-controlled source — the same symlink-write-through class as the cycle-1 Dockerfile fix. Add the unlink-then-write guard mirroring the Dockerfile/task.toml guards, plus a link-mode regression test that seeds the source file and asserts it is unchanged (proven red without the guard). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../benchmarks/spider2_dbt/harbor_view.py | 7 ++++ tests/unit/test_spider2_dbt_harbor_view.py | 34 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/razorback/benchmarks/spider2_dbt/harbor_view.py b/src/razorback/benchmarks/spider2_dbt/harbor_view.py index 3f8fa3d..1f869f5 100644 --- a/src/razorback/benchmarks/spider2_dbt/harbor_view.py +++ b/src/razorback/benchmarks/spider2_dbt/harbor_view.py @@ -202,6 +202,13 @@ def _ensure_workspace_preflight_image_layer( return script_path = environment_dir / "razorback_spider2_preflight.py" + # In `view_mode="link"` a source task that ships a file with this exact name + # is reflected as a symlink back into the shared source tree; writing through + # it would follow the link and corrupt the version-controlled source. Replace + # the symlink with a real, view-owned file so the write stays inside the view + # (mirrors the Dockerfile/task.toml unlink-then-write guards). + if script_path.is_symlink(): + script_path.unlink() script_path.write_text(preflight_script_text()) text = dockerfile.read_text() diff --git a/tests/unit/test_spider2_dbt_harbor_view.py b/tests/unit/test_spider2_dbt_harbor_view.py index 6ef417c..b48b484 100644 --- a/tests/unit/test_spider2_dbt_harbor_view.py +++ b/tests/unit/test_spider2_dbt_harbor_view.py @@ -350,3 +350,37 @@ def test_link_mode_injects_layers_but_never_mutates_source_dockerfile(tmp_path): # symlink, and no idempotency marker leaked back into the source. assert source_dockerfile.read_text() == source_dockerfile_before assert "Razorback:" not in source_dockerfile.read_text() + + +def test_link_mode_preflight_script_never_mutates_source_named_file(tmp_path): + """A source task that happens to ship environment/razorback_spider2_preflight.py + must not be corrupted in link mode. + + `_ensure_workspace_preflight_image_layer` writes the preflight script to + environment/razorback_spider2_preflight.py. Under `view_mode="link"` the + reflected file is a symlink back into the source tree, so an unguarded + `write_text` would follow the link and overwrite the user's source file — + the same symlink-write-through class fixed for the Dockerfile/task.toml. + """ + source = _write_source( + tmp_path / "source", with_packages=True, with_duckdb=True + ) + source_script = source / "environment" / "razorback_spider2_preflight.py" + source_script_before = "# user's own file, not the generated preflight\n" + source_script.write_text(source_script_before) + + view = materialize_spider2_harbor_task_view( + source_task_dir=source, + view_root=tmp_path / "views", + task_slug="spider2-fixture-001", + view_mode="link", + ) + + # The view owns a real preflight script carrying the generated content. + view_script = view / "environment" / "razorback_spider2_preflight.py" + assert view_script.is_file() + assert not view_script.is_symlink() + assert "def preflight_spider2_workspace" in view_script.read_text() + + # The SOURCE file is byte-for-byte unchanged — no write followed the symlink. + assert source_script.read_text() == source_script_before From fb867705e9c5c523442d59839a2fc3f835bb7e77 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:24:01 +0800 Subject: [PATCH 14/19] docs(spider2-dbt): append cycle-3 implementation stage report (preflight symlink guard) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 0c96c95..237e2e0 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -216,3 +216,16 @@ Captain chose fix-now. Routing back to `implementation`: 1. **[medium] Preflight helper write can still follow a source symlink in link mode (`harbor_view.py:187-188`).** `_ensure_workspace_preflight_image_layer` does `script_path.write_text(preflight_script_text())` on `environment/razorback_spider2_preflight.py` with NO `is_symlink()` guard. Under `view_mode="link"`, if a source task ships a file with that exact name, the view path is a symlink into the source and the write corrupts the user's source file — the same class as the cycle-1 Dockerfile fix. **Fix:** add the same `if script_path.is_symlink(): script_path.unlink()` guard before the helper `write_text` (mirror the Dockerfile/`task.toml` guards). Add a link-mode regression test that SEEDS `environment/razorback_spider2_preflight.py` in the source fixture and proves the source file is unchanged after materialization (fails without the guard). Keep all prior ACs, the rider, and cycle-1/cycle-2 fixes green. + +## Stage Report: implementation (cycle 3 — preflight symlink guard) + +- DONE: Add the `if script_path.is_symlink(): script_path.unlink()` guard immediately before the `script_path.write_text(preflight_script_text())` in `_ensure_workspace_preflight_image_layer` (`harbor_view.py` ~line 187), mirroring the existing Dockerfile/task.toml unlink-then-write guards, so link mode can never corrupt a source-provided `razorback_spider2_preflight.py`. + Guard added before the `write_text` in `harbor_view.py` (commit c0d9da1), with a comment citing the Dockerfile/task.toml precedent. Mirrors `materialize.py:144-145` and the two cycle-1 Dockerfile-helper guards. +- DONE: Add a link-mode regression test that SEEDS `environment/razorback_spider2_preflight.py` in a source fixture, materializes with view_mode="link", and asserts the SOURCE file content is unchanged (and the view file is view-owned). Confirm it FAILS without the guard and passes with it. + `test_link_mode_preflight_script_never_mutates_source_named_file`: seeds the source file, materializes link mode, asserts view script `is_file() and not is_symlink()` carries the generated content while the source is byte-for-byte unchanged. Proven RED before the guard (`assert not is_symlink()` -> True) and GREEN after. +- DONE: Keep all prior ACs, the build-context rider, and the cycle-1 (Dockerfile) + cycle-2 (db_name/schema) fixes green: `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py`. Leave the worktree clean (restore the harness `uv.lock`). + 39 passed, 751 deselected (was 38 + the new regression). Cycle-1 `test_link_mode_injects_layers_but_never_mutates_source_dockerfile` and cycle-2 db_name/schema tests intact. `uv.lock` restored; `git status` shows only the two intended files (now committed). + +### Summary + +Closed the remaining symlink-write-through gap (cycle-2 Codex finding): `_ensure_workspace_preflight_image_layer` wrote `environment/razorback_spider2_preflight.py` with no symlink guard, so a source task shipping a file of that exact name would be corrupted under `view_mode="link"`. Added the identical unlink-then-write guard used for the Dockerfile/task.toml helpers and a seed-the-filename link-mode regression test (proven red-then-green). Focused fix only — no refactor, no regression: full spider2 acceptance is 39 passed, prior cycle-1/cycle-2 fixes intact, `uv.lock` restored, worktree clean. From 6f1688188e1e8e03d6178d7d38155271b992932e Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:31:24 +0800 Subject: [PATCH 15/19] =?UTF-8?q?validation(spider2-dbt):=20cycle=203=20PA?= =?UTF-8?q?SSED=20=E2=80=94=20preflight=20symlink=20guard=20verified=20loa?= =?UTF-8?q?d-bearing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independently re-verified commit c0d9da1: the preflight-helper write guard (is_symlink/unlink before write_text) is real and load-bearing — seeding a colliding source file and materializing in link mode leaves the source byte-for-byte unchanged; stripping the guard corrupts the source and fails the regression test. Full symlink-write-through class now closed (4 guards). No regression: spider2_dbt acceptance 39 passed, generic materializer/leakage unchanged, ade_bench 13 passed, /app contract pinned. Code review: ready to merge, zero blocking findings. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 15 ++++ .../spider2-dbt-harbor-view-ade-parity.md | 68 +++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index 237e2e0..eb11a6f 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -229,3 +229,18 @@ Captain chose fix-now. Routing back to `implementation`: ### Summary Closed the remaining symlink-write-through gap (cycle-2 Codex finding): `_ensure_workspace_preflight_image_layer` wrote `environment/razorback_spider2_preflight.py` with no symlink guard, so a source task shipping a file of that exact name would be corrupted under `view_mode="link"`. Added the identical unlink-then-write guard used for the Dockerfile/task.toml helpers and a seed-the-filename link-mode regression test (proven red-then-green). Focused fix only — no refactor, no regression: full spider2 acceptance is 39 passed, prior cycle-1/cycle-2 fixes intact, `uv.lock` restored, worktree clean. + +## Stage Report: validation (cycle 3 — preflight symlink guard re-review) + +- DONE: Confirm the cycle-3 fix: `_ensure_workspace_preflight_image_layer` now guards the preflight-helper write with `if script_path.is_symlink(): unlink()` before write_text; exercise it — seed a source file named `razorback_spider2_preflight.py`, materialize in link mode, assert the SOURCE is byte-for-byte unchanged and the view script is view-owned. Confirm the regression test fails without the guard. + Guard at `harbor_view.py:210-211`. Exercised via `_write_source` + seeded colliding source file, `view_mode="link"`: view script `is_file() and not is_symlink()` with generated content, SOURCE unchanged (`== SENTINEL`). Stripped only this guard → SOURCE corrupted + `test_link_mode_preflight_script_never_mutates_source_named_file` FAILS at `assert not is_symlink()`; restored → green. +- DONE: Confirm the full symlink-write-through class is closed (Dockerfile + task.toml + preflight helper all guarded) and the cycle-1/cycle-2 fixes are intact (db_name pin, fail-closed multi-DB, schema-aware sources, importable resolver for r5). + 4 `is_symlink()` guards in `harbor_view.py` (3 Dockerfile + preflight script); exhaustive module search (corroborated by code-review agent) finds no remaining unguarded write; `copytree` is view-owned-dir, not write-through. Cycle-1 Dockerfile-guard test + cycle-2 db_name-pin/fail-closed/schema-aware tests all green (10 passed subset); `resolve_spider2_db_name` importable. +- DONE: Confirm no regression: AC-1/2/3 + build-context rider green; `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` (expect ~39 passed); generic materialize.py/leakage.py unchanged. Give a gate verdict (PASSED -> done, or REJECTED -> implementation). + 39 passed, 751 deselected. Generic `materialize.py`/`leakage.py` byte-for-byte unchanged vs `996d42b` (empty diff); ade_bench regression 13 passed; `/app` contract pinned. Pre-existing failures confirmed on base (`razorback.score.load` absent in `996d42b`; 3 named files untouched). `uv.lock` ignored per dispatch. Code review (504c23c..c312719): Ready to merge — Yes, zero blocking findings. +- DONE: Gate verdict. + PASSED → done. Cycle-3 re-review section appended to validation/spider2-dbt-harbor-view-ade-parity.md. + +### Summary + +Independently re-verified the cycle-3 preflight-symlink-guard fix (commit `c0d9da1`, HEAD `c312719`) with no production code written. The guard (`if script_path.is_symlink(): script_path.unlink()` before the preflight-script `write_text`) is real and load-bearing: exercised end-to-end against a seeded colliding source file in link mode, the SOURCE stays byte-for-byte unchanged while the view owns a real file; stripping the guard corrupts the source and fails the regression test. This closes the last instance of the symlink-write-through class — `harbor_view.py` now carries 4 `is_symlink()` guards (3 Dockerfile + preflight script) and an exhaustive search (corroborated by the code-review agent) finds no remaining unguarded write-through site. No regression: acceptance is 39 passed, generic materializer/leakage unchanged, ade_bench 13 passed, the `/app` contract pinned, and cycles 1-2 (db_name pin, fail-closed multi-DB, schema-aware sources, importable r5 resolver, Dockerfile guards) are intact. Pre-existing unrelated failures predate the branch on base `996d42b`. Code review verdict: Ready to merge. **Gate: PASSED → done.** diff --git a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md index 69548b0..a5bf700 100644 --- a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md @@ -332,3 +332,71 @@ difference correct. Both cycle-2 findings are genuinely fixed and load-bearing; the resolver is importable for r5; no regression to AC-1/2/3, the build-context rider, or the cycle-1 unlink fix. **PASSED → `done`.** + +--- + +## Cycle 3 (preflight-helper symlink guard) re-review + +**Range:** `504c23c` (cycle-3 PASSED state) .. `c312719` (HEAD). Fix commit +`c0d9da1`. Independent verification only — no production code written. + +**Context:** A Codex re-review found the symlink-write-through class was not +fully closed: `_ensure_workspace_preflight_image_layer` wrote +`environment/razorback_spider2_preflight.py` with NO `is_symlink()` guard, so a +source task shipping a file of that exact name would be corrupted under +`view_mode="link"` — the same class as the cycle-1 Dockerfile fix. + +### Fix confirmed (the last write-through instance) + +- The guard `if script_path.is_symlink(): script_path.unlink()` is present at + `harbor_view.py:210-211`, immediately before `script_path.write_text(...)` + (line 212), on the exact path written. Mirrors the cycle-1 Dockerfile guards + and `materialize.py:144-145`. +- **Exercised end-to-end** (not text-inspected): built a real source task via + the test's `_write_source` helper, seeded + `environment/razorback_spider2_preflight.py` with a sentinel, materialized + `view_mode="link"`. Result: view script `is_file() and not is_symlink()` with + the generated content; the SOURCE file byte-for-byte unchanged (`== SENTINEL`). +- **Guard proven load-bearing:** stripped ONLY the preflight-script guard → + re-running the same exercise CORRUPTS the source (it gets overwritten with the + full generated preflight content); the regression test + `test_link_mode_preflight_script_never_mutates_source_named_file` FAILS at + `assert not is_symlink()`. Restored → green; tree clean. + +### Full symlink-write-through class closed + +`harbor_view.py` now has 4 `is_symlink()` guards: 3 Dockerfile writes +(build-context, dbt-deps, preflight RUN injection) + the preflight-script write. +An exhaustive search of the spider2_dbt module (confirmed by the code-review +agent) finds no remaining unguarded `write_text`/`open(...,'w')`. The lone +`shutil.copytree` (line 140) writes into a fresh view-owned dir and is not a +write-through risk (the materializer reflects directories as real dirs, only +files as symlinks). + +### No regression (cycles 1-2 intact) + +- `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` + → **39 passed, 751 deselected** (38 + the new regression test). +- Cycle-1 Dockerfile guard test + cycle-2 db_name-pin/fail-closed/schema-aware + tests all present and green (10 passed in the targeted subset). +- Generic `materialize.py`/`leakage.py`: `git diff 996d42b..HEAD` empty — + byte-for-byte unchanged. ade_bench harbor_view + preflight regression → 13 passed. +- `/app` + `/app/.duckdb` contract still pinned (`_APP_ROOT="/app"`, + preflight `--workspace /app --db-name `, COPY `dbt_project/ /app/`). +- Pre-existing unrelated failures confirmed on base: `razorback.score.load` + absent in `996d42b` (the `test_task_identity_scoring` import error); the three + named files untouched by this branch. Harness `uv.lock` ignored per dispatch. + +### Code review + +`superpowers:requesting-code-review` dispatched against `504c23c..c312719`. +Verdict: **Ready to merge — Yes.** Zero Critical / Important findings. One Minor +note (the preflight-script write has no idempotency short-circuit, so a repeat +materialize re-unlinks-and-rewrites identical content) — harmless, explicitly +not worth changing. The reviewer independently confirmed there is no 5th +write-through site and the `copytree` is structurally safe. + +### Gate + +The cycle-3 fix is real and load-bearing, the full symlink-write-through class +is closed, and nothing from cycles 1-2 regressed. **PASSED → `done`.** From 886eac3a1bf45ec74b5f12599d3fe4d61828a201 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:44:29 +0800 Subject: [PATCH 16/19] =?UTF-8?q?feedback:=20validation=20gate=20rejected?= =?UTF-8?q?=20(cycle=204)=20=E2=80=94=20honor=20dbt=20target=20in=20db=5Fn?= =?UTF-8?q?ame=20resolver?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../spider2-dbt-harbor-view-ade-parity.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index eb11a6f..dadf104 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -244,3 +244,15 @@ Closed the remaining symlink-write-through gap (cycle-2 Codex finding): `_ensure ### Summary Independently re-verified the cycle-3 preflight-symlink-guard fix (commit `c0d9da1`, HEAD `c312719`) with no production code written. The guard (`if script_path.is_symlink(): script_path.unlink()` before the preflight-script `write_text`) is real and load-bearing: exercised end-to-end against a seeded colliding source file in link mode, the SOURCE stays byte-for-byte unchanged while the view owns a real file; stripping the guard corrupts the source and fails the regression test. This closes the last instance of the symlink-write-through class — `harbor_view.py` now carries 4 `is_symlink()` guards (3 Dockerfile + preflight script) and an exhaustive search (corroborated by the code-review agent) finds no remaining unguarded write-through site. No regression: acceptance is 39 passed, generic materializer/leakage unchanged, ade_bench 13 passed, the `/app` contract pinned, and cycles 1-2 (db_name pin, fail-closed multi-DB, schema-aware sources, importable r5 resolver, Dockerfile guards) are intact. Pre-existing unrelated failures predate the branch on base `996d42b`. Code review verdict: Ready to merge. **Gate: PASSED → done.** + +### Cycle 4 — validation gate REJECTED (2026-06-18, captain via Codex review) + +Codex found a real correctness bug in the db_name resolver; captain chose +fix-now-then-converge (no further re-review after this). Routing back to +`implementation`: + +1. **[high] profiles.yml resolver ignores the dbt `target:` (`preflight.py` `_read_profiles_db_path`).** + The resolver iterates `outputs.values()` and returns the FIRST DuckDB `path:`, never reading the profile's `target:` field. dbt uses `outputs[target]`, so on a multi-output profile (dev/prod) the resolver can pin the wrong DB — the preflight would validate `/app/.duckdb` while dbt/the agent use the target-selected database. Confirmed live (no `target` reference in the module). Shared with r5, which imports this resolver. + **Fix:** resolve the active output via the profile's `target:` value before returning its `path:`; fail closed if `target` is missing/unknown or the selected output is non-DuckDB. Add a test with `dev`/`prod` outputs where `target` is NOT the first mapping entry, asserting the target output's DB is pinned. Keep all prior ACs + cycle-1/2/3 fixes green. + +Per captain: after this fix re-validates clean, proceed to PR — no further per-fix re-review cycle on ny. From 13efa0b9866b5530693f8ed655f22fac23f1b06c Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:47:27 +0800 Subject: [PATCH 17/19] fix(spider2-dbt): honor dbt target: in db_name resolver, fail closed otherwise The profiles.yml resolver iterated outputs.values() and returned the first DuckDB path:, ignoring the profile's target:. dbt selects outputs[target], so on a multi-output profile (dev/prod) the preflight could pin the wrong DB. Now returns outputs[target]'s DuckDB path; fails closed when several outputs exist but target is missing/unknown (unresolved dbt target) or the target output is non-DuckDB (target output not duckdb). Single-output (no target) and glob fallbacks preserved. Shared resolver -> correctness matters for the r5 verifier. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../benchmarks/spider2_dbt/preflight.py | 95 +++++++++++++++++-- .../test_spider2_dbt_workspace_preflight.py | 70 ++++++++++++++ 2 files changed, 156 insertions(+), 9 deletions(-) diff --git a/src/razorback/benchmarks/spider2_dbt/preflight.py b/src/razorback/benchmarks/spider2_dbt/preflight.py index bfa552d..b669018 100644 --- a/src/razorback/benchmarks/spider2_dbt/preflight.py +++ b/src/razorback/benchmarks/spider2_dbt/preflight.py @@ -96,8 +96,11 @@ def resolve_spider2_db_name(workspace: Path, *, task_slug: str) -> str: resolution so all three agree on which DuckDB the agent operates against. Resolution order: - 1. The dbt `profiles.yml` `path:` value — strip directories and the - `.duckdb` suffix to get the stem. + 1. The dbt `profiles.yml` active output's `path:` value — the active + output is `outputs[target]` (dbt's own selection rule), not whichever + output is listed first. Strip directories and the `.duckdb` suffix to + get the stem. Fails closed when several outputs exist but `target:` is + missing/unknown, or when the target output is non-DuckDB. 2. Exactly one `*.duckdb` already present in the workspace — use its stem. 3. The task slug (used as the DB name when the project ships nothing). @@ -107,7 +110,7 @@ def resolve_spider2_db_name(workspace: Path, *, task_slug: str) -> str: """ workspace = Path(workspace) - profile_path = _read_profiles_db_path(workspace) + profile_path = _read_profiles_db_path(workspace, task_slug=task_slug) if profile_path: return Path(profile_path).name.removesuffix(".duckdb") @@ -128,14 +131,39 @@ def resolve_spider2_db_name(workspace: Path, *, task_slug: str) -> str: return task_slug -def _read_profiles_db_path(workspace: Path) -> str | None: - """Return the first dbt `profiles.yml` DuckDB output `path:`, if any.""" +def _read_profiles_db_path(workspace: Path, *, task_slug: str) -> str | None: + """Return the active dbt `profiles.yml` DuckDB output `path:`, if any. + + dbt selects the active output as `outputs[target]`, so this honors the + profile's `target:` field rather than returning whichever output happens + to be listed first. With multiple outputs the `target:` is REQUIRED and + must name a DuckDB output: + + * a profile with exactly one output uses it (no `target:` needed); + * a profile with several outputs and a `target:` returns + `outputs[target]`'s DuckDB `path:`; + * fail CLOSED (`Spider2WorkspacePreflightError`) when several outputs + exist but `target:` is missing/unknown (`unresolved dbt target`), or + when the target-selected output is not DuckDB (`target output not + duckdb`) — never silently pin the wrong DB. + + Profiles whose active output ships no `.duckdb` `path:` contribute nothing + here, leaving the single-glob / slug fallbacks in `resolve_spider2_db_name` + to take over. + """ try: import yaml except Exception: return None if not workspace.is_dir(): return None + + def _duckdb_path(output: dict[str, Any]) -> str | None: + path = output.get("path") + if isinstance(path, str) and path.strip().endswith(".duckdb"): + return path.strip() + return None + for profiles_path in sorted(workspace.rglob("profiles.yml")): try: document = yaml.safe_load(profiles_path.read_text()) @@ -143,10 +171,59 @@ def _read_profiles_db_path(workspace: Path) -> str | None: continue for profile in _iter_dicts(list(_as_dict(document).values())): outputs = _as_dict(profile.get("outputs")) - for output in _iter_dicts(list(outputs.values())): - path = output.get("path") - if isinstance(path, str) and path.strip().endswith(".duckdb"): - return path.strip() + output_dicts = { + name: out + for name, out in outputs.items() + if isinstance(out, dict) + } + if not output_dicts: + continue + + target = profile.get("target") + if isinstance(target, str) and target.strip(): + target = target.strip() + if target not in output_dicts: + raise Spider2WorkspacePreflightError( + { + "status": "failed", + "reason": "unresolved dbt target", + "task_id": task_slug, + "target": target, + "outputs": sorted(output_dicts), + } + ) + selected = output_dicts[target] + path = _duckdb_path(selected) + if path is None: + raise Spider2WorkspacePreflightError( + { + "status": "failed", + "reason": "target output not duckdb", + "task_id": task_slug, + "target": target, + "type": selected.get("type"), + } + ) + return path + + # No explicit target. A single output is unambiguous; preserve the + # historical single-output fallback. With several outputs and no + # target, dbt cannot pick one either -> fail closed. + if len(output_dicts) == 1: + (only_output,) = output_dicts.values() + path = _duckdb_path(only_output) + if path is not None: + return path + continue + raise Spider2WorkspacePreflightError( + { + "status": "failed", + "reason": "unresolved dbt target", + "task_id": task_slug, + "target": None, + "outputs": sorted(output_dicts), + } + ) return None diff --git a/tests/unit/test_spider2_dbt_workspace_preflight.py b/tests/unit/test_spider2_dbt_workspace_preflight.py index e3751b6..e43d9f0 100644 --- a/tests/unit/test_spider2_dbt_workspace_preflight.py +++ b/tests/unit/test_spider2_dbt_workspace_preflight.py @@ -314,3 +314,73 @@ def test_resolve_db_name_profile_pins_db_among_many(tmp_path: Path) -> None: ) ) assert resolve_spider2_db_name(tmp_path, task_slug="x") == "warehouse" + + +def test_resolve_db_name_honors_target_not_first_output(tmp_path: Path) -> None: + # dbt resolves the active output via `outputs[target]`. With `target: prod` + # but `dev` listed FIRST, the resolver must pin the prod DB, not dev. + # (Old first-output behavior returned dev_warehouse here — wrong DB.) + (tmp_path / "profiles.yml").write_text( + "\n".join( + [ + "example:", + " target: prod", + " outputs:", + " dev:", + " type: duckdb", + " path: dev_warehouse.duckdb", + " prod:", + " type: duckdb", + " path: prod_warehouse.duckdb", + "", + ] + ) + ) + assert resolve_spider2_db_name(tmp_path, task_slug="x") == "prod_warehouse" + + +def test_resolve_db_name_fails_closed_when_target_missing(tmp_path: Path) -> None: + # A profile with outputs but no `target:` cannot be resolved unambiguously. + (tmp_path / "profiles.yml").write_text( + "\n".join( + [ + "example:", + " outputs:", + " dev:", + " type: duckdb", + " path: dev_warehouse.duckdb", + " prod:", + " type: duckdb", + " path: prod_warehouse.duckdb", + "", + ] + ) + ) + with pytest.raises(Spider2WorkspacePreflightError) as exc_info: + resolve_spider2_db_name(tmp_path, task_slug="x") + assert exc_info.value.payload["reason"] == "unresolved dbt target" + + +def test_resolve_db_name_fails_closed_when_target_output_non_duckdb( + tmp_path: Path, +) -> None: + # target points at a postgres output -> the duckdb contract cannot be honored. + (tmp_path / "profiles.yml").write_text( + "\n".join( + [ + "example:", + " target: prod", + " outputs:", + " dev:", + " type: duckdb", + " path: dev_warehouse.duckdb", + " prod:", + " type: postgres", + " host: db", + "", + ] + ) + ) + with pytest.raises(Spider2WorkspacePreflightError) as exc_info: + resolve_spider2_db_name(tmp_path, task_slug="x") + assert exc_info.value.payload["reason"] == "target output not duckdb" From 0caaec5bd6742ec8d4b0b09dc9b3c4fb742e7164 Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:47:53 +0800 Subject: [PATCH 18/19] docs(spider2-dbt): append cycle-4 implementation stage report (honor dbt target) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index dadf104..af5c216 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -256,3 +256,16 @@ fix-now-then-converge (no further re-review after this). Routing back to **Fix:** resolve the active output via the profile's `target:` value before returning its `path:`; fail closed if `target` is missing/unknown or the selected output is non-DuckDB. Add a test with `dev`/`prod` outputs where `target` is NOT the first mapping entry, asserting the target output's DB is pinned. Keep all prior ACs + cycle-1/2/3 fixes green. Per captain: after this fix re-validates clean, proceed to PR — no further per-fix re-review cycle on ny. + +## Stage Report: implementation (cycle 4 — honor dbt target in resolver) + +- DONE: In `_read_profiles_db_path` (`preflight.py`), resolve the active dbt output via the profile's `target:` field before returning its DuckDB `path:` (do NOT return the first output unconditionally). Fail closed if `target` is missing/unknown for a profile, or if the target-selected output is non-DuckDB. Preserve the existing single-output and glob fallbacks for profiles without an explicit target. + Rewrote `_read_profiles_db_path` to read `profile["target"]` and return `outputs[target]`'s DuckDB `path:`. Multi-output + missing/unknown target -> `Spider2WorkspacePreflightError(reason="unresolved dbt target")`; target output non-DuckDB -> `reason="target output not duckdb"`. Single-output-with-no-target still returns its path; non-`.duckdb` active output returns None so the single-glob/slug fallbacks in `resolve_spider2_db_name` still take over. Commit 462cbf2. +- DONE: Add a regression test with a profile that has `dev` and `prod` outputs where `target` is NOT the first mapping entry (e.g. target: prod, outputs ordered dev then prod), asserting the resolver pins the TARGET output's DB. Confirm it fails against the old first-output behavior. + `test_resolve_db_name_honors_target_not_first_output`: `target: prod`, outputs ordered `dev` then `prod`, asserts `prod_warehouse`. Proven RED on the old first-output code (returned `dev_warehouse`), GREEN after the fix. Added two companions: `_fails_closed_when_target_missing` (multi-output, no target) and `_fails_closed_when_target_output_non_duckdb` (target -> postgres), both RED before / GREEN after. +- DONE: Keep all prior ACs, the build-context rider, and the cycle-1/2/3 fixes green: `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py`. The resolver stays importable for r5. Leave the worktree clean (restore harness uv.lock). + 42 passed, 751 deselected (was 39 + 3 new target tests). `resolve_spider2_db_name` importable (verified via `python -c`). `uv.lock` restored; `git status` shows only the two intended (now committed) files. + +### Summary + +Fixed the cycle-4 Codex correctness finding: `_read_profiles_db_path` returned the first DuckDB output `path:` and never read the profile's `target:`, so a multi-output (dev/prod) profile could pin the wrong DB while dbt/the agent use `outputs[target]`. The resolver now selects `outputs[target]` and fails closed (`unresolved dbt target` when target is missing/unknown among multiple outputs; `target output not duckdb` when the target output isn't DuckDB), while preserving the single-output and glob/slug fallbacks for profiles without an explicit target. Three TDD-first regression tests cover the target-not-first, missing-target, and non-DuckDB-target cases (all proven red then green). Focused fix only — no refactor; shared resolver stays importable so the r5 verifier inherits the correct resolution. Acceptance is 42 passed; worktree clean. From 280b6ff1287994c239b989d33613e868c402a81d Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Thu, 18 Jun 2026 18:51:51 +0800 Subject: [PATCH 19/19] =?UTF-8?q?validation(spider2-dbt):=20cycle=204=20PA?= =?UTF-8?q?SSED=20=E2=80=94=20dbt=20target=20resolver=20verified=20load-be?= =?UTF-8?q?aring?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../spider2-dbt-harbor-view-ade-parity.md | 13 +++++ .../spider2-dbt-harbor-view-ade-parity.md | 56 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md index af5c216..b2a273a 100644 --- a/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/spider2-dbt-harbor-view-ade-parity.md @@ -269,3 +269,16 @@ Per captain: after this fix re-validates clean, proceed to PR — no further per ### Summary Fixed the cycle-4 Codex correctness finding: `_read_profiles_db_path` returned the first DuckDB output `path:` and never read the profile's `target:`, so a multi-output (dev/prod) profile could pin the wrong DB while dbt/the agent use `outputs[target]`. The resolver now selects `outputs[target]` and fails closed (`unresolved dbt target` when target is missing/unknown among multiple outputs; `target output not duckdb` when the target output isn't DuckDB), while preserving the single-output and glob/slug fallbacks for profiles without an explicit target. Three TDD-first regression tests cover the target-not-first, missing-target, and non-DuckDB-target cases (all proven red then green). Focused fix only — no refactor; shared resolver stays importable so the r5 verifier inherits the correct resolution. Acceptance is 42 passed; worktree clean. + +## Stage Report: validation (cycle 4 — honor dbt target re-review) + +- DONE: Confirm the cycle-4 fix: `_read_profiles_db_path` now selects `outputs[target]`'s DuckDB path (not the first output), and fails closed when target is missing/unknown across multiple outputs or the target output is non-DuckDB. Exercise it — a dev/prod profile with target NOT first pins the target DB; confirm the regression test fails against first-output behavior. Single-output/no-target and glob/slug fallbacks still work. + Exercised `resolve_spider2_db_name` directly: 7/7 behaviors confirmed (target-not-first → `prod_warehouse`; multi-no-target & unknown-target → `unresolved dbt target`; non-duckdb-target → `target output not duckdb`; single-no-target, single-glob, slug fallbacks all resolve). Load-bearing: reverting `_read_profiles_db_path` to first-output behavior makes all 3 cycle-4 tests FAIL (target-not-first returned wrong DB `dev_warehouse`); reverted patch, `preflight.py` byte-identical to committed. +- DONE: Confirm no regression across all prior cycles: AC-1/2/3, build-context rider, cycle-1 (Dockerfile guard), cycle-2 (db_name pin + fail-closed + schema-aware), cycle-3 (preflight-helper guard). `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` (expect ~42 passed). Generic materialize.py/leakage.py unchanged. resolve_spider2_db_name importable for r5. + 42 passed, 751 deselected (EXIT 0). Cycle-1/2/3 guard+schema+db_name subset 6 passed; 4 `is_symlink()` guards intact. `materialize.py`/`leakage.py` byte-for-byte unchanged vs `996d42b` (empty diff); ade_bench 40 passed/1 skipped. `resolve_spider2_db_name` importable. Pre-existing failures (`razorback.score.load` absent on base; 3 named files untouched by branch) confirmed not regressions; `uv.lock` ignored; worktree clean. +- DONE: Give a gate verdict (PASSED -> done, or REJECTED -> implementation). + PASSED → done. Cycle-4 section appended to validation/spider2-dbt-harbor-view-ade-parity.md. + +### Summary + +Final independent re-verification of the cycle-4 fix (commit `462cbf2`, HEAD `5d16851`) from the clean worktree — no production code written. The fix is real and load-bearing: `_read_profiles_db_path` now returns `outputs[target]`'s DuckDB path (proven by exercising the resolver across 7 cases including target-not-first, missing/unknown target, and non-DuckDB target), and reverting it to the old first-output behavior makes all three cycle-4 regression tests fail. Nothing from cycles 1-3 regressed: acceptance is 42 passed, the 4 symlink guards and schema-aware/db_name-pin fixes are intact, generic `materialize.py`/`leakage.py` are byte-for-byte unchanged, and `resolve_spider2_db_name` stays importable for r5. The pre-existing unrelated failures predate this branch on base `996d42b` and the branch never touches those files. **Gate: PASSED → done.** Ready for PR per captain (no further per-fix re-review). diff --git a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md index a5bf700..611eec9 100644 --- a/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md +++ b/docs/razorback-implementation/validation/spider2-dbt-harbor-view-ade-parity.md @@ -400,3 +400,59 @@ write-through site and the `copytree` is structurally safe. The cycle-3 fix is real and load-bearing, the full symlink-write-through class is closed, and nothing from cycles 1-2 regressed. **PASSED → `done`.** + +## Cycle 4 — final re-review (honor dbt `target:` in db_name resolver) + +Independent re-verification of the cycle-4 fix (commit `462cbf2`, branch HEAD +`5d16851`) from the clean worktree. No production code written — verification +only. Per captain: last re-validation before PR. + +### The fix is real and load-bearing + +`_read_profiles_db_path` (`preflight.py:134-227`) now reads the profile's +`target:` and returns `outputs[target]`'s DuckDB `path:` instead of the first +output. Exercised the importable `resolve_spider2_db_name` directly against +temp profiles — 7/7 behaviors confirmed: + +- `target: prod` with `dev` listed FIRST → pins `prod_warehouse` (NOT `dev`). +- multi-output, no `target:` → fails closed `reason="unresolved dbt target"`. +- `target:` names an unknown output → fails closed `unresolved dbt target`. +- `target:` points at a `postgres` output → fails closed + `reason="target output not duckdb"`. +- single output, no `target:` → still resolves (fallback preserved). +- no profiles + exactly one `*.duckdb` → glob-stem fallback works. +- no profiles + no DB → task-slug fallback works. + +**Regression tests proven load-bearing.** Temporarily reverted +`_read_profiles_db_path` to first-output behavior in the working tree and +re-ran the three cycle-4 tests: all 3 FAILED +(`honors_target_not_first_output`, `fails_closed_when_target_missing`, +`fails_closed_when_target_output_non_duckdb`) — the target-not-first case +returned the wrong DB (`dev_warehouse`) under the old logic. Reverted the +patch; `preflight.py` is byte-identical to committed (`git diff` empty). + +### No regression across cycles 1-3 + +- `uv run pytest -k spider2_dbt --ignore=tests/unit/test_task_identity_scoring.py` + → **42 passed, 751 deselected** (EXIT 0) — was 39 + 3 new target tests. +- Cycle-1 Dockerfile guard, cycle-3 preflight-symlink guard, cycle-2 + schema-aware source + db_name-pin/fail-closed tests → 6 passed targeted subset. + `harbor_view.py` still carries 4 `is_symlink()` guards. +- Generic `materialize.py`/`leakage.py`: `git diff 996d42b..HEAD` empty — + byte-for-byte unchanged. ade_bench harbor_view + preflight → 40 passed, 1 skipped. +- `resolve_spider2_db_name` importable for r5 + (`from razorback.benchmarks.spider2_dbt.preflight import resolve_spider2_db_name`). +- `/app` + `/app/.duckdb` contract still pinned. +- Branch touches only `spider2_dbt/{harbor_view,preflight}.py`, their two test + files, and the entity/validation docs. The three pre-existing-failing test + files (`test_task_identity_scoring`, `test_generate_matrix_specs`, + `test_rk_research_new`) are untouched; `razorback.score.load` is genuinely + absent on base `996d42b` — pre-existing, not a regression. Harness `uv.lock` + ignored per dispatch; worktree clean. + +### Gate + +The cycle-4 resolver fix honors `outputs[target]`, fails closed correctly, and +is proven load-bearing by reverting the regression tests. Cycles 1-3 are +intact, generic surfaces unchanged, acceptance is 42 passed. +**PASSED → `done`.**