diff --git a/.changepacks/changepack_log_export_actionability.json b/.changepacks/changepack_log_export_actionability.json new file mode 100644 index 00000000..18cf6624 --- /dev/null +++ b/.changepacks/changepack_log_export_actionability.json @@ -0,0 +1,7 @@ +{ + "changes": { + "crates/devup-mcp/Cargo.toml": "Minor" + }, + "note": "Make export and feature_trace answers easier to act on: feature_trace reads an explicit componentPath first; a frame link with frameIds retries against its parent Section; add summary and reviewChecklist; add `fields` to narrow rawSnapshot/rawPayload/sourceMap; large debug outputs travel as resources under auto delivery; sourceMap entries carry the line-height calculation", + "date": "2026-10-02T12:41:10.3830670Z" +} diff --git a/crates/devup-mcp-devup-ui/src/codegen/component.rs b/crates/devup-mcp-devup-ui/src/codegen/component.rs index 4a104a85..651b2540 100644 --- a/crates/devup-mcp-devup-ui/src/codegen/component.rs +++ b/crates/devup-mcp-devup-ui/src/codegen/component.rs @@ -1220,6 +1220,7 @@ fn finalize_codegen_output( } } output.source_map.describe_properties(&output.tsx); + output.source_map.describe_conversions(snapshot); crate::provenance::attributes::audit_properties(snapshot, &mut output, options, root_id); output.fidelity_report = validate_fidelity(snapshot, root_id, &output)?; Ok(output) diff --git a/crates/devup-mcp-devup-ui/src/provenance.rs b/crates/devup-mcp-devup-ui/src/provenance.rs index 1effe3ee..3a4ba970 100644 --- a/crates/devup-mcp-devup-ui/src/provenance.rs +++ b/crates/devup-mcp-devup-ui/src/provenance.rs @@ -35,6 +35,10 @@ pub struct ProvenanceEntry { /// Internal renderer/validator bookkeeping; never a consumer-facing offset. #[serde(skip)] pub generated_range: Option, + /// How a generated value was computed from the raw one, when that is not a + /// plain copy (e.g. a percent line-height written as px). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub calculation: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub generated_property: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -122,6 +126,50 @@ impl SourceMap { } } + /// Explain unit conversions the generator performs, so a reader can + /// reproduce `lineHeight="26px"` from the raw `{unit: PERCENT, value: 160}`. + pub(crate) fn describe_conversions(&mut self, snapshot: &devup_mcp_figma::Snapshot) { + for entry in &mut self.entries { + if entry.property.as_deref() != Some("lineHeight") { + continue; + } + let Some(node) = entry + .node_id + .as_deref() + .and_then(|id| snapshot.nodes.get(id)) + else { + continue; + }; + let view = node.typed_view(); + let Some(line_height) = view.value("lineHeight") else { + continue; + }; + if line_height["unit"] != "PERCENT" { + continue; + } + let percent = + (line_height["value"].as_f64().unwrap_or_default() * 100.0).round() / 100.0; + let bound = view + .value("boundVariables") + .and_then(|b| b.get("fontSize")) + .is_some(); + entry.calculation = Some( + match ( + view.value("fontSize").and_then(serde_json::Value::as_f64), + bound, + ) { + (Some(size), false) => format!( + "round(fontSize * percent / 100) = round({size} * {percent} / 100) = {}px", + (size * percent / 100.0).round() + ), + _ => format!( + "{percent}% kept as a percentage because fontSize is variable-bound or unknown" + ), + }, + ); + } + } + pub fn empty() -> Self { Self { version: 2, @@ -1452,6 +1500,7 @@ pub(crate) fn finalize_tsx( continue; }; entries.push(ProvenanceEntry { + calculation: None, generated_property: None, generated_range: Some(range.clone()), json_pointer: None, @@ -1698,6 +1747,7 @@ pub(crate) fn finalize_tsx( && let Some((start, end)) = asset_prop_range(opening) { entries.push(ProvenanceEntry { + calculation: None, generated_property: None, generated_range: Some(GeneratedRange { start: range.start + open_relative + start, @@ -1827,6 +1877,7 @@ fn add_flattened_resource_entries( let source = &tsx[range.start..range.end]; if let Some((start, end)) = asset_range_in_node_source(source) { entries.push(ProvenanceEntry { + calculation: None, generated_property: None, generated_range: Some(GeneratedRange { start: range.start + start, @@ -2268,6 +2319,7 @@ fn generated_entry( resolution: &str, ) -> ProvenanceEntry { ProvenanceEntry { + calculation: None, generated_property: None, generated_range: Some(GeneratedRange { start, end }), json_pointer: None, @@ -2335,3 +2387,51 @@ fn strip_markers(marked: &str) -> (String, Vec<(String, GeneratedRange)>) { pub(crate) fn json_pointer_segment(value: &str) -> String { value.replace('~', "~0").replace('/', "~1") } + +#[cfg(test)] +mod line_height_calculation_tests { + use super::*; + use serde_json::json; + + fn entry(node: &str) -> ProvenanceEntry { + ProvenanceEntry { + calculation: None, + generated_property: Some("lineHeight=\"26px\"".into()), + generated_range: None, + json_pointer: None, + node_id: Some(node.into()), + property: Some("lineHeight".into()), + variable_id: None, + style_id: None, + asset_id: None, + resolution: "raw-fallback".into(), + } + } + + #[test] + fn percent_line_height_records_how_px_was_derived() { + let snapshot: devup_mcp_figma::Snapshot = serde_json::from_value(json!({ + "fileKey":"k","roots":["t","b"],"diagnostics":[],"nodes":{ + "t":{"id":"t","type":"TEXT","fields":{"fontSize":16.25,"lineHeight":{"unit":"PERCENT","value":160}}}, + "b":{"id":"b","type":"TEXT","fields":{"fontSize":16,"lineHeight":{"unit":"PERCENT","value":150}, + "boundVariables":{"fontSize":{"id":"v"}}}}} + })).unwrap(); + let mut map = SourceMap::empty(); + map.entries = vec![entry("t"), entry("b")]; + map.describe_conversions(&snapshot); + assert!( + map.entries[0] + .calculation + .as_deref() + .unwrap() + .ends_with("= 26px") + ); + assert!( + map.entries[1] + .calculation + .as_deref() + .unwrap() + .contains("kept as a percentage") + ); + } +} diff --git a/crates/devup-mcp-devup-ui/src/theme/devup_json.rs b/crates/devup-mcp-devup-ui/src/theme/devup_json.rs index 6ddf9e9b..a888261a 100644 --- a/crates/devup-mcp-devup-ui/src/theme/devup_json.rs +++ b/crates/devup-mcp-devup-ui/src/theme/devup_json.rs @@ -328,6 +328,7 @@ pub fn generate_devup_json( } let category = if kind == "color" { "colors" } else { "length" }; source_entries.push(ProvenanceEntry { + calculation: None, generated_property: None, generated_range: None, json_pointer: Some(format!( @@ -450,6 +451,7 @@ pub fn generate_devup_json( slots[level] = Some(typography_value(&style.value, &variable_names)); } source_entries.push(ProvenanceEntry { + calculation: None, generated_property: None, generated_range: None, json_pointer: Some(format!( @@ -473,6 +475,7 @@ pub fn generate_devup_json( slots[level] = Some(shadow); } source_entries.push(ProvenanceEntry { + calculation: None, generated_property: None, generated_range: None, json_pointer: Some(format!( diff --git a/crates/devup-mcp-devup-ui/tests/snapshots/wquw_151__wquw_151_proofread_source_map.snap b/crates/devup-mcp-devup-ui/tests/snapshots/wquw_151__wquw_151_proofread_source_map.snap index 9707a58a..d4bbaa71 100644 --- a/crates/devup-mcp-devup-ui/tests/snapshots/wquw_151__wquw_151_proofread_source_map.snap +++ b/crates/devup-mcp-devup-ui/tests/snapshots/wquw_151__wquw_151_proofread_source_map.snap @@ -136,6 +136,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(28 * 140 / 100) = 39px", "generatedProperty": "lineHeight=\"39px\"", "nodeId": "3879:35520", "property": "lineHeight", @@ -290,6 +291,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "3879:35523", "property": "lineHeight", @@ -354,6 +356,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(17 * 170 / 100) = 29px", "generatedProperty": "lineHeight=\"29px\"", "nodeId": "3879:35524", "property": "lineHeight", @@ -528,6 +531,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "3879:35528", "property": "lineHeight", @@ -592,6 +596,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(17 * 170 / 100) = 29px", "generatedProperty": "lineHeight=\"29px\"", "nodeId": "3879:35529", "property": "lineHeight", @@ -844,6 +849,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(15 * 160 / 100) = 24px", "generatedProperty": "lineHeight=\"24px\"", "nodeId": "3879:35535", "property": "lineHeight", @@ -952,6 +958,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(14 * 160 / 100) = 22px", "generatedProperty": "lineHeight=\"22px\"", "nodeId": "3879:35536", "property": "lineHeight", @@ -1083,6 +1090,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "3879:35538", "property": "lineHeight", @@ -1141,6 +1149,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "3879:35539", "property": "lineHeight", @@ -1407,6 +1416,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "I3879:35545;1690:32933", "property": "lineHeight", @@ -1526,6 +1536,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(14 * 160 / 100) = 22px", "generatedProperty": "lineHeight=\"22px\"", "nodeId": "I3879:35545;1690:32947", "property": "lineHeight", @@ -1639,6 +1650,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(22 * 140 / 100) = 31px", "generatedProperty": "lineHeight=\"31px\"", "nodeId": "3879:35547", "property": "lineHeight", @@ -1776,6 +1788,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "I3879:35549;1690:32933", "property": "lineHeight", @@ -1895,6 +1908,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(14 * 160 / 100) = 22px", "generatedProperty": "lineHeight=\"22px\"", "nodeId": "I3879:35549;1690:32947", "property": "lineHeight", @@ -2008,6 +2022,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(18 * 180 / 100) = 32px", "generatedProperty": "lineHeight=\"32px\"", "nodeId": "3879:35551", "property": "lineHeight", @@ -2151,6 +2166,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "I3879:35553;1690:32933", "property": "lineHeight", @@ -2270,6 +2286,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(14 * 160 / 100) = 22px", "generatedProperty": "lineHeight=\"22px\"", "nodeId": "I3879:35553;1690:32947", "property": "lineHeight", @@ -2602,6 +2619,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "I3879:35557;1690:32933", "property": "lineHeight", @@ -2721,6 +2739,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(14 * 160 / 100) = 22px", "generatedProperty": "lineHeight=\"22px\"", "nodeId": "I3879:35557;1690:32947", "property": "lineHeight", @@ -2834,6 +2853,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(18 * 180 / 100) = 32px", "generatedProperty": "lineHeight=\"32px\"", "nodeId": "3879:35559", "property": "lineHeight", @@ -2977,6 +2997,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(16 * 160 / 100) = 26px", "generatedProperty": "lineHeight=\"26px\"", "nodeId": "I3879:35561;1690:32933", "property": "lineHeight", @@ -3096,6 +3117,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(14 * 160 / 100) = 22px", "generatedProperty": "lineHeight=\"22px\"", "nodeId": "I3879:35561;1690:32947", "property": "lineHeight", @@ -3209,6 +3231,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(18 * 180 / 100) = 32px", "generatedProperty": "lineHeight=\"32px\"", "nodeId": "3879:35563", "property": "lineHeight", @@ -3427,6 +3450,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(18 * 170 / 100) = 31px", "generatedProperty": "lineHeight=\"31px\"", "nodeId": "3879:35566", "property": "lineHeight", @@ -3532,6 +3556,7 @@ expression: output.source_map "resolution": "raw-fallback" }, { + "calculation": "round(fontSize * percent / 100) = round(18 * 170 / 100) = 31px", "generatedProperty": "lineHeight=\"31px\"", "nodeId": "3879:35568", "property": "lineHeight", diff --git a/crates/devup-mcp/src/server/delivery.rs b/crates/devup-mcp/src/server/delivery.rs index 1b15839d..c715e9bd 100644 --- a/crates/devup-mcp/src/server/delivery.rs +++ b/crates/devup-mcp/src/server/delivery.rs @@ -13,6 +13,15 @@ use serde_json::Value; pub const MAX_INLINE_OUTPUT_BYTES: usize = 256 * 1024; pub const MAX_INLINE_TOTAL_BYTES: usize = 1024 * 1024; pub const RESOURCE_CHUNK_BYTES: usize = 256 * 1024; +/// Debug outputs (rawSnapshot/rawPayload) describe the design, not the screen, +/// and dwarf the code. Under `auto` they travel as resources past this size so +/// the warnings and checklist are not buried under them; `inline` still forces +/// them into the response. +pub const MAX_INLINE_DEBUG_BYTES: usize = 32 * 1024; + +fn is_debug_output(output: &ProjectedOutput) -> bool { + output.name.starts_with("raw-") +} #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "kebab-case")] @@ -121,7 +130,11 @@ pub fn choose_delivery( })?; match mode { DeliveryMode::Auto => Ok(DeliveryDecision { - inline: every_output_inline && total_bytes <= MAX_INLINE_TOTAL_BYTES, + inline: every_output_inline + && total_bytes <= MAX_INLINE_TOTAL_BYTES + && !outputs + .iter() + .any(|o| is_debug_output(o) && o.bytes.len() > MAX_INLINE_DEBUG_BYTES), }), DeliveryMode::Inline if total_bytes > MAX_INLINE_TOTAL_BYTES => { Err(inline_size_error(outputs, total_bytes, None)) @@ -364,3 +377,29 @@ mod r12_tests { ); } } + +#[cfg(test)] +mod debug_delivery_tests { + use super::*; + + #[test] + fn large_debug_output_goes_to_resource_under_auto_but_inline_forces_it() { + let big = ProjectedOutput::text( + "raw-snapshot.json", + "application/json", + vec![b'a'; 40 * 1024], + ); + let code = ProjectedOutput::text("Screen.tsx", "text/plain", vec![b'a'; 40 * 1024]); + assert!( + !choose_delivery(DeliveryMode::Auto, std::slice::from_ref(&big)) + .unwrap() + .inline + ); + assert!(choose_delivery(DeliveryMode::Auto, &[code]).unwrap().inline); + assert!( + choose_delivery(DeliveryMode::Inline, &[big]) + .unwrap() + .inline + ); + } +} diff --git a/crates/devup-mcp/src/server/feature_trace.rs b/crates/devup-mcp/src/server/feature_trace.rs index 49c8de29..9f5d29e0 100644 --- a/crates/devup-mcp/src/server/feature_trace.rs +++ b/crates/devup-mcp/src/server/feature_trace.rs @@ -533,7 +533,7 @@ fn bound(mut value: Value, max_items: usize) -> Value { .as_object() .unwrap() .iter() - .filter(|(k, _)| !matches!(k.as_str(), "truncation" | "limits" | "status")) + .filter(|(k, _)| !matches!(k.as_str(), "truncation" | "limits" | "status" | "summary")) .max_by_key(|(_, v)| v.to_string().len()) .map(|(k, _)| k.clone()); let Some(key) = key else { @@ -645,16 +645,44 @@ pub(super) async fn run( } } } - if supplied(&input.component_path) - && !selected_files.contains(input.component_path.as_deref().unwrap()) + // Anchor-first: an explicit componentPath is read directly. The capped UI + // inventory only enriches it (import sites, reuse ranking); it never gates it. + let mut anchor_status = Value::Null; + if let Some(requested) = input + .component_path + .as_deref() + .filter(|s| !s.trim().is_empty()) { - chain.push(hop( - "screen-component", - json!(input.component_path), - Value::Null, - json!({}), - Some("Component path is absent from the bounded authoritative UI inventory."), - )); + let normalized = anchor_path(requested); + let in_inventory = selected_files.contains(&normalized); + match resolve_anchor(&root, &normalized) { + Ok(()) => { + selected_files.insert(normalized.clone()); + anchor_status = json!({"path":normalized,"read":"direct","inInventory":in_inventory, + "inventoryTruncated":ui["truncation"]["truncated"]}); + if !in_inventory { + chain.push(hop( + "screen-component", + json!(requested), + json!(normalized), + json!({"declaration":"Explicit componentPath anchor read directly from disk","inventoryNote":"Not present in the capped UI inventory; import sites and reuse ranking for this file are unavailable."}), + None, + )); + } + } + Err(reason) => { + anchor_status = json!({"path":normalized,"read":"failed","reason":reason}); + chain.push(hop( + "screen-component", + json!(requested), + Value::Null, + json!({}), + Some(&format!( + "Explicit componentPath could not be read: {reason}" + )), + )); + } + } } for (i, op) in operations.iter().enumerate() { let id = supplied(&input.operation_id) @@ -966,8 +994,13 @@ pub(super) async fn run( "implementedEvidence":implemented.get(*state), "reason":"Presence records explicit state declarations or App Router boundary components only. Missing evidence is unknown, never assumed absent; runtime coverage requires tests."})).collect::>(); let acceptance=input.acceptance_criteria.iter().map(|criterion|json!({"criterion":criterion,"status":"UNVERIFIED","reason":"Caller acceptance text is preserved without semantic interpretation; supply tests or inspect the linked evidence."})).collect::>(); + let summary = summarize( + &chain, + &anchor_status, + ui["truncation"]["truncated"] == true, + ); Ok(bound( - json!({"status":"OK","projectRoot":root,"anchors":{"routePath":input.route_path,"figmaNodeId":input.figma_node_id,"artifactId":input.artifact_id,"operationId":input.operation_id,"apiPath":input.api_path,"method":input.method,"componentPath":input.component_path,"tableName":input.table_name},"requirement":input.requirement,"acceptanceMatrix":acceptance, + json!({"status":"OK","summary":summary,"projectRoot":root,"anchors":{"routePath":input.route_path,"figmaNodeId":input.figma_node_id,"artifactId":input.artifact_id,"operationId":input.operation_id,"apiPath":input.api_path,"method":input.method,"componentPath":input.component_path,"tableName":input.table_name},"requirement":input.requirement,"acceptanceMatrix":acceptance, "chain":chain,"artifacts":traced_artifacts.into_values().collect::>(),"sourceOwnership":ownership,"reuseCandidates":reuse,"designContract":comparisons,"requiredStates":states, "designEvidence":design.evidence,"designDiagnostics":design.diagnostics,"diagnostics":diagnostics, "inventoryEvidence":{"uiTruncation":ui["truncation"],"uiLimits":ui["limits"],"uiDiagnostics":ui["diagnostics"],"uiExcludedPaths":ui["excludedPaths"],"unparsedFiles":ui["unparsedFiles"],"api":{"found":api["found"],"excludedPaths":api["excludedPaths"],"authorityNote":api["authorityNote"],"issues":array(&api["specs"]).iter().filter(|v|v.get("parseError").is_some()||v.get("readError").is_some()).collect::>()}, @@ -977,6 +1010,60 @@ pub(super) async fn run( max_items, )) } +/// Normalise a caller-supplied component path to the inventory's `a/b/c.tsx` form. +fn anchor_path(path: &str) -> String { + let unified = path.trim().replace('\\', "/"); + unified.trim_start_matches("./").to_owned() +} +/// An anchor must be a regular file strictly inside the project root. +fn resolve_anchor(root: &Path, relative_path: &str) -> Result<(), String> { + let candidate = Path::new(relative_path); + if candidate.is_absolute() + || candidate + .components() + .any(|c| matches!(c, std::path::Component::ParentDir)) + { + return Err("path must be relative to the project root and may not contain '..'".into()); + } + let full = root.join(candidate); + let canonical = full.canonicalize().map_err(|e| format!("{e}"))?; + let canonical_root = root.canonicalize().map_err(|e| format!("{e}"))?; + if !canonical.starts_with(&canonical_root) { + return Err("path resolves outside the project root".into()); + } + if !canonical.is_file() { + return Err("path is not a regular file".into()); + } + Ok(()) +} + +/// Summary-first digest: what is resolved, what is not, and what a human must +/// still check. Everything else in the response is detail behind it. +fn summarize(chain: &[Value], anchor: &Value, truncated_inventory: bool) -> Value { + let unresolved = chain + .iter() + .filter(|h| h["status"] == "UNVERIFIED") + .map(|h| json!({"kind":h["kind"],"from":h["from"],"reason":h["reason"]})) + .collect::>(); + let resolved = chain.iter().filter(|h| h["status"] == "RESOLVED").count(); + let mut required = vec![]; + if anchor["read"] == "failed" { + required.push(json!( + "Fix componentPath: the explicit anchor could not be read." + )); + } + let mut recommended = vec![]; + if truncated_inventory { + recommended.push(json!("UI inventory was capped; import sites and reuse ranking may be incomplete. Anchor files were still read directly.")); + } + if !unresolved.is_empty() { + recommended.push(json!( + "Verify each UNVERIFIED hop manually; absence of a link is not proof of absence." + )); + } + json!({"hopsResolved":resolved,"hopsUnverified":unresolved.len(),"anchor":anchor, + "required":required,"recommended":recommended,"unverified":unresolved}) +} fn handler_matches(handler: &Handler, operation: &Operation, root: &Path) -> bool { let authority = root.join(&operation.file).parent().unwrap().to_path_buf(); handler.file.starts_with(authority.join("src/routes")) diff --git a/crates/devup-mcp/src/server/feature_trace_tests.rs b/crates/devup-mcp/src/server/feature_trace_tests.rs index 398550c3..f0d71808 100644 --- a/crates/devup-mcp/src/server/feature_trace_tests.rs +++ b/crates/devup-mcp/src/server/feature_trace_tests.rs @@ -381,3 +381,33 @@ async fn database_parse_failure_is_preserved_as_unverified_evidence() { .any(|h| h["kind"] == "model-columns" && h["status"] == "UNVERIFIED") ); } + +#[tokio::test] +async fn explicit_component_anchor_is_read_even_when_absent_from_inventory() { + let f = Fixture::new(); + // `.hidden-dir` is not scanned by the UI inventory, so only the direct read can find it. + f.write( + "lib/Standalone.tsx", + "export function Standalone() { api.post('createUser'); return ; }", + ); + let v = f + .trace(json!({"componentPath":"lib\\Standalone.tsx"})) + .await; + assert_eq!(v["summary"]["anchor"]["read"], "direct", "{v}"); + let hops = v["chain"].as_array().unwrap(); + assert!( + hops.iter() + .any(|h| h["kind"] == "component-api" && h["status"] == "RESOLVED"), + "anchor file must be parsed for API references: {v}" + ); +} + +#[tokio::test] +async fn unreadable_or_escaping_component_anchor_is_a_required_fix() { + let f = Fixture::new(); + for path in ["components/Missing.tsx", "../outside.tsx"] { + let v = f.trace(json!({"componentPath":path})).await; + assert_eq!(v["summary"]["anchor"]["read"], "failed", "{path}: {v}"); + assert!(!v["summary"]["required"].as_array().unwrap().is_empty()); + } +} diff --git a/crates/devup-mcp/src/server/field_select.rs b/crates/devup-mcp/src/server/field_select.rs new file mode 100644 index 00000000..467e21c2 --- /dev/null +++ b/crates/devup-mcp/src/server/field_select.rs @@ -0,0 +1,169 @@ +//! Field selection for the diagnostic outputs. +//! +//! A raw snapshot of one screen can run to tens of thousands of tokens, and the +//! usual question ("what are the exact fills / spacing / type values here?") +//! needs a handful of properties. `fields` narrows `rawSnapshot`, `rawPayload` +//! and the sourceMap entries to the named raw Figma properties, so the answer +//! fits in one response instead of being cut off and re-queried. +use std::collections::BTreeSet; + +use serde_json::Value; + +/// Group names a caller can use instead of listing raw property names. +const GROUPS: &[(&str, &[&str])] = &[ + ( + "colors", + &[ + "fills", + "strokes", + "effects", + "opacity", + "boundVariables", + "fillStyleId", + "strokeStyleId", + ], + ), + ( + "spacing", + &[ + "itemSpacing", + "counterAxisSpacing", + "paddingTop", + "paddingRight", + "paddingBottom", + "paddingLeft", + "layoutMode", + "primaryAxisAlignItems", + "counterAxisAlignItems", + ], + ), + ( + "typography", + &[ + "fontSize", + "fontName", + "fontWeight", + "lineHeight", + "letterSpacing", + "textStyleId", + "styledTextSegments", + "textAlignHorizontal", + "characters", + ], + ), + ("shadow", &["effects"]), + ( + "radius", + &[ + "cornerRadius", + "topLeftRadius", + "topRightRadius", + "bottomLeftRadius", + "bottomRightRadius", + ], + ), + ( + "size", + &[ + "width", + "height", + "layoutSizingHorizontal", + "layoutSizingVertical", + "layoutGrow", + "absoluteBoundingBox", + ], + ), +]; + +/// Properties every pruned node keeps so the tree stays navigable. +const STRUCTURE: &[&str] = &["childrenIds", "parentId", "name", "visible", "isAsset"]; + +/// Expand group names and drop blanks. Empty input means "no selection". +pub(super) fn expand(requested: &[String]) -> BTreeSet { + let mut selected = BTreeSet::new(); + for name in requested.iter().map(|n| n.trim()).filter(|n| !n.is_empty()) { + match GROUPS + .iter() + .find(|(group, _)| group.eq_ignore_ascii_case(name)) + { + Some((_, members)) => selected.extend(members.iter().map(|m| (*m).to_owned())), + None => { + selected.insert(name.to_owned()); + } + } + } + selected +} + +/// Keep only the selected properties on every node of a raw snapshot or +/// payload. Node identity, type and tree links are always kept. +pub(super) fn prune_nodes(raw: &mut Value, selected: &BTreeSet) { + if selected.is_empty() { + return; + } + match raw { + Value::Object(object) => { + if let Some(Value::Object(nodes)) = object.get_mut("nodes") { + for node in nodes.values_mut() { + if let Some(Value::Object(fields)) = node.get_mut("fields") { + fields.retain(|key, _| { + selected.contains(key) || STRUCTURE.contains(&key.as_str()) + }); + } + if let Some(extra) = node.get_mut("extra").and_then(Value::as_object_mut) { + extra.clear(); + } + } + } + for value in object.values_mut() { + prune_nodes(value, selected); + } + } + Value::Array(values) => values.iter_mut().for_each(|v| prune_nodes(v, selected)), + _ => {} + } +} + +/// Keep sourceMap entries whose raw `property` was selected. +pub(super) fn filter_entries(entries: &mut Vec, selected: &BTreeSet) { + if !selected.is_empty() { + entries.retain(|e| e["property"].as_str().is_some_and(|p| selected.contains(p))); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn groups_expand_and_names_pass_through() { + let selected = expand(&["Typography".into(), "cornerRadius".into(), " ".into()]); + assert!(selected.contains("lineHeight") && selected.contains("cornerRadius")); + assert!(!selected.contains("fills")); + } + + #[test] + fn pruning_keeps_structure_and_selected_fields_only() { + let mut raw = json!({"snapshot":{"nodes":{"1:1":{"id":"1:1","type":"TEXT", + "fields":{"fontSize":16,"fills":[1],"childrenIds":[],"name":"t"},"extra":{"big":1}}}}}); + prune_nodes(&mut raw, &expand(&["fontSize".into()])); + let node = &raw["snapshot"]["nodes"]["1:1"]; + assert_eq!(node["id"], "1:1"); + assert!(node["fields"].get("fills").is_none()); + assert_eq!(node["fields"]["fontSize"], 16); + assert!(node["fields"].get("childrenIds").is_some()); + assert!(node["extra"].as_object().unwrap().is_empty()); + } + + #[test] + fn empty_selection_changes_nothing() { + let mut raw = json!({"nodes":{"a":{"fields":{"fills":[1]}}}}); + let before = raw.clone(); + prune_nodes(&mut raw, &expand(&[])); + assert_eq!(raw, before); + let mut entries = vec![json!({"property":"w"})]; + filter_entries(&mut entries, &expand(&[])); + assert_eq!(entries.len(), 1); + } +} diff --git a/crates/devup-mcp/src/server/mod.rs b/crates/devup-mcp/src/server/mod.rs index 9af9c660..365c6e5b 100644 --- a/crates/devup-mcp/src/server/mod.rs +++ b/crates/devup-mcp/src/server/mod.rs @@ -5,6 +5,7 @@ pub mod delivery; mod design_drift; mod diagnostics; mod feature_trace; +mod field_select; mod guide; pub mod operation; pub mod output; @@ -16,6 +17,7 @@ mod quality; mod release_check; pub mod resources; mod result_contract; +mod review; pub mod self_update; mod skills; mod stack_diff; @@ -80,6 +82,7 @@ pub struct FigmaExportWorkflowInput { /// URL path segments are percent-encoded. Omit to keep placeholder paths. #[serde(default)] pub asset_public_root: Option, + /// Poll a server-local asset job. Omit url/artifactId/assetRequests when polling. #[serde(default)] pub job_id: Option, @@ -1609,10 +1612,14 @@ impl DevupServer { if let Some(artifact_id) = input.artifact_id.as_deref() { if input.url.is_some() || input.refresh { - return Err(to_mcp_error(DevupError::new( + return Err(to_mcp_error(DevupError::with_details( ErrorCode::DevupFigmaHandoffInvalid, "artifactId cannot be used together with url or refresh.", false, + json!({"stage":"preflight","reason":"invalid-argument-combination", + "distinctFrom":"DEVUP_FIGMA_HANDOFF_EXPIRED means the artifactId itself is gone; this means the call mixed incompatible arguments.", + "nextAction":{"tool":"devup_figma_export", + "how":"Send either artifactId (reuse, no Figma call) or url (+ refresh:true to recollect), never both."}}), ))); } let artifact = self @@ -1719,6 +1726,7 @@ impl DevupServer { asset_captures: asset_selections, asset_output_paths, asset_public_root: asset_public_root.clone(), + fields: input.fields.clone(), delivery, }, request, @@ -1760,6 +1768,7 @@ impl DevupServer { asset_captures: asset_selections, asset_output_paths, asset_public_root: asset_public_root.clone(), + fields: input.fields.clone(), delivery, }, &artifact.payload, @@ -1839,31 +1848,48 @@ impl DevupServer { all_screens: input.all_screens, }); } - let result = self - .start_operation( - PendingOperation::Export { - outputs: input.outputs, - component_name: input.component_name, - include_diagnostics: input.include_diagnostics, - root_layout, - asset_names_per_node: input.asset_names_per_node, - scope: input.scope, - strict: input.strict, - output_paths: input.output_paths, - page_scaffold: input.page_scaffold, - previous_design_fingerprints: input.previous_design_fingerprints, - frame_ids, - all_screens: input.all_screens, - asset_captures: asset_selections, - asset_output_paths, - asset_public_root: asset_public_root.clone(), - delivery, - }, - request, - input.refresh, - ) + let operation = PendingOperation::Export { + outputs: input.outputs, + component_name: input.component_name, + include_diagnostics: input.include_diagnostics, + root_layout, + asset_names_per_node: input.asset_names_per_node, + scope: input.scope, + strict: input.strict, + output_paths: input.output_paths, + page_scaffold: input.page_scaffold, + previous_design_fingerprints: input.previous_design_fingerprints, + frame_ids, + all_screens: input.all_screens, + asset_captures: asset_selections, + asset_output_paths, + asset_public_root: asset_public_root.clone(), + fields: input.fields.clone(), + delivery, + }; + // A frame link plus frameIds is a precise request; refusing it because + // the plugin wants the parent Section only makes the caller repeat + // the ancestor lookup the plugin already did. Retry once, there. + let retry_with = |error: &DevupError| section_retry(error, &operation, &request); + let result = match self + .start_operation(operation.clone(), request.clone(), input.refresh) .await - .map_err(to_mcp_error)?; + { + Err(error) => match retry_with(&error) { + Some((operation, request, resolved)) => self + .start_operation(operation, request, input.refresh) + .await + .map(|mut value| { + if let Some(object) = value.as_object_mut() { + object.insert("autoResolvedSection".into(), resolved); + } + value + }), + None => Err(error), + }, + ok => ok, + } + .map_err(to_mcp_error)?; Ok(tool_result(with_project_checks( result, input.project_root.as_deref(), @@ -2245,6 +2271,7 @@ fn with_project_checks( project_root: Option<&str>, lookup: &skills::Lookup, ) -> Value { + review::attach(&mut result); const OUTPUTS: [&str; 3] = ["tsx", "componentTsx", "responsiveTsx"]; let mut generated: Vec<(String, String)> = OUTPUTS .into_iter() @@ -2426,6 +2453,50 @@ fn with_error_identity(mut error: ErrorData) -> ErrorData { error } +/// The one automatic recovery for `DEVUP_SECTION_REQUIRED`: re-aim the same +/// request at the ancestor Section the plugin reported, keeping the requested +/// frame as the selection. Returns `None` when the plugin named no Section. +fn section_retry( + error: &DevupError, + operation: &PendingOperation, + request: &CollectionRequest, +) -> Option<(PendingOperation, CollectionRequest, Value)> { + if error.details["pluginCode"] != "DEVUP_SECTION_REQUIRED" { + return None; + } + let section_id = error.details["sectionId"].as_str()?; + let requested = request.target.node_id.clone()?; + let PendingOperation::Export { + frame_ids, + all_screens, + .. + } = operation + else { + return None; + }; + let mut operation = operation.clone(); + let mut request = request.clone(); + let selection = if frame_ids.is_empty() && !all_screens { + vec![requested.clone()] + } else { + frame_ids.clone() + }; + if let PendingOperation::Export { frame_ids, .. } = &mut operation { + frame_ids.clone_from(&selection); + } + request.target.node_id = Some(section_id.to_owned()); + request.section = Some(SectionReadOptions { + frame_ids: selection.clone(), + all_screens: *all_screens, + }); + Some(( + operation, + request, + json!({"requestedNodeId":requested,"sectionId":section_id,"frameIds":selection, + "note":"The url named a node inside a SECTION; the export was re-run against that SECTION with the node selected."}), + )) +} + /// Maps a [`DevupError`] onto the JSON-RPC error the caller actually sees. /// /// The protocol code is not decoration here: the caller is usually an agent @@ -2705,3 +2776,70 @@ mod r8_recovery_tests { } } } + +#[cfg(test)] +mod section_retry_tests { + use super::*; + use devup_mcp_devup_ui::codegen::RootLayout; + + fn export(frame_ids: Vec) -> PendingOperation { + PendingOperation::Export { + outputs: vec!["tsx".into()], + previous_design_fingerprints: None, + component_name: None, + include_diagnostics: false, + root_layout: RootLayout::Standalone, + asset_names_per_node: true, + scope: "node".into(), + strict: false, + output_paths: Default::default(), + page_scaffold: None, + frame_ids, + all_screens: false, + asset_captures: vec![], + asset_output_paths: Default::default(), + asset_public_root: None, + fields: vec![], + delivery: DeliveryMode::Auto, + } + } + + fn target() -> FigmaTarget { + FigmaTarget { + file_key: "abc".into(), + node_id: Some("1:2".into()), + branch_key: None, + } + } + + #[test] + fn frame_link_is_retried_against_its_section_with_the_frame_selected() { + let error = DevupError::with_details( + ErrorCode::DevupSnapshotUnsupported, + "DEVUP_SECTION_REQUIRED", + false, + json!({"pluginCode":"DEVUP_SECTION_REQUIRED","sectionId":"9:9","nodeId":"1:2"}), + ); + let request = CollectionRequest::new(target(), CollectionScope::Node); + let (operation, request, note) = section_retry(&error, &export(vec![]), &request).unwrap(); + assert_eq!(request.target.node_id.as_deref(), Some("9:9")); + assert_eq!(request.section.unwrap().frame_ids, vec!["1:2".to_owned()]); + assert!( + matches!(operation, PendingOperation::Export { frame_ids, .. } if frame_ids == ["1:2"]) + ); + assert_eq!(note["sectionId"], "9:9"); + } + + #[test] + fn no_section_or_other_codes_are_not_retried() { + let request = CollectionRequest::new(target(), CollectionScope::Node); + for details in [ + json!({"pluginCode":"DEVUP_SECTION_REQUIRED","sectionId":null}), + json!({"pluginCode":"DEVUP_NODE_NOT_FOUND","sectionId":"9:9"}), + ] { + let error = + DevupError::with_details(ErrorCode::DevupSnapshotUnsupported, "x", false, details); + assert!(section_retry(&error, &export(vec![]), &request).is_none()); + } + } +} diff --git a/crates/devup-mcp/src/server/operation.rs b/crates/devup-mcp/src/server/operation.rs index 0eb04cfc..c9a62b05 100644 --- a/crates/devup-mcp/src/server/operation.rs +++ b/crates/devup-mcp/src/server/operation.rs @@ -40,6 +40,7 @@ pub enum PendingOperation { asset_captures: Vec, asset_output_paths: BTreeMap, asset_public_root: Option, + fields: Vec, delivery: DeliveryMode, }, Search { diff --git a/crates/devup-mcp/src/server/projection.rs b/crates/devup-mcp/src/server/projection.rs index 89021105..3bfa9d79 100644 --- a/crates/devup-mcp/src/server/projection.rs +++ b/crates/devup-mcp/src/server/projection.rs @@ -1247,8 +1247,10 @@ async fn project_operation( asset_captures, mut asset_output_paths, mut asset_public_root, + fields, delivery, } => { + let selected_fields = super::field_select::expand(&fields); page_scaffold::validate(page_scaffold.as_ref(), &outputs)?; let previous_fingerprints = super::design_drift::validate_request( &outputs, @@ -1731,10 +1733,20 @@ async fn project_operation( frame[field] = json!(output.tsx); attach_fidelity(&mut frame, &output.fidelity_report, include_diagnostics); if outputs.iter().any(|output| output == "sourceMap") { + let mut frame_entries: Vec = output + .source_map + .property_entries() + .into_iter() + .filter_map(|entry| serde_json::to_value(entry).ok()) + .collect(); + super::field_select::filter_entries( + &mut frame_entries, + &selected_fields, + ); frame["sourceMap"] = json!({"version":output.source_map.version, "designFingerprints":design_fingerprints, "resolutionSemantics":{"axis":"mapping-method","dictionary":"/resolutionSemantics"}, - "entries":output.source_map.property_entries(),"source":{"fileKey":vouched_file_key(payload), + "entries":frame_entries,"source":{"fileKey":vouched_file_key(payload), "rootNodeId":candidate.node.node_id,"sourceVersion":payload.source_version, "generatedOutput":field,"mappingKind":"node-field-property"}}); } @@ -2129,6 +2141,8 @@ async fn project_operation( false, ) })?; + let mut raw = raw; + super::field_select::prune_nodes(&mut raw, &selected_fields); if output_paths.contains_key("rawSnapshot") { pending_text_outputs.insert( "rawSnapshot".to_owned(), @@ -2156,6 +2170,8 @@ async fn project_operation( false, ) })?; + let mut raw = raw; + super::field_select::prune_nodes(&mut raw, &selected_fields); if output_paths.contains_key("rawPayload") { pending_text_outputs.insert( "rawPayload".to_owned(), @@ -2166,11 +2182,18 @@ async fn project_operation( } if outputs.iter().any(|output| output == "sourceMap") && !section_tsx_projected { + let mut tsx_entries: Vec = tsx_source_map + .map(|source_map| source_map.property_entries()) + .unwrap_or_default() + .into_iter() + .filter_map(|entry| serde_json::to_value(entry).ok()) + .collect(); + super::field_select::filter_entries(&mut tsx_entries, &selected_fields); let source_map = json!({ "version": 2, "designFingerprints":design_fingerprints, "resolutionSemantics":devup_mcp_devup_ui::provenance::resolution_semantics(), - "tsx": tsx_source_map.map(|source_map| source_map.property_entries()).unwrap_or_default(), + "tsx": tsx_entries, "devupJson": devup_json_source_map .map(|source_map| source_map.entries) .unwrap_or_default(), @@ -5136,6 +5159,7 @@ mod w1_regressions { asset_captures: vec![], asset_output_paths: BTreeMap::new(), asset_public_root: None, + fields: vec![], delivery: DeliveryMode::Inline, } } @@ -5724,6 +5748,41 @@ mod w1_regressions { } } + #[tokio::test] + async fn fields_narrow_raw_snapshot_and_source_map() { + let mut op = operation(&["tsx", "rawSnapshot", "sourceMap"]); + if let PendingOperation::Export { fields, .. } = &mut op { + *fields = vec!["typography".into()]; + } + let result = project(payload(), op).await.unwrap(); + for node in result["rawSnapshot"]["nodes"].as_object().unwrap().values() { + for key in node["fields"].as_object().unwrap().keys() { + assert!( + [ + "fontSize", + "fontName", + "fontWeight", + "lineHeight", + "letterSpacing", + "textStyleId", + "styledTextSegments", + "textAlignHorizontal", + "characters", + "childrenIds", + "parentId", + "name", + "visible", + "isAsset" + ] + .contains(&key.as_str()), + "unselected field survived: {key}" + ); + } + } + for entry in result["sourceMap"]["tsx"].as_array().into_iter().flatten() { + assert!(entry["property"] != "width", "{entry}"); + } + } #[tokio::test] async fn r16_non_projection_partial_has_its_own_status_cause() { let mut data = payload(); diff --git a/crates/devup-mcp/src/server/review.rs b/crates/devup-mcp/src/server/review.rs new file mode 100644 index 00000000..413e673f --- /dev/null +++ b/crates/devup-mcp/src/server/review.rs @@ -0,0 +1,157 @@ +//! Summary-first digest of an export response. +//! +//! The full response stays as it is; this adds a short `summary` and, when the +//! result is not exact/complete, a `reviewChecklist` that says what a human +//! still has to check, where, and how. Everything is derived from fields the +//! response already carries, so nothing here can disagree with them. +use serde_json::{Value, json}; + +const MAX_ITEMS: usize = 20; + +fn text(value: &Value) -> &str { + value.as_str().unwrap_or("") +} + +/// One checklist item per diagnostic that needs a human, grouped by code so a +/// hundred identical findings read as one line with a node list. +fn items_from_issues(scope: &str, issues: &[Value], out: &mut Vec) { + let mut groups: Vec<(String, Vec<&Value>)> = vec![]; + for issue in issues { + let code = text(&issue["code"]).to_owned(); + match groups.iter_mut().find(|(c, _)| *c == code) { + Some((_, members)) => members.push(issue), + None => groups.push((code, vec![issue])), + } + } + for (code, members) in groups { + let lossy = members + .iter() + .any(|m| matches!(text(&m["fidelityImpact"]), "lossy" | "failed")); + let nodes: Vec<&Value> = members.iter().map(|m| &m["nodeId"]).collect(); + let first = members[0]; + let how = first["details"]["nextAction"] + .as_str() + .map(str::to_owned) + .or_else(|| first["details"]["nextAction"]["how"].as_str().map(str::to_owned)) + .unwrap_or_else(|| { + "Compare the generated property with the raw Figma value for these nodes (export rawSnapshot with debug: true) before shipping.".into() + }); + out.push(json!({ + "severity": if lossy { "required" } else { "recommended" }, + "scope": scope, + "code": code, + "count": members.len(), + "what": first["message"], + "properties": members.iter().filter_map(|m| m["property"].as_str()).collect::>(), + "nodeIds": nodes.into_iter().take(10).collect::>(), + "how": how, + })); + } +} + +/// `round(fontSize * percent / 100)` is the one unit conversion the generator +/// performs that a reader cannot reproduce from the raw field alone: Figma +/// stores `lineHeight: {unit: PERCENT, value: 160}`, the TSX says `26px`. +fn has_line_height(value: &Value) -> bool { + value["sourceMap"]["entries"] + .as_array() + .is_some_and(|entries| { + entries + .iter() + .any(|e| matches!(text(&e["property"]), "lineHeight" | "styledTextSegments")) + }) +} + +fn collect(response: &Value, scope: &str, out: &mut Vec) { + if let Some(issues) = response["projectionIssues"].as_array() { + items_from_issues(scope, issues, out); + } + if response["quality"]["assets"] == "partial" || response["quality"]["assets"] == "failed" { + out.push(json!({"severity":"required","scope":scope,"code":"ASSETS_INCOMPLETE", + "what":"Some assets were not collected.","how":"Read assetSummary.unavailable[]; re-request those assets by id with assetRequests (original url, not artifactId)."})); + } + if response["quality"]["acquisition"] == "partial" { + out.push(json!({"severity":"recommended","scope":scope,"code":"ACQUISITION_PARTIAL", + "what":"The Figma snapshot was incomplete.","how":"Check completenessReport for what is missing; re-export with refresh:true if the missing part matters."})); + } + if has_line_height(response) { + out.push(json!({"severity":"recommended","scope":scope,"code":"LINE_HEIGHT_CONVERSION", + "what":"Percent line-height is written as px.", + "how":"Verify with round(fontSize * percent / 100): e.g. 160% at 16.25px = 26px. When fontSize is bound to a variable the generator keeps the percentage (`160%`) instead of px."})); + } +} + +/// Add `summary` and, only when something needs review, `reviewChecklist`. +pub(super) fn attach(result: &mut Value) { + let mut items = vec![]; + collect(result, "response", &mut items); + if let Some(frames) = result["frames"].as_array() { + for frame in frames { + let scope = frame["nodeId"].as_str().unwrap_or("frame"); + collect(frame, scope, &mut items); + } + } + let required = items.iter().filter(|i| i["severity"] == "required").count(); + let recommended = items.len() - required; + let projection = result["quality"]["projection"].clone(); + let next = if required > 0 { + "Resolve every `required` item in reviewChecklist before using this output." + } else if recommended > 0 { + "Usable; spot-check the `recommended` items in reviewChecklist." + } else { + "Nothing to review." + }; + if let Some(object) = result.as_object_mut() { + object.insert( + "summary".into(), + json!({"status":object.get("status"),"projection":projection, + "required":required,"recommended":recommended,"next":next}), + ); + if !items.is_empty() { + items.sort_by_key(|i| i["severity"] != "required"); + items.truncate(MAX_ITEMS); + object.insert("reviewChecklist".into(), json!(items)); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn clean_response_has_summary_and_no_checklist() { + let mut value = + json!({"status":"complete","quality":{"projection":"exact","assets":"not-requested"}}); + attach(&mut value); + assert_eq!(value["summary"]["required"], 0); + assert!(value.get("reviewChecklist").is_none()); + } + + #[test] + fn issues_group_by_code_and_lossy_is_required() { + let mut value = json!({"status":"partial","quality":{"projection":"lossy"}, + "projectionIssues":[ + {"code":"A","nodeId":"1:1","property":"w","fidelityImpact":"lossy","message":"m"}, + {"code":"A","nodeId":"1:2","property":"h","fidelityImpact":"lossy","message":"m"}, + {"code":"DEVUP_CODEGEN_PROPERTY_UNMAPPED","nodeId":"1:3","property":"x","fidelityImpact":"none","message":"u"}]}); + attach(&mut value); + let list = value["reviewChecklist"].as_array().unwrap(); + assert_eq!(list.len(), 2); + assert_eq!(list[0]["severity"], "required"); + assert_eq!(list[0]["count"], 2); + assert_eq!(value["summary"]["required"], 1); + assert_eq!(value["summary"]["recommended"], 1); + } + + #[test] + fn line_height_conversion_is_explained_when_typography_is_mapped() { + let mut value = json!({"status":"complete","quality":{"projection":"exact"}, + "sourceMap":{"entries":[{"property":"lineHeight"}]}}); + attach(&mut value); + assert_eq!( + value["reviewChecklist"][0]["code"], + "LINE_HEIGHT_CONVERSION" + ); + } +} diff --git a/crates/devup-mcp/src/server/tools.rs b/crates/devup-mcp/src/server/tools.rs index c6e42af9..49a56e0a 100644 --- a/crates/devup-mcp/src/server/tools.rs +++ b/crates/devup-mcp/src/server/tools.rs @@ -108,6 +108,13 @@ pub struct FigmaExportInput { /// asking for them by habit spent about eight bytes for every one of code. #[serde(default)] pub debug: bool, + /// Narrow rawSnapshot, rawPayload and sourceMap to these raw Figma property + /// names (e.g. "fills", "fontSize", "itemSpacing") or groups: "colors", + /// "spacing", "typography", "shadow", "radius", "size". Node identity and + /// tree links are always kept. Use it so a diagnostic answer fits in one + /// response instead of being cut off. + #[serde(default)] + pub fields: Vec, #[serde(default)] pub strict: bool, #[serde(default)] diff --git a/crates/devup-mcp/src/server/validation.rs b/crates/devup-mcp/src/server/validation.rs index 874d3bf7..1de08cbd 100644 --- a/crates/devup-mcp/src/server/validation.rs +++ b/crates/devup-mcp/src/server/validation.rs @@ -192,7 +192,7 @@ pub(super) fn validate_outputs(outputs: &[String], debug: bool) -> Result<(), De )); } if !debug && DIAGNOSIS_OUTPUTS.contains(&output.as_str()) { - return Err(DevupError::new( + return Err(DevupError::with_details( ErrorCode::DevupInvalidInput, format!( "{output} is the collected design in raw form, for deciding whether a \ @@ -202,6 +202,9 @@ pub(super) fn validate_outputs(outputs: &[String], debug: bool) -> Result<(), De debug: true to read it." ), false, + json!({"stage":"preflight","nextAction":{"tool":"devup_figma_export", + "arguments":{"debug":true}, + "how":"Repeat the same call with debug: true. Pair rawSnapshot with sourceMap, and reuse cache.artifactId for later projections instead of re-collecting."}}), )); } }