Multi-hop promotion fix + CONTRIBUTING + aspirational ROADMAP

CONTRIBUTING.md

@@ -0,0 +1,57 @@
+# Contributing to sediment
+
+Thanks for your interest. sediment is a small project with a strong point of view, so this guide is short but opinionated.
+
+## The one rule that governs everything
+
+**Deterministic core, AI at the edges.** The transform path (ingest → staging → marts) is plain, tested, reproducible dbt SQL. AI participates at three seams only: proposing models (`scaffold`), explaining runs (`orchestrate`), and answering questions (`ask` / the dashboard). Contributions that put an LLM inside the deterministic transform path will be declined regardless of how clever they are — that boundary is the product.
+
+## Getting set up
+
+```bash
+git clone https://github.com/camharris93/sediment.git
+cd sediment
+pip install -r requirements.txt
+python run.py up          # full offline build: download → load → profile → dbt run → dbt test
+python run.py dashboard   # http://localhost:8501
+```
+
+No API key is needed for the core data flow or the test suite. The AI seams need an Anthropic key (see `.env.example`).
+
+## Before you open a PR
+
+```bash
+ruff check .
+python -m pytest -m "not live"
+```
+
+Both must be green. CI runs the same suite on Linux and Windows across Python 3.10 and 3.13, plus a from-scratch `run.py up` build and the replay evals — so a change that only works on your machine, with your warehouse state, will fail there.
+
+## Security-sensitive areas
+
+The read-only SQL guard (`engine/query/execution.py`) is a real security boundary: chat-generated SQL runs against it. If you change the guard or anything that feeds it:
+
+1. **Write the attack first.** Add the motivating adversarial case to `tests/test_read_only_guard.py` before the fix — statement chaining, file-reading table functions, extension loads, and friends all have precedent there.
+2. Prefer structural rules (e.g. "no FROM source backed by a function, allowlist excepted") over blocklists of known-bad names.
+
+The same spirit applies to `engine/modeling.py` (chat → dbt model promotion) and path handling in `engine/config.py`.
+
+## What contributions are welcome
+
+- **Bug fixes with a failing test.** The test is the contribution; the fix is the follow-through.
+- **Ingest/profile coverage** for new tabular formats and messier real-world files.
+- **Eval questions.** New golden questions in `evals/golden_questions.yaml` with result invariants make the NL→SQL agent measurably better or expose where it isn't.
+- **Docs and quickstart friction.** If step 3 confused you, that confusion is a bug report.
+- **Reference datasets/patterns** that run offline on synthetic or public data. If it can't run on a laptop with fake data, it doesn't belong here.
+
+For anything larger, open an issue first — see [ROADMAP.md](ROADMAP.md) for where the project is headed, and remember the audience: solo and small-team practitioners. Features that only make sense for a twelve-person platform team are out of scope.
+
+## Style
+
+- Python: `ruff` is the arbiter; match the surrounding code otherwise.
+- SQL/dbt: every model states what one row means. If you can't write that sentence, the model isn't done.
+- Windows matters: this project develops and CIs on Windows and Linux both. Avoid Unicode in console `print()` and hardcoded `/` path assumptions.
+
+## License
+
+MIT. By contributing you agree your contributions are licensed under the same terms.

README.md

@@ -413,6 +413,8 @@ python evals/harness.py    # replay the NL→SQL golden questions through L3–L
   cross-platform CI matrix, hardened read-only guard (blocks local-file reads / multi-statement /
   file writes), and `pip install` packaging (the `sediment` command + `sediment init`)
 
+Where it goes from here: **[ROADMAP.md](ROADMAP.md)**. Want to help: **[CONTRIBUTING.md](CONTRIBUTING.md)**.
+
 ## Decisions taken (PRD §11 open questions)
 
 - **Visualization → Streamlit** for v1: pure-Python, guaranteed-green offline, no

ROADMAP.md

