diff --git a/canonic/cli/commands/review.py b/canonic/cli/commands/review.py index b5cde64..fea0463 100644 --- a/canonic/cli/commands/review.py +++ b/canonic/cli/commands/review.py @@ -31,7 +31,7 @@ _PENDING_DIFFS_DIR = "pending-diffs" -_ACTIONS = r"\[a]ccept / \[r]eject / \[s]kip / \[f]reeze / \[q]uit" +_ACTIONS = r"\[a]ccept / \[r]eject / \[s]kip / \[f]reeze / \[c]urate / \[q]uit" def _resolve_run_dir(project_root: Path, run_id: str | None) -> Path | None: @@ -56,7 +56,7 @@ def review( ) -> None: """Interactively review pending proposals one by one. - Accepts, rejects, skips, or freezes each proposal. Resumable after quit or crash. + Accepts, rejects, skips, freezes, or curates each proposal. Resumable after quit or crash. """ root = find_project_root() if root is None: @@ -115,8 +115,8 @@ def review( while True: raw = typer.prompt(">", prompt_suffix=" ").strip().lower() - if raw not in {"a", "r", "s", "f", "q"}: - _console.print(f"[red]unknown action {raw!r}[/red]; choose a/r/s/f/q") + if raw not in {"a", "r", "s", "f", "c", "q"}: + _console.print(f"[red]unknown action {raw!r}[/red]; choose a/r/s/f/c/q") continue break @@ -134,6 +134,10 @@ def review( apply_entry(root, run, proposal, freeze=True) new_status = ProposalStatus.FROZEN _console.print(f"[green]frozen[/green] → {proposal.target}") + elif raw == "c": + apply_entry(root, run, proposal, curate=True) + new_status = ProposalStatus.CURATED + _console.print(f"[green]curated[/green] → {proposal.target}") elif raw == "r": new_status = ProposalStatus.REJECTED _console.print(f"[red]rejected[/red] → {proposal.target}") diff --git a/canonic/ingestion/pending.py b/canonic/ingestion/pending.py index 460f2ba..9a71d6f 100644 --- a/canonic/ingestion/pending.py +++ b/canonic/ingestion/pending.py @@ -74,6 +74,7 @@ class ProposalStatus(StrEnum): ACCEPTED = "accepted" REJECTED = "rejected" FROZEN = "frozen" + CURATED = "curated" class PendingProposalEntry(BaseModel): @@ -253,6 +254,7 @@ def apply_entry( entry: PendingProposalEntry, *, freeze: bool = False, + curate: bool = False, ) -> None: """Apply one proposal from ``run`` to the working directory. @@ -260,19 +262,28 @@ def apply_entry( :func:`~canonic.ingestion.pipeline.write_emitted_diffs` function the pipeline uses. When ``freeze=True`` the written file additionally gets ``meta.frozen: true`` set so a subsequent ``canonic ingest`` flags conflicts instead of editing the fact (AC5 / E4 §5.3). - ``freeze`` is silently ignored for PRUNE ops (there is no file left to annotate). + When ``curate=True`` the written file gets ``meta.provenance: human_curated`` set — the + only system-mediated way for a fact to be promoted out of ``inferred`` + (AMENDMENT-provenance-promotion, S17). ``curate`` and ``freeze`` are independent and may + both be set. Both are silently ignored for PRUNE ops (there is no file left to annotate). """ from canonic.ingestion.models import ProposalOp from canonic.ingestion.pipeline import write_emitted_diffs from canonic.semantic.loader import dump_semantic_source, load_semantic_source + from canonic.semantic.models import Provenance diff = run.diff_for(entry) write_emitted_diffs(project_root, [diff]) - if freeze and diff.op is not ProposalOp.PRUNE: + if (freeze or curate) and diff.op is not ProposalOp.PRUNE: path = project_root / diff.target if path.exists() and diff.target.endswith(".yaml"): source = load_semantic_source(path) - updated_meta = source.meta.model_copy(update={"frozen": True}) - frozen_source = source.model_copy(update={"meta": updated_meta}) - path.write_text(dump_semantic_source(frozen_source)) + meta_updates: dict[str, object] = {} + if freeze: + meta_updates["frozen"] = True + if curate: + meta_updates["provenance"] = Provenance.HUMAN_CURATED + updated_meta = source.meta.model_copy(update=meta_updates) + updated_source = source.model_copy(update={"meta": updated_meta}) + path.write_text(dump_semantic_source(updated_source)) diff --git a/canonic/semantic/models.py b/canonic/semantic/models.py index 9491efd..1a27358 100644 --- a/canonic/semantic/models.py +++ b/canonic/semantic/models.py @@ -189,7 +189,11 @@ class FinalityMeta(BaseModel): class SourceMeta(BaseModel): - """System-managed provenance metadata, not hand-edited.""" + """System-managed provenance metadata. + + Written only by ``canonic ingest`` (sets ``inferred``) or ``canonic review``/``apply`` + via ``curate``/``freeze`` (AMENDMENT-provenance-promotion) — never edited directly. + """ model_config = ConfigDict(frozen=True) diff --git a/docs/cli-reference/review-apply.mdx b/docs/cli-reference/review-apply.mdx index abc3d50..7b1f104 100644 --- a/docs/cli-reference/review-apply.mdx +++ b/docs/cli-reference/review-apply.mdx @@ -21,9 +21,12 @@ canonic review --run-id 2024-05-01T12-00-00 For each pending proposal you're shown the diff and prompted: ``` -[a]ccept / [r]eject / [s]kip / [f]reeze / [q]uit +[a]ccept / [r]eject / [s]kip / [f]reeze / [c]urate / [q]uit ``` +- **freeze** writes the file and sets `meta.frozen: true` — a subsequent `canonic ingest` flags any conflicting evidence instead of editing the fact, regardless of tier. +- **curate** writes the file and sets `meta.provenance: human_curated` — the only way a fact gets promoted out of `inferred`. A curated fact is protected from future `inferred` evidence the same way any other `human_curated` fact is (see [provenance tiers](/concepts/ingestion-and-reconciliation#reconciliation)), but unlike `freeze` it isn't an unconditional lock: equally-or-more-authoritative future evidence can still propose an edit. + Resumable; quitting or crashing mid-review picks back up at the first still-pending item on the next invocation. ## `canonic apply` @@ -38,4 +41,4 @@ canonic apply .canonic/pending-diffs/2024-05-01T12-00-00 | --- | --- | | `run_dir` | Path to the pending-diff run directory to apply (positional). | -Proposals already in a terminal state (accepted / rejected / frozen), or whose diff file was deleted, are silently skipped. No git interaction; applied files appear as unstaged changes in your working tree. +Proposals already in a terminal state (accepted / rejected / frozen / curated), or whose diff file was deleted, are silently skipped. No git interaction; applied files appear as unstaged changes in your working tree. `canonic apply` doesn't expose `freeze` or `curate` — every applied proposal resolves to `accepted`; use `canonic review` for either. diff --git a/docs/concepts/connectors.mdx b/docs/concepts/connectors.mdx index c2a5f7b..2efdabe 100644 --- a/docs/concepts/connectors.mdx +++ b/docs/concepts/connectors.mdx @@ -82,7 +82,7 @@ When live introspection is unavailable or partial, canonic descends a priority o 5. **Sample-based inference** *(not yet implemented)*. 6. **Hand-authored `semantics/*.yaml`**: validated against the live source before being trusted (below). -Acquisition tier and provenance are independent axes. A tier-2 `dbt` import still enters [reconciliation](/concepts/ingestion-and-reconciliation#reconciliation) at provenance `inferred`, exactly like raw introspection, so it never displaces a `human_curated` or `board_approved` fact. Its acquisition tier only breaks ties **within** the `inferred` band: when a modeling-code definition and a raw-introspection fact describe the same relation without disagreeing on type, the modeling-code evidence is preferred. Structural fields (`grain`, `joins`, `measures`) still always require review regardless of confidence. See a [worked example](/concepts/ingestion-and-reconciliation#example-adding-a-definition-connector-after-an-initial-apply). +Acquisition tier and provenance are independent axes. A tier-2 `dbt` import still enters [reconciliation](/concepts/ingestion-and-reconciliation#reconciliation) at provenance `inferred`, exactly like raw introspection, so it never displaces a `human_curated` or `board_approved` fact. Its acquisition tier only breaks ties **within** the `inferred` band: when a modeling-code definition and a raw-introspection fact describe the same relation without disagreeing on type, the modeling-code evidence is preferred. Structural fields (`grain`, `joins`, `measures`) still always require review regardless of confidence. See a [worked example](/concepts/ingestion-and-reconciliation#example-adding-a-definition-connector-after-an-initial-apply). A fact only reaches `human_curated` through an explicit [`canonic review curate`](/cli-reference/review-apply), never automatically from a higher acquisition tier. Partial capability is never silent: if only some relations are introspectable, the gap is reported rather than omitted. diff --git a/docs/concepts/ingestion-and-reconciliation.mdx b/docs/concepts/ingestion-and-reconciliation.mdx index 4b27ca2..6ee7e90 100644 --- a/docs/concepts/ingestion-and-reconciliation.mdx +++ b/docs/concepts/ingestion-and-reconciliation.mdx @@ -51,13 +51,15 @@ Merges each proposal against the currently accepted file with a deterministic de **Provenance tiers:** `board_approved > human_curated > inferred`. Higher always wins: since new evidence is always `inferred`, it can never silently displace a curated or approved fact. A **frozen** fact is never edited by reconciliation regardless of the incoming tier or confidence; conflicting evidence is flagged, not applied. +Provenance only ever rises through a deliberate human action: [`canonic review curate`](/cli-reference/review-apply) sets `meta.provenance: human_curated` on the resulting fact. Accepting a proposal never does this on its own — an accepted fact stays `inferred` until explicitly curated. (`canonic apply`, the batch path, doesn't expose `curate` yet — same as it doesn't expose `freeze`.) + The `answer_outcome` row is a deliberate exception to that tier logic: it always reconciles to a contradiction, never to an add or edit, no matter how low-confidence or high-tier the target binding is. This is an attribution safeguard, feedback should never silently rewrite a definition; a human always reviews it. See [Feedback loop (E11)](#feedback-loop-e11) below. A conflict is never resolved by silent overwrite. The worst case is a flagged contradiction a human resolves: contradictions ride into the review surface (a PR comment in headless mode) and don't fail a run by default. ### Example: adding a definition connector after an initial apply -Say a connection starts out queryable-only. `canonic ingest` runs [live introspection](/concepts/connectors#schema-acquisition-ladder) and proposes semantic sources at provenance `inferred`, the tier every new proposal starts at. `canonic apply` accepts them into `semantics/*.yaml`. Accepting a file this way commits it, but doesn't curate it: it's still `inferred`. +Say a connection starts out queryable-only. `canonic ingest` runs [live introspection](/concepts/connectors#schema-acquisition-ladder) and proposes semantic sources at provenance `inferred`, the tier every new proposal starts at. `canonic apply` accepts them into `semantics/*.yaml`. Accepting a file this way commits it, but doesn't curate it: it's still `inferred` — promoting it to `human_curated` takes an explicit [`canonic review curate`](/cli-reference/review-apply). Now a `dbt` connector is added to the same connection and `canonic ingest` runs again. `extract_definitions` produces definition evidence from the compiled manifest, an acquisition tier above raw introspection, but it still enters reconciliation at provenance `inferred`, same as the existing fact. Provenance and acquisition tier are independent: the two proposals disagreeing on the same relation is not a "existing tier higher" case and not a silent win either way. Instead, the modeling-code evidence is preferred as the tie-break within the `inferred` band, so the run resolves to the "conflicts, tier ≤ proposal" row above: propose an **edit**. That diff lands under `.canonic/pending-diffs//` and goes through [`canonic review`](/cli-reference/review-apply) / [`canonic apply`](/cli-reference/review-apply) exactly like the first round. If the edit touches `grain`, `joins`, or `measures`, it's ineligible for auto-apply no matter how confident the proposal is, per the `never` list under [Propose-only by default](#propose-only-by-default) below. @@ -95,4 +97,4 @@ Re-running `canonic ingest` refreshes from all configured sources, but a source ## After a run -Diffs land under `.canonic/pending-diffs//`. Walk through them interactively with [`canonic review`](/cli-reference/review-apply), or batch-apply everything still pending with [`canonic apply`](/cli-reference/review-apply). +Diffs land under `.canonic/pending-diffs//`. Walk through them interactively with [`canonic review`](/cli-reference/review-apply) — accept, reject, skip, freeze, or curate each one — or batch-apply everything still pending with [`canonic apply`](/cli-reference/review-apply). diff --git a/docs/end-to-end-example.mdx b/docs/end-to-end-example.mdx index 4e30ed9..f8301a1 100644 --- a/docs/end-to-end-example.mdx +++ b/docs/end-to-end-example.mdx @@ -108,7 +108,7 @@ Seven tables didn't change: same `source_fingerprint`, so nothing is proposed be The deterministic builder didn't just notice the new column — it drafted an additive `sum` measure for it too, `confidence: 1.0`. It's still `propose-only`: this project's `canonic.yaml` has `reconcile.auto_apply.enabled: false` (the default), and structural fields like new measures never auto-apply regardless of confidence. `canonic review` shows the same diff and prompt as before; accept it, and `damages.yaml` now has `waived_amount` and `total_waived_amount`. - If `damages.yaml` had instead been `human_curated` (hand-edited after the wizard, as [the tracked rental example](https://github.com/mischuh/canonic/tree/main/examples/rental) is), this same run would **not** propose a clean edit. A curated file always outranks new `inferred` evidence — reconciliation flags a **contradiction** and leaves the file untouched, for every table whose drafted shape no longer matches the hand-curated one, not just the changed table. See [provenance tiers](/concepts/ingestion-and-reconciliation#reconciliation). + If `damages.yaml` had instead been `human_curated` (promoted with [`canonic review curate`](/cli-reference/review-apply) after the wizard, as [the tracked rental example](https://github.com/mischuh/canonic/tree/main/examples/rental) is), this same run would **not** propose a clean edit. A curated file always outranks new `inferred` evidence — reconciliation flags a **contradiction** and leaves the file untouched, for every table whose drafted shape no longer matches the curated one, not just the changed table. See [provenance tiers](/concepts/ingestion-and-reconciliation#reconciliation). At this point `waived_amount` is a real, queryable measure — but a raw `(source, measure)` pair, not yet a canonical metric. diff --git a/tests/cli/test_review.py b/tests/cli/test_review.py index 8f586fd..ad23436 100644 --- a/tests/cli/test_review.py +++ b/tests/cli/test_review.py @@ -120,7 +120,7 @@ def test_actions_menu_renders_literal_brackets(self, project: Path) -> None: result = CliRunner().invoke(app, ["review"], input="q\n") assert result.exit_code == 0, result.output - assert "[a]ccept / [r]eject / [s]kip / [f]reeze / [q]uit" in result.output + assert "[a]ccept / [r]eject / [s]kip / [f]reeze / [c]urate / [q]uit" in result.output def test_proposal_line_renders_literal_brackets(self, project: Path) -> None: _write_run(project) @@ -236,6 +236,28 @@ def test_freeze_updates_status_to_frozen(self, project: Path) -> None: assert run.proposals[0].status is ProposalStatus.FROZEN +class TestReviewCurate: + def test_curate_writes_human_curated_provenance(self, project: Path) -> None: + """S17: curate writes the file and sets meta.provenance=human_curated.""" + from canonic.semantic.loader import load_semantic_source + + _write_run(project) + result = CliRunner().invoke(app, ["review"], input="c\n") + + assert result.exit_code == 0, result.output + target = project / "semantics" / "warehouse_pg" / "table_1.yaml" + assert target.exists() + source = load_semantic_source(target) + assert source.meta.provenance is Provenance.HUMAN_CURATED + + def test_curate_updates_status_to_curated(self, project: Path) -> None: + run_dir = _write_run(project) + CliRunner().invoke(app, ["review"], input="c\n") + + run = PendingRun.load(run_dir) + assert run.proposals[0].status is ProposalStatus.CURATED + + class TestReviewNothingPending: def test_exits_cleanly_when_all_resolved(self, project: Path) -> None: _write_run(project) diff --git a/tests/ingestion/test_pending.py b/tests/ingestion/test_pending.py index 70c150c..d54851c 100644 --- a/tests/ingestion/test_pending.py +++ b/tests/ingestion/test_pending.py @@ -525,3 +525,71 @@ def test_ac5_frozen_fact_flags_contradiction_on_reingest(self, tmp_path: Path) - report = engine.reconcile([conflicting_proposal], accepted_store) assert any(e.decision is ReconciliationDecision.CONTRADICTION for e in report.entries) + + def test_curate_writes_human_curated_provenance(self, tmp_path: Path) -> None: + """S17: curate writes meta.provenance=human_curated (AMENDMENT-provenance-promotion).""" + from canonic.config import scaffold_project + from canonic.semantic.loader import load_semantic_source + + scaffold_project(tmp_path) + emission = _emission_with_valid_source() + run_dir = PendingDiffStore(tmp_path).write("run1", emission) + run = PendingRun.load(run_dir) + + apply_entry(tmp_path, run, run.proposals[0], curate=True) + + target = tmp_path / run.proposals[0].target + assert target.exists() + source = load_semantic_source(target) + assert source.meta.provenance is Provenance.HUMAN_CURATED + assert source.meta.frozen is False + + def test_curated_fact_flags_contradiction_on_reingest(self, tmp_path: Path) -> None: + """S18: after curate, conflicting inferred evidence yields CONTRADICTION not EDIT.""" + from canonic.config import ReconcileConfig, scaffold_project + from canonic.ingestion.models import ReconciliationDecision + from canonic.ingestion.reconciliation import ( + DiskAcceptedStore, + NullReconcileDrafter, + ReconciliationEngine, + ) + + scaffold_project(tmp_path) + emission = _emission_with_valid_source() + run_dir = PendingDiffStore(tmp_path).write("run1", emission) + run = PendingRun.load(run_dir) + + apply_entry(tmp_path, run, run.proposals[0], curate=True) + + target_str = run.proposals[0].target + accepted_store = DiskAcceptedStore(tmp_path) + fact = accepted_store.get(target_str) + assert fact is not None + assert fact.provenance is Provenance.HUMAN_CURATED + + conflicting_proposal = _proposal( + target=target_str, + op=ProposalOp.EDIT, + content={**_VALID_SOURCE_CONTENT, "name": "different_orders"}, + ) + engine = ReconciliationEngine(ReconcileConfig(), NullReconcileDrafter()) + report = engine.reconcile([conflicting_proposal], accepted_store) + + assert any(e.decision is ReconciliationDecision.CONTRADICTION for e in report.entries) + + def test_curate_and_freeze_compose(self, tmp_path: Path) -> None: + """S19: curate and freeze are independent and may both be set in one apply.""" + from canonic.config import scaffold_project + from canonic.semantic.loader import load_semantic_source + + scaffold_project(tmp_path) + emission = _emission_with_valid_source() + run_dir = PendingDiffStore(tmp_path).write("run1", emission) + run = PendingRun.load(run_dir) + + apply_entry(tmp_path, run, run.proposals[0], curate=True, freeze=True) + + target = tmp_path / run.proposals[0].target + source = load_semantic_source(target) + assert source.meta.provenance is Provenance.HUMAN_CURATED + assert source.meta.frozen is True