From 92e27d4485cff168e454965fe0c7c53d61475506 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 10 Nov 2026 22:59:27 +0000 Subject: [PATCH 1/2] feat(plan): include each changed model's diffs in plan JSON sqb plan --json now carries the query diff, config diff and schema findings that the text plan prints under a changed model, so other tools can show the same detail. The text plan reads the same diffs and only colours them. A plan read from the server drops the query and config diffs for someone who may not read SQL on every node, as it already did for Python diffs. A location drops model diffs from a plan that would otherwise exceed the server's size limit, so a large change still plans. --- .../src/native_commands/constants.rs | 3 ++ .../native_commands/main/_location_plan.rs | 23 +++------ .../native_commands/main/_plan_document.rs | 38 ++++++++++++++ .../src/native_commands/main/mod.rs | 1 + .../sqlbuild-cli/src/native_commands/tests.rs | 2 + .../src/native_commands/tests/helpers.rs | 9 ++++ .../tests/test_location_plan.rs | 39 ++++++++++++++ .../src/native_commands/tests/test_types.rs | 8 +++ .../src/platform/main/read_person_plan.rs | 25 +++++---- .../tests/api/test_person_plans.rs | 6 ++- .../_helpers/plan_output/plan_json.rs | 26 ++++++++++ .../_helpers/plan_output/plan_text.rs | 51 +++++++++++++------ .../output/main/plan_json/test_plan_json.py | 19 +++++++ website/src/content/docs/docs/cli/plan.mdx | 2 +- 14 files changed, 210 insertions(+), 42 deletions(-) create mode 100644 crates/sqlbuild-cli/src/native_commands/main/_plan_document.rs create mode 100644 crates/sqlbuild-cli/src/native_commands/tests/test_location_plan.rs diff --git a/crates/sqlbuild-cli/src/native_commands/constants.rs b/crates/sqlbuild-cli/src/native_commands/constants.rs index bd2eb9b498..6406a3a58b 100644 --- a/crates/sqlbuild-cli/src/native_commands/constants.rs +++ b/crates/sqlbuild-cli/src/native_commands/constants.rs @@ -106,3 +106,6 @@ pub(crate) const PASSWORD_INPUT_BYTES: u64 = 4096; /// The command a location runs to answer a plan request. pub(crate) const PLAN_COMMAND: &str = "plan"; +/// How much `sqb plan --json` output a location reads; model diffs over the server's cap are dropped. +pub(crate) const PLAN_OUTPUT_BYTES: usize = + 8 * sqlbuild_orchestration::platform::constants::PLAN_DOCUMENT_BYTES; diff --git a/crates/sqlbuild-cli/src/native_commands/main/_location_plan.rs b/crates/sqlbuild-cli/src/native_commands/main/_location_plan.rs index aee88ba2d4..30b540040a 100644 --- a/crates/sqlbuild-cli/src/native_commands/main/_location_plan.rs +++ b/crates/sqlbuild-cli/src/native_commands/main/_location_plan.rs @@ -5,15 +5,15 @@ use std::future::Future; use std::path::Path; use std::process::Stdio; -use serde_json::{Value, json}; -use sqlbuild_orchestration::platform::constants::PLAN_DOCUMENT_BYTES; +use serde_json::Value; use sqlbuild_orchestration::store::errors::ControlError; use tokio::io::AsyncReadExt; use tokio::process::Child; use crate::argv::main::accepted_settings_arguments::accepted_settings_arguments; use crate::native_commands::_helpers::execution::location_child; -use crate::native_commands::constants::PLAN_COMMAND; +use crate::native_commands::constants::{PLAN_COMMAND, PLAN_OUTPUT_BYTES}; +use crate::native_commands::main::_plan_document::{failure, plan_document}; use crate::native_commands::models::LocationServe; /// The plan document, an `{"error": ...}` report when planning failed, or `None` when stopped first. @@ -31,7 +31,7 @@ pub(super) async fn plan_location( .take() .ok_or_else(|| ControlError::Invalid("plan output is unavailable".into()))?; let mut output: Vec = Vec::new(); - let limit = u64::try_from(PLAN_DOCUMENT_BYTES).unwrap_or(u64::MAX) + 1; + let limit = u64::try_from(PLAN_OUTPUT_BYTES).unwrap_or(u64::MAX) + 1; tokio::pin!(stopped); let status = tokio::select! { result = &mut stopped => { @@ -46,13 +46,10 @@ pub(super) async fn plan_location( child.wait().await } => result?, }; - if output.len() > PLAN_DOCUMENT_BYTES { - return Ok(Some(failure("the plan is too large to show"))); - } - Ok(Some(match serde_json::from_slice::(&output) { - Ok(document) if document.is_object() && matches!(status.code(), Some(0 | 1)) => document, - _ => failure("sqb plan did not produce a plan; see the location's log"), - })) + Ok(Some(plan_document( + &output, + matches!(status.code(), Some(0 | 1)), + ))) } /// The started child, or why the frozen request cannot become plan arguments. @@ -83,7 +80,3 @@ fn plan_child( .stdout(Stdio::piped()) .spawn()) } - -fn failure(message: &str) -> Value { - json!({"error": {"code": "plan_failed", "message": message, "help": null}}) -} diff --git a/crates/sqlbuild-cli/src/native_commands/main/_plan_document.rs b/crates/sqlbuild-cli/src/native_commands/main/_plan_document.rs new file mode 100644 index 0000000000..58428ee462 --- /dev/null +++ b/crates/sqlbuild-cli/src/native_commands/main/_plan_document.rs @@ -0,0 +1,38 @@ +//! Reading `sqb plan --json` output as the plan a location sends to the server. + +use serde_json::{Value, json}; +use sqlbuild_orchestration::platform::constants::PLAN_DOCUMENT_BYTES; + +use crate::native_commands::constants::PLAN_OUTPUT_BYTES; + +/// The plan `sqb plan --json` printed, without model diffs when they would put it over the server's cap. +pub(in crate::native_commands) fn plan_document(output: &[u8], planned: bool) -> Value { + if output.len() > PLAN_OUTPUT_BYTES { + return failure("the plan is too large to show"); + } + let mut document = match serde_json::from_slice::(output) { + Ok(document) if document.is_object() && planned => document, + _ => return failure("sqb plan did not produce a plan; see the location's log"), + }; + if output.len() <= PLAN_DOCUMENT_BYTES { + return document; + } + let models = document.get_mut("models").and_then(Value::as_array_mut); + for model in models + .into_iter() + .flatten() + .filter_map(Value::as_object_mut) + { + model.remove("query_diff"); + model.remove("config_diff"); + } + match serde_json::to_vec(&document) { + Ok(bytes) if bytes.len() <= PLAN_DOCUMENT_BYTES => document, + _ => failure("the plan is too large to show"), + } +} + +/// The `{"error": ...}` report a location sends when it has no plan to show. +pub(in crate::native_commands) fn failure(message: &str) -> Value { + json!({"error": {"code": "plan_failed", "message": message, "help": null}}) +} diff --git a/crates/sqlbuild-cli/src/native_commands/main/mod.rs b/crates/sqlbuild-cli/src/native_commands/main/mod.rs index d308f7d5b8..813d58cd45 100644 --- a/crates/sqlbuild-cli/src/native_commands/main/mod.rs +++ b/crates/sqlbuild-cli/src/native_commands/main/mod.rs @@ -4,6 +4,7 @@ pub(super) mod _location_metadata; pub(super) mod _location_plan; pub(super) mod _location_sensor; pub(super) mod _location_work; +pub(super) mod _plan_document; pub(super) mod _run_admin; pub(super) mod _run_control; pub(super) mod _run_cost; diff --git a/crates/sqlbuild-cli/src/native_commands/tests.rs b/crates/sqlbuild-cli/src/native_commands/tests.rs index 2f86f371e1..36ed1676b5 100644 --- a/crates/sqlbuild-cli/src/native_commands/tests.rs +++ b/crates/sqlbuild-cli/src/native_commands/tests.rs @@ -6,6 +6,8 @@ mod decision; mod helpers; #[path = "tests/test_init.rs"] mod init; +#[path = "tests/test_location_plan.rs"] +mod location_plan; #[path = "tests/test_paths.rs"] mod paths; #[path = "tests/test_project_facts.rs"] diff --git a/crates/sqlbuild-cli/src/native_commands/tests/helpers.rs b/crates/sqlbuild-cli/src/native_commands/tests/helpers.rs index e54ee1dd6d..ec817e0ae9 100644 --- a/crates/sqlbuild-cli/src/native_commands/tests/helpers.rs +++ b/crates/sqlbuild-cli/src/native_commands/tests/helpers.rs @@ -26,3 +26,12 @@ pub(super) fn write_tree(root: &Path, directories: &[&str], files: &[(&str, &str std::fs::write(root.join(relative), contents).expect("write file"); } } + +/// `sqb plan --json` output naming `count` changed models, each with `diff_line` as its query diff. +pub(super) fn plan_output(diff_line: &str, count: usize) -> Vec { + let models: Vec = (0..count) + .map(|i| serde_json::json!({"name": format!("model_{i}"), "reason": "query_changed", "query_diff": [diff_line], "config_diff": ["+a"]})) + .collect(); + serde_json::to_vec(&serde_json::json!({"selected_count": count, "models": models})) + .expect("plan output") +} diff --git a/crates/sqlbuild-cli/src/native_commands/tests/test_location_plan.rs b/crates/sqlbuild-cli/src/native_commands/tests/test_location_plan.rs new file mode 100644 index 0000000000..b74e5070e1 --- /dev/null +++ b/crates/sqlbuild-cli/src/native_commands/tests/test_location_plan.rs @@ -0,0 +1,39 @@ +use serde_json::{Value, json}; +use sqlbuild_orchestration::platform::constants::PLAN_DOCUMENT_BYTES; + +use crate::native_commands::main::_plan_document::plan_document; +use crate::native_commands::tests::helpers::plan_output; +use crate::native_commands::tests::test_types::PlanDocumentTestCase; + +#[test] +fn given_plan_output_when_reading_it_on_a_location_then_only_oversized_diffs_are_dropped() { + let test_cases = [ + PlanDocumentTestCase { + description: "a plan under the cap keeps its diffs", + output: plan_output("+SELECT 2", 2), + planned: true, + expected_first_model: json!({"name": "model_0", "reason": "query_changed", "query_diff": ["+SELECT 2"], "config_diff": ["+a"]}), + }, + PlanDocumentTestCase { + description: "diffs over the cap are dropped and the plan is kept", + output: plan_output(&"x".repeat(1024), PLAN_DOCUMENT_BYTES / 1024 + 1), + planned: true, + expected_first_model: json!({"name": "model_0", "reason": "query_changed"}), + }, + PlanDocumentTestCase { + description: "a failed sqb plan is not read as a plan", + output: plan_output("+SELECT 2", 1), + planned: false, + expected_first_model: Value::Null, + }, + ]; + for test_case in test_cases { + let document: Value = plan_document(&test_case.output, test_case.planned); + assert_eq!( + document["models"][0], + test_case.expected_first_model, + "{description}", + description = test_case.description + ); + } +} diff --git a/crates/sqlbuild-cli/src/native_commands/tests/test_types.rs b/crates/sqlbuild-cli/src/native_commands/tests/test_types.rs index 252ef24b95..93270cf91a 100644 --- a/crates/sqlbuild-cli/src/native_commands/tests/test_types.rs +++ b/crates/sqlbuild-cli/src/native_commands/tests/test_types.rs @@ -81,3 +81,11 @@ pub(super) struct InitFailureTestCase { pub(super) expected_stderr_start: &'static str, pub(super) expected_exit_code: i32, } + +/// One `sqb plan --json` output, whether it exited as a plan, and the first model the server receives. +pub(super) struct PlanDocumentTestCase { + pub(super) description: &'static str, + pub(super) output: Vec, + pub(super) planned: bool, + pub(super) expected_first_model: serde_json::Value, +} diff --git a/crates/sqlbuild-orchestration/src/platform/main/read_person_plan.rs b/crates/sqlbuild-orchestration/src/platform/main/read_person_plan.rs index 6dd96a14dc..5d24032915 100644 --- a/crates/sqlbuild-orchestration/src/platform/main/read_person_plan.rs +++ b/crates/sqlbuild-orchestration/src/platform/main/read_person_plan.rs @@ -11,7 +11,7 @@ use crate::store::errors::ControlError; use crate::store::main::{begin::begin, commit::commit}; use crate::store::models::{ControlStore, TransactionMode}; -/// Python source and dependency diffs stay out of the plan unless the reader may also read SQL on every current node. +/// SQL, config, Python source and dependency diffs stay out unless the reader may read SQL on every current node. pub async fn read_person_plan( store: &ControlStore, (context, principal_id): (&AuditContext, Uuid), @@ -41,12 +41,12 @@ pub async fn read_person_plan( .await?; commit(transaction).await?; if !sql { - without_identity_diffs(&mut record); + without_code_diffs(&mut record); } Ok(Some(record)) } -fn without_identity_diffs(record: &mut LocationWorkRecord) { +fn without_code_diffs(record: &mut LocationWorkRecord) { let Some(LocationWorkOutcome::Plan { document }) = record .result_json .as_mut() @@ -54,12 +54,19 @@ fn without_identity_diffs(record: &mut LocationWorkRecord) { else { return; }; - let nodes = document - .get_mut("python_nodes") - .and_then(serde_json::Value::as_array_mut); - for node in nodes.into_iter().flatten() { - if let Some(node) = node.as_object_mut() { - node.remove("identity_diff"); + for (section, keys) in [ + ("python_nodes", &["identity_diff"][..]), + ("models", &["query_diff", "config_diff"][..]), + ] { + let entries = document + .get_mut(section) + .and_then(serde_json::Value::as_array_mut); + for entry in entries.into_iter().flatten() { + if let Some(entry) = entry.as_object_mut() { + for key in keys { + entry.remove(*key); + } + } } } } diff --git a/crates/sqlbuild-orchestration/tests/api/test_person_plans.rs b/crates/sqlbuild-orchestration/tests/api/test_person_plans.rs index 424004c9ef..a774eac04b 100644 --- a/crates/sqlbuild-orchestration/tests/api/test_person_plans.rs +++ b/crates/sqlbuild-orchestration/tests/api/test_person_plans.rs @@ -304,7 +304,7 @@ fn given_person_plans_when_requested_claimed_and_read_then_they_stay_location_wo ); } let document = json!({ - "models": [{"name": "orders", "action": "create_table", "reason": "first_run"}], + "models": [{"name": "orders", "action": "create_table", "reason": "first_run", "query_diff": ["-SELECT 1", "+SELECT 2"], "config_diff": ["-a", "+b"]}], "warnings": [{"severity": "warning", "message": "connect failed: db_password=orders-pass"}], "python_nodes": [{"name": "orders_export", "identity_diff": {"source_diff": ["-a", "+b"]}}], }); @@ -356,7 +356,9 @@ fn given_person_plans_when_requested_claimed_and_read_then_they_stay_location_wo description = test_case.description ); assert!( - document["python_nodes"][0].get("identity_diff").is_none(), + document["python_nodes"][0].get("identity_diff").is_none() + && document["models"][0].get("query_diff").is_none() + && document["models"][0].get("config_diff").is_none(), "{description}: viewers lack view.sql", description = test_case.description ); diff --git a/crates/sqlbuild-python/src/bindings/_helpers/plan_output/plan_json.rs b/crates/sqlbuild-python/src/bindings/_helpers/plan_output/plan_json.rs index a9755d48f3..94b43956a1 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/plan_output/plan_json.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/plan_output/plan_json.rs @@ -10,6 +10,7 @@ use crate::bindings::_helpers::cursor::scalars::sentinel_to_token; use crate::bindings::_helpers::plan_output::cursor_details::{ CursorDetails, cursor_details, owner, }; +use crate::bindings::_helpers::plan_output::plan_text::{model_config_diff, model_query_diff}; use crate::bindings::_helpers::plan_output::python_identity::{ definition_source_text, dependency_source_text, }; @@ -231,6 +232,31 @@ fn model_json<'py>(entry: &Bound<'py, PyAny>) -> PyResult> { if !qualified.is_none() { model.set_item("qualified_name", qualified)?; } + let reason: String = text(&native_field(&native_field(entry, "reason")?, "value")?)?; + if let Some(diff) = model_query_diff(entry, &reason)? { + model.set_item("query_diff", diff)?; + } + if let Some(diff) = model_config_diff(entry, &reason)? { + model.set_item("config_diff", diff)?; + } + let findings: Bound<'py, PyAny> = native_field(entry, "schema_findings")?; + if findings.is_truthy()? { + model.set_item( + "schema_findings", + mapped(&findings, |finding| { + Ok(dict( + py, + vec![ + ("kind", enum_value(finding, "kind")?), + ("column", native_field(finding, "column_name")?), + ("expected_type", native_field(finding, "expected_type")?), + ("actual_type", native_field(finding, "actual_type")?), + ], + )? + .into_any()) + })?, + )?; + } let hints: Bound<'py, PyAny> = native_field(entry, "column_rename_hints")?; if hints.is_truthy()? { model.set_item( diff --git a/crates/sqlbuild-python/src/bindings/_helpers/plan_output/plan_text.rs b/crates/sqlbuild-python/src/bindings/_helpers/plan_output/plan_text.rs index e53da1d79b..568bf822dc 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/plan_output/plan_text.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/plan_output/plan_text.rs @@ -151,7 +151,11 @@ fn config_json<'py>(metadata_json: &Bound<'py, PyAny>) -> PyResult, reason: &str) -> PyResult>> { +/// The plain unified diff of a model's config since its last build, when its config changed. +pub(crate) fn model_config_diff( + entry: &Bound<'_, PyAny>, + reason: &str, +) -> PyResult>> { if !native_field(entry, "config_changed")?.is_truthy()? && reason != CONFIG_CHANGED { return Ok(None); } @@ -166,7 +170,35 @@ fn config_diff(entry: &Bound<'_, PyAny>, reason: &str) -> PyResult()?, + ¤t.extract::()?, + ) + .map(Some) +} + +/// The plain unified diff of a changed model's query since its last build, when that query is known. +pub(crate) fn model_query_diff( + entry: &Bound<'_, PyAny>, + reason: &str, +) -> PyResult>> { + let previous: Bound<'_, PyAny> = native_field(entry, "previous_query_sql")?; + if previous.is_none() + || (!native_field(entry, "query_changed")?.is_truthy()? && reason != QUERY_CHANGED) + { + return Ok(None); + } + unified_diff( + entry.py(), + &previous.extract::()?, + &native_field(entry, "fingerprint_query_sql")?.extract::()?, + ) + .map(Some) +} + +fn coloured(diff: Option>) -> Option> { + diff.map(|lines| format_query_diff(&lines, CliStyle { use_color: true })) } fn model_text(entry: &Bound<'_, PyAny>) -> PyResult { @@ -198,17 +230,6 @@ fn model_text(entry: &Bound<'_, PyAny>) -> PyResult { message: text(&native_field(&hint, "message")?)?, }); } - let previous_query: Bound<'_, PyAny> = native_field(entry, "previous_query_sql")?; - let query_diff_lines: Option> = if previous_query.is_none() - || (!native_field(entry, "query_changed")?.is_truthy()? && reason != QUERY_CHANGED) - { - None - } else { - Some(query_diff( - &previous_query, - &native_field(entry, "fingerprint_query_sql")?, - )?) - }; Ok(ModelPlanText { materialization_label: model_materialization_label(entry)?, custom_materialization_name: optional_text(&native_field( @@ -223,8 +244,8 @@ fn model_text(entry: &Bound<'_, PyAny>) -> PyResult { changed_functions: texts(&native_field(entry, "changed_functions")?)?, schema_findings, rename_hints, - config_diff: config_diff(entry, &reason)?, - query_diff: query_diff_lines, + config_diff: coloured(model_config_diff(entry, &reason)?), + query_diff: coloured(model_query_diff(entry, &reason)?), name, action, reason, diff --git a/tests/unit/src/sqlbuild/cli/output/main/plan_json/test_plan_json.py b/tests/unit/src/sqlbuild/cli/output/main/plan_json/test_plan_json.py index 19192c4d82..e2e0f2e647 100644 --- a/tests/unit/src/sqlbuild/cli/output/main/plan_json/test_plan_json.py +++ b/tests/unit/src/sqlbuild/cli/output/main/plan_json/test_plan_json.py @@ -462,6 +462,25 @@ def test_given_direct_diagnostics_policy_when_formatting_plan_json_then_evidence expected_keys=("models",), expected_fragments=('"reason": "renamed"', '"query_changed": true'), ), + JsonOutputTestCase( + description="plan json includes the query diff a changed model prints", + plan_output=build_plan_output( + model_entries=( + build_model_entry( + name="fact_orders", + action=PlanAction.CREATE_TABLE, + reason=PlanReason.QUERY_CHANGED, + previous_query_sql="SELECT id FROM orders", + ), + ), + ), + expected_keys=("models",), + expected_fragments=( + '"query_diff": [', + '"-SELECT id FROM orders"', + '"+SELECT * FROM fact_orders"', + ), + ), JsonOutputTestCase( description="plan json includes seeds and warnings", plan_output=build_plan_output( diff --git a/website/src/content/docs/docs/cli/plan.mdx b/website/src/content/docs/docs/cli/plan.mdx index 8fb761fc2a..44514fdb97 100644 --- a/website/src/content/docs/docs/cli/plan.mdx +++ b/website/src/content/docs/docs/cli/plan.mdx @@ -109,7 +109,7 @@ Seeds (1) When query, function, or schema changes are detected, the plan lists each affected model under its own change with its backfill action. A model that calls a changed function directly appears under `Function changed` with `cause: function changed`; models further downstream continue forward. See [Replay decisions](/docs/concepts/planning/replay-decisions). -In `sqb plan --json`, each model's `reason` is its own change. A direct caller of a changed function has `"reason": "function_changed"` and a `changed_functions` list, and a renamed model whose query also changed has `"reason": "renamed"` and `"query_changed": true`. +In `sqb plan --json`, each model's `reason` is its own change. A direct caller of a changed function has `"reason": "function_changed"` and a `changed_functions` list, and a renamed model whose query also changed has `"reason": "renamed"` and `"query_changed": true`. A changed model also carries the detail the text plan prints under it: `query_diff` when its query changed and the previous query is known, and `config_diff` when its config changed, each as unified diff lines, and `schema_findings` for added, removed, or retyped columns. Warnings and maintenance notices, such as `migrate_from can be removed`, are listed in a `Warnings` section before the final `Plan complete` line. `sqb plan --json` output is unchanged. From 07c3f7a84f089ec1ed37338a10a9b08c70ce53b4 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 00:32:42 +0000 Subject: [PATCH 2/2] fix(plan): keep one entry function per plan module and refresh the plan docs skill Move the plan failure report into the shared execution helpers so the plan document module exposes only plan_document, and regenerate the bundled docs skill page for the new plan JSON keys. --- .../src/native_commands/_helpers/execution.rs | 5 +++++ .../src/native_commands/main/_location_plan.rs | 5 +++-- .../src/native_commands/main/_plan_document.rs | 14 +++++--------- .../skills/sqlbuild/references/docs/cli/plan.md | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/crates/sqlbuild-cli/src/native_commands/_helpers/execution.rs b/crates/sqlbuild-cli/src/native_commands/_helpers/execution.rs index b7ea9fa268..a7a8b314c4 100644 --- a/crates/sqlbuild-cli/src/native_commands/_helpers/execution.rs +++ b/crates/sqlbuild-cli/src/native_commands/_helpers/execution.rs @@ -980,3 +980,8 @@ pub(crate) fn run_command( output, )) } + +/// The `{"error": ...}` report a location sends when it has no plan to show. +pub(crate) fn plan_failure(message: &str) -> serde_json::Value { + json!({"error": {"code": "plan_failed", "message": message, "help": null}}) +} diff --git a/crates/sqlbuild-cli/src/native_commands/main/_location_plan.rs b/crates/sqlbuild-cli/src/native_commands/main/_location_plan.rs index 30b540040a..0f6ed68932 100644 --- a/crates/sqlbuild-cli/src/native_commands/main/_location_plan.rs +++ b/crates/sqlbuild-cli/src/native_commands/main/_location_plan.rs @@ -12,8 +12,9 @@ use tokio::process::Child; use crate::argv::main::accepted_settings_arguments::accepted_settings_arguments; use crate::native_commands::_helpers::execution::location_child; +use crate::native_commands::_helpers::execution::plan_failure; use crate::native_commands::constants::{PLAN_COMMAND, PLAN_OUTPUT_BYTES}; -use crate::native_commands::main::_plan_document::{failure, plan_document}; +use crate::native_commands::main::_plan_document::plan_document; use crate::native_commands::models::LocationServe; /// The plan document, an `{"error": ...}` report when planning failed, or `None` when stopped first. @@ -24,7 +25,7 @@ pub(super) async fn plan_location( ) -> Result, ControlError> { let mut child = match plan_child((options, script), (selection, settings)) { Ok(child) => child?, - Err(message) => return Ok(Some(failure(&message))), + Err(message) => return Ok(Some(plan_failure(&message))), }; let stdout = child .stdout diff --git a/crates/sqlbuild-cli/src/native_commands/main/_plan_document.rs b/crates/sqlbuild-cli/src/native_commands/main/_plan_document.rs index 58428ee462..fd194c52cb 100644 --- a/crates/sqlbuild-cli/src/native_commands/main/_plan_document.rs +++ b/crates/sqlbuild-cli/src/native_commands/main/_plan_document.rs @@ -1,18 +1,19 @@ //! Reading `sqb plan --json` output as the plan a location sends to the server. -use serde_json::{Value, json}; +use serde_json::Value; use sqlbuild_orchestration::platform::constants::PLAN_DOCUMENT_BYTES; +use crate::native_commands::_helpers::execution::plan_failure; use crate::native_commands::constants::PLAN_OUTPUT_BYTES; /// The plan `sqb plan --json` printed, without model diffs when they would put it over the server's cap. pub(in crate::native_commands) fn plan_document(output: &[u8], planned: bool) -> Value { if output.len() > PLAN_OUTPUT_BYTES { - return failure("the plan is too large to show"); + return plan_failure("the plan is too large to show"); } let mut document = match serde_json::from_slice::(output) { Ok(document) if document.is_object() && planned => document, - _ => return failure("sqb plan did not produce a plan; see the location's log"), + _ => return plan_failure("sqb plan did not produce a plan; see the location's log"), }; if output.len() <= PLAN_DOCUMENT_BYTES { return document; @@ -28,11 +29,6 @@ pub(in crate::native_commands) fn plan_document(output: &[u8], planned: bool) -> } match serde_json::to_vec(&document) { Ok(bytes) if bytes.len() <= PLAN_DOCUMENT_BYTES => document, - _ => failure("the plan is too large to show"), + _ => plan_failure("the plan is too large to show"), } } - -/// The `{"error": ...}` report a location sends when it has no plan to show. -pub(in crate::native_commands) fn failure(message: &str) -> Value { - json!({"error": {"code": "plan_failed", "message": message, "help": null}}) -} diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/plan.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/plan.md index 38771ec77c..fc8cb69d9c 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/plan.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/plan.md @@ -119,7 +119,7 @@ Seeds (1) When query, function, or schema changes are detected, the plan lists each affected model under its own change with its backfill action. A model that calls a changed function directly appears under `Function changed` with `cause: function changed`; models further downstream continue forward. See [Replay decisions](../concepts/planning/replay-decisions.md). -In `sqb plan --json`, each model's `reason` is its own change. A direct caller of a changed function has `"reason": "function_changed"` and a `changed_functions` list, and a renamed model whose query also changed has `"reason": "renamed"` and `"query_changed": true`. +In `sqb plan --json`, each model's `reason` is its own change. A direct caller of a changed function has `"reason": "function_changed"` and a `changed_functions` list, and a renamed model whose query also changed has `"reason": "renamed"` and `"query_changed": true`. A changed model also carries the detail the text plan prints under it: `query_diff` when its query changed and the previous query is known, and `config_diff` when its config changed, each as unified diff lines, and `schema_findings` for added, removed, or retyped columns. Warnings and maintenance notices, such as `migrate_from can be removed`, are listed in a `Warnings` section before the final `Plan complete` line. `sqb plan --json` output is unchanged.