@@ -0,0 +1,58 @@
+# sediment — Roadmap
+
+sediment today (v0.2) is a dataset-agnostic analytics stack in a box: point it at tabular data and one command builds warehouse → tested dbt models → dashboard, offline, with AI at exactly three seams — proposing models (`scaffold`), explaining runs (`orchestrate`), and answering questions (`ask`) — and never inside the deterministic transform path.
+
+This document is deliberately **aspirational**. It describes where the project wants to go, in order of intent, not a schedule. Versions past v0.3 are directions, and real usage gets to reorder them — a feature nobody pulls for is a feature this roadmap will happily drop.
+
+---
+
+## v0.3 — The accumulation layer
+
+*Theme: answers should leave something behind.*
+
+Most analytics work evaporates: the question gets answered, the context gets lost, and six months later nobody can say what the number meant or why. v0.3 adds the small, boring artifacts that make work accumulate instead.
+
+- **Grain tests (`assert_grain`).** Every table declares, in one sentence, what one row means — "one row per customer per month" — and a ~15-line test fails the build the moment that stops being true. This sounds trivial; it prevents the single most common analytics disaster, the silently duplicated join that double-counts everything downstream. You find out at build time, not in the meeting.
+- **Metric registry.** One small YAML file per metric, in git: the definition in plain English, who owns it, since when, what changed and why — and a mandatory *honest caveats* section (what the metric can't see, where it misleads). A `sediment validate` command checks the registry in CI. The point: definitions live in version control, not in someone's head, and a metric that claims no weaknesses is refused on principle.
+- **Spec and decision-log templates.** Markdown you can steal one file at a time: a scoring-system spec with a structural caveats section, a metric-change RFC, an append-only "why is it like this" log — so "why does churn exclude trials?" is answered by a dated entry, not an annual meeting.
+
+## v0.4 — Point it at anything
+
+*Theme: useful against the stack you already have, not just the one sediment builds.*
+
+- **`sediment doctor`.** Run it against an *existing* warehouse and dbt project and get a health report: tables with no declared grain, joins at risk of fan-out, untested models, metrics computed in two places with two answers, row counts drifting week over week. Today sediment shines on greenfield builds; `doctor` makes it valuable in the first hour against a stack you inherited.
+- **More sources.** Databases and APIs, not just files — with scheduled refresh, so a sediment workspace can stay current without a human re-running ingest.
+- **Installable patterns.** `sediment add pattern growth-accounting` drops a complete, runnable reference implementation — growth accounting (new/retained/resurrected/churned), funnel & retention, composite health scoring — each on synthetic data, each a working example of the conventions rather than a blog post about them.
+
+## v0.5 — Sentinel
+
+*Theme: the pipeline watches itself.*
+
+- **`orchestrate` becomes a standing watch.** Scheduled runs across one or many workspaces: it builds, tests, explains any failure in plain English with the offending rows, flags drift and anomalies, and notifies you (or opens an issue) instead of waiting to be run. The difference between a pipeline you check and a pipeline that tells you.
+- **A published trust ledger.** The golden-question eval suite runs on every release and the accuracy numbers get published with it. If an AI is going to answer questions about your data, its error rate should be a number you can look up, not a vibe.
+
+## v0.6 — Ask, everywhere
+
+*Theme: the trust architecture becomes the interface.*
+
+- **Shareable Ask.** A governed, read-only link where a stakeholder asks questions in English and every answer arrives with its SQL, its validation trace, and its trust badge. The server-side read-only guarantee already exists; this makes it a surface you'd hand to a non-technical colleague without flinching.
+- **MCP server.** Expose the warehouse to any AI agent *through* the L1–L7 validation ladder and the read-only guard, instead of handing it a raw connection. "Let the agent query the database" is the obvious future and the obvious nightmare; sediment's whole trust stack is the difference between the two.
+- **Registry-aware answers.** The agent cites canonical definitions from the metric registry and declines to invent new ones on the fly. When it's asked about "active users," it answers with *your* definition — caveats included.
+
+## v1.0 — The layered stack
+
+*Theme: the thesis, realized in software.*
+
+- **Conversation becomes convention.** Every promoted model, accepted definition, and logged decision accumulates into the workspace's memory. The agent builds on last month's answers instead of rediscovering them; ask a question twice a quarter apart and the second answer knows about the first. Analytics work that leaves layers — not as a document about the idea, but as default behavior.
+- **Portability where pulled.** DuckDB-native today, and DuckDB goes further than most people think. Adapters for other warehouses arrive when real usage demands them, not before.
+
+## Beyond — the north star
+
+**The one-person data platform.** A single practitioner credibly runs many data stacks at once: `scaffold` does the first draft of the building, sentinel does the watching, Ask does the answering, and the accumulated layers do the remembering — while the human does the part that's actually human: judgment. The stack a small team deserves, without the platform team.
+
+## What this roadmap will not do
+
+- Put AI in the deterministic transform path. Ever. That boundary is the product.
+- Grow enterprise governance features — committees, stewardship workflows, forty-page policies.
+- Chase features that only make sense for large platform teams. The audience is the practitioner carrying the whole stack alone.
+- Treat this document as a commitment device. It's a compass; pull requests and real usage are the map.

dashboard/app.py

@@ -439,9 +439,9 @@ def _store_response(resp) -> dict:
                 # Make the answer available to the Build tab (self-contained SQL) and
                 # the chart seeder.
                 if resp.kind == "answer" and resp.mh and resp.mh.sql:
-                    from engine.query.grounding import inline_synthetic_ctes
-                    ctx_syn = session.base_ctx().with_synthetic_tables(session.prior_synthetics())
-                    inlined = inline_synthetic_ctes(resp.mh.sql, ctx_syn)
+                    # Use the just-appended turn so promotion SQL includes
+                    # within-turn hop deps, but not the answer's own turn alias.
+                    inlined = session.self_contained_sql_for_turn(session.turns[-1])
                     st.session_state.last_answer = {"q": msg, "sql": inlined}
                     if is_build_mode() and st.button("📈 Chart this in the Report"):
                         st.session_state.report_seed = {"sql": inlined, "instruction": msg, "title": msg[:60]}

engine/query/agent.py

@@ -96,10 +96,11 @@ def handle_message(session: Session, message: str, *, build_mode: bool,
                                      "app (`python run.py dashboard`) to build.")
         # Make the SQL self-contained: inline any conversational/hop synthetic
         # tables (turn_N_result / hop_N_result) it references, so the committed
-        # model has no dangling refs.
-        from .grounding import inline_synthetic_ctes
-        ctx_syn = session.base_ctx().with_synthetic_tables(session.prior_synthetics())
-        sql = inline_synthetic_ctes(last.result.sql, ctx_syn)
+        # model has no dangling refs. Exclude the answer's own turn alias.
+        sql = session.self_contained_sql_for_turn(last)
+        if not sql:
+            return ChatResponse(kind="error",
+                                text="Build failed: the last answer has no SQL to promote.")
         sample = last.result.rows[:8]
         name = cls.get("model_name") or _auto_name(last.question)
         mode = cls.get("mode") or "save"

engine/query/conversation.py

@@ -101,6 +101,36 @@ def refresh(self) -> None:
     def prior_synthetics(self) -> list[TableInfo]:
         return [t.synthetic for t in self.turns if t.synthetic is not None]
 
+    def synthetics_before(self, turn_index: int) -> list[TableInfo]:
+        """Synthetic result tables from turns before `turn_index`.
+
+        This is used when promoting a specific answer into a model. The answer's
+        own turn-level synthetic (`turn_<N>_result`) is just an alias for the
+        answer itself, so including it while inlining that same answer can create
+        duplicate/shadowing CTE names. Earlier turns remain valid dependencies.
+        """
+        return [t.synthetic for t in self.turns
+                if t.index < turn_index and t.synthetic is not None]
+
+    def self_contained_sql_for_turn(self, turn: Turn) -> str | None:
+        """Return executable SQL for a turn with all synthetic deps inlined.
+
+        A promoted model must not reference ephemeral `hop_<N>_result` or
+        conversation `turn_<N>_result` tables. For a multi-hop turn, the display
+        SQL can still reference earlier hop outputs; include those hop synthetics
+        explicitly, plus any prior-turn synthetics the question depended on.
+        """
+        sql = getattr(turn.result, "sql", None)
+        if not sql:
+            return None
+        hop_synthetics = [
+            h.synthetic for h in getattr(turn.result, "hops", [])
+            if getattr(h, "synthetic", None) is not None
+        ]
+        ctx = self.base_ctx().with_synthetic_tables(
+            self.synthetics_before(turn.index) + hop_synthetics)
+        return inline_synthetic_ctes(sql, ctx)
+
     def history_for_prompt(self) -> list[dict[str, Any]]:
         """Compact per-turn summaries the intent/planner layers embed so they can
         resolve references like 'those', 'now just X', 'that ranking'."""

tests/test_conversation.py

@@ -0,0 +1,111 @@
+from __future__ import annotations
+
+from types import SimpleNamespace
+
+import duckdb
+
+from engine.query.conversation import Session, Turn
+from engine.query.grounding import ColumnInfo, GroundingContext, TableInfo
+
+
+def _synthetic(name: str, sql: str, columns: list[ColumnInfo] | None = None) -> TableInfo:
+    return TableInfo(
+        schema="",
+        name=name,
+        fully_qualified=name,
+        row_count=1,
+        columns=columns or [ColumnInfo("player_id", "INTEGER", False)],
+        is_synthetic=True,
+        synthetic_sql=sql,
+    )
+
+
+def test_self_contained_sql_for_turn_inlines_multi_hop_deps_without_self_alias():
+    """Promoting a multi-hop answer must not leave dangling hop_N_result refs.
+
+    This mirrors the Build-tab failure mode: the final/display SQL defines its
+    own turn_1_result CTE, but still reads hop_1_result from an earlier hop.
+    The session also has a current-turn synthetic named turn_1_result; that alias
+    represents the answer itself and must not be injected into the answer SQL.
+    """
+    hop_1 = _synthetic(
+        "hop_1_result",
+        "SELECT 1 AS player_id, 'A Player' AS player_name, 42 AS pts",
+        [
+            ColumnInfo("player_id", "INTEGER", False),
+            ColumnInfo("player_name", "VARCHAR", False),
+            ColumnInfo("pts", "INTEGER", False),
+        ],
+    )
+    display_sql = """
+WITH turn_1_result AS (
+    SELECT 1 AS player_id
+),
+base AS (
+    SELECT h.player_id, h.player_name, h.pts
+    FROM hop_1_result h
+    INNER JOIN turn_1_result t
+        ON h.player_id = t.player_id
+)
+SELECT * FROM base
+"""
+    result = SimpleNamespace(sql=display_sql, hops=[SimpleNamespace(synthetic=hop_1)])
+    # This is the current answer's own turn-level alias. If the promotion helper
+    # used session.prior_synthetics() wholesale, it would inject this and create
+    # a duplicate/shadowing turn_1_result CTE.
+    current_turn_alias = _synthetic("turn_1_result", "SELECT 999 AS player_id")
+    turn = Turn(index=1, question="multi-hop question", result=result,
+                synthetic=current_turn_alias)
+    session = Session(dataset="nba", turns=[turn],
+                      _base_ctx=GroundingContext(schemas=[], tables=[], relationships=[]))
+
+    promoted = session.self_contained_sql_for_turn(turn)
+
+    assert promoted is not None
+    assert "hop_1_result AS (" in promoted
+    assert "SELECT 999 AS player_id" not in promoted
+    assert promoted.lower().count("turn_1_result as") == 1
+
+    rows = duckdb.connect().execute(promoted).fetchall()
+    assert rows == [(1, "A Player", 42)]
+
+
+def test_self_contained_sql_for_turn_inlines_prior_turn_and_hop_deps():
+    prior = _synthetic(
+        "turn_1_result",
+        "SELECT 7 AS player_id",
+        [ColumnInfo("player_id", "INTEGER", False)],
+    )
+    prior_turn = Turn(
+        index=1,
+        question="top players",
+        result=SimpleNamespace(sql="SELECT 7 AS player_id", hops=[]),
+        synthetic=prior,
+    )
+    hop_1 = _synthetic(
+        "hop_1_result",
+        "SELECT 7 AS player_id, 12 AS pts",
+        [ColumnInfo("player_id", "INTEGER", False), ColumnInfo("pts", "INTEGER", False)],
+    )
+    current_sql = """
+SELECT h.player_id, h.pts
+FROM hop_1_result h
+INNER JOIN turn_1_result t
+    ON h.player_id = t.player_id
+"""
+    current = Turn(
+        index=2,
+        question="score those players",
+        result=SimpleNamespace(sql=current_sql, hops=[SimpleNamespace(synthetic=hop_1)]),
+        synthetic=_synthetic("turn_2_result", "SELECT 123 AS should_not_inline"),
+    )
+    session = Session(dataset="nba", turns=[prior_turn, current],
+                      _base_ctx=GroundingContext(schemas=[], tables=[], relationships=[]))
+
+    promoted = session.self_contained_sql_for_turn(current)
+
+    assert promoted is not None
+    assert "hop_1_result AS (" in promoted
+    assert "turn_1_result AS (" in promoted
+    assert "SELECT 123 AS should_not_inline" not in promoted
+    assert duckdb.connect().execute(promoted).fetchall() == [(7, 12)]