From 0f7127400599f76a26a8cc82495ff52fb29185e8 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Tue, 8 Sep 2026 16:57:27 +0900 Subject: [PATCH 1/3] fix: send the fidelity report when it disagrees with the grade above it Keying it on quality.projection hid a real defect. projection_quality is computed from diagnostics alone, so a coverage shortfall that raises no diagnostic leaves the grade reading exact - and the condition then took the report away with it. Measured on a real 50-node screen (3997:47884): fidelity.variables was 19 of 20 while quality.projection read exact. The missing one is a \\\ binding on an ellipse whose parent is exported as an SVG, so the theme colour is baked into the file and can never follow the theme. Three play buttons on that screen are affected. Before this change the response showed the shortfall; after the first attempt it hid it. The condition is now strict_compatible, the same predicate \strict: true\ refuses on, so the two cannot disagree about whether anything was lost. On the ten real captured screens it stays silent for every screen with neither a shortfall nor an approximation, which is what this is for. --- .../changepack_log_fidelity_visibility.json | 7 +++ crates/devup-mcp/src/server/projection.rs | 44 +++++++------------ crates/devup-mcp/tests/composite_export.rs | 30 ++++++++++--- 3 files changed, 48 insertions(+), 33 deletions(-) create mode 100644 .changepacks/changepack_log_fidelity_visibility.json diff --git a/.changepacks/changepack_log_fidelity_visibility.json b/.changepacks/changepack_log_fidelity_visibility.json new file mode 100644 index 0000000..910cc1d --- /dev/null +++ b/.changepacks/changepack_log_fidelity_visibility.json @@ -0,0 +1,7 @@ +{ + "changes": { + "crates/devup-mcp/Cargo.toml": "Patch" + }, + "note": "Send the fidelity report whenever it disagrees with the grade above it, rather than whenever the grade is not exact. Keying it on quality.projection was wrong in a way that hid a real defect: projection_quality is computed from diagnostics alone, so a coverage shortfall that raises no diagnostic leaves the grade reading exact and took the report away with it. Measured on a real 50-node screen, fidelity.variables was 19 of 20 - a $primary binding on an ellipse whose parent was exported as an SVG, so the theme colour is frozen into the file and cannot follow the theme - while quality.projection read exact. Before this the response showed that shortfall; after the first attempt it hid it. The condition is now strict_compatible, which is the same predicate strict: true refuses on, so the two cannot disagree about whether anything was lost. On the ten real captured screens it stays silent for every screen that has neither a shortfall nor an approximation, which is what the change was for.", + "date": "2026-09-08T17:20:00+09:00" +} diff --git a/crates/devup-mcp/src/server/projection.rs b/crates/devup-mcp/src/server/projection.rs index 6196486..5f0bee3 100644 --- a/crates/devup-mcp/src/server/projection.rs +++ b/crates/devup-mcp/src/server/projection.rs @@ -24,8 +24,8 @@ use super::{ output::{OutputPolicy, OutputTransaction}, parse_scope, quality::{ - AcquisitionQuality, OutputQuality, ProjectionQuality, acquisition_quality, assets_quality, - projection_quality, theme_quality, + AcquisitionQuality, OutputQuality, acquisition_quality, assets_quality, projection_quality, + theme_quality, }, section_candidate_as_explore, section_index_from_payload, }; @@ -412,26 +412,26 @@ pub(super) fn artifact_metadata(artifact: &ArtifactLookup) -> Value { /// response carried before this, none of it actionable. It is attached /// whenever the capture is not clean, and whenever the caller asked for /// diagnostics and therefore wants the detail regardless. -/// Adds `fidelity` only when the conversion was not exact. +/// Adds `fidelity` whenever the report has something `quality` does not say. /// -/// `quality.projection` is the signal a caller acts on; `fidelity` is the -/// drill-down beneath it, at 437 measured bytes, repeated once per screen on -/// a Section export. Tying it to that same grade keeps the two from -/// disagreeing: an exact conversion sends the grade alone, and anything less -/// sends the axes that explain it. +/// Keyed on the report itself, not on `quality.projection`. That was the +/// first attempt and it hid a real defect: `projection_quality` is computed +/// from diagnostics alone, so a coverage shortfall that raises no diagnostic +/// leaves the grade reading `exact` and took the report away with it. +/// Measured on a real 50-node screen, `variables` was 19 of 20 - a `$primary` +/// binding frozen into an exported SVG - while `quality.projection` said +/// `exact`, so the response went from showing the shortfall to hiding it. /// -/// Deliberately not keyed on `strict_compatible`, which also fails on a -/// coverage shortfall that changes nothing about the output - that would put -/// the report back on almost every response while `quality` still read -/// `exact`. `strict: true` keeps using `strict_compatible` to refuse, and -/// returns the same report in the error. +/// `strict_compatible` is the same predicate `strict: true` refuses on, which +/// keeps the two from disagreeing about whether anything was lost. On the ten +/// real captured screens it is true - nothing sent - for every screen with no +/// shortfall and no approximation, which is what this is for. fn attach_fidelity( response: &mut Value, report: &devup_mcp_devup_ui::provenance::FidelityReport, - projection: ProjectionQuality, include_diagnostics: bool, ) { - if include_diagnostics || projection != ProjectionQuality::Exact { + if include_diagnostics || !report.strict_compatible() { response["fidelity"] = json!(report); } } @@ -754,12 +754,7 @@ pub(super) async fn complete_operation( "quality": frame_quality, "tsx": output.tsx }); - attach_fidelity( - &mut frame, - &output.fidelity_report, - frame_quality.projection, - include_diagnostics, - ); + attach_fidelity(&mut frame, &output.fidelity_report, include_diagnostics); attach_completeness_report( &mut frame, frame_quality, @@ -1186,12 +1181,7 @@ pub(super) async fn complete_operation( // point; each Section frame already carries its own. if !section_tsx_projected && let Some(report) = fidelity_reports.first() { let mut carrier = Value::Object(Map::new()); - attach_fidelity( - &mut carrier, - report, - quality.projection, - include_diagnostics, - ); + attach_fidelity(&mut carrier, report, include_diagnostics); if let Some(fidelity) = carrier.get("fidelity") { result.insert("fidelity".to_owned(), fidelity.clone()); } diff --git a/crates/devup-mcp/tests/composite_export.rs b/crates/devup-mcp/tests/composite_export.rs index b21bb78..a3a86d3 100644 --- a/crates/devup-mcp/tests/composite_export.rs +++ b/crates/devup-mcp/tests/composite_export.rs @@ -244,12 +244,30 @@ async fn a_clean_conversion_sends_the_code_and_the_grade_but_not_the_paperwork() assert_eq!(result["quality"]["projection"], "exact"); assert_eq!(result["quality"]["acquisition"], "complete"); assert!(result["tsx"].as_str().unwrap().contains("$primary")); - for silent in ["fidelity", "completenessReport"] { - assert!( - result.get(silent).is_none(), - "{silent} says nothing a clean quality has not already said" - ); - } + // `completenessReport` is keyed on the capture, which is clean here. + assert!( + result.get("completenessReport").is_none(), + "a clean capture says nothing the grade has not already said" + ); + // `fidelity` is keyed on the report itself rather than on + // `quality.projection`, because the grade is computed from diagnostics + // alone: a coverage shortfall that raises none leaves it reading `exact` + // while an axis is short. This fixture is exactly that case, so the + // report is sent - and printing it is the point, since hiding it is what + // the first attempt at this did on a real screen. + let fidelity = result + .get("fidelity") + .expect("a report that disagrees with a clean grade must be sent"); + assert_eq!(result["quality"]["projection"], "exact"); + assert!( + fidelity["variables"]["basisPoints"].as_u64() < Some(10_000) + || fidelity["layout"]["basisPoints"].as_u64() < Some(10_000) + || fidelity["nodes"]["basisPoints"].as_u64() < Some(10_000) + || fidelity["text"]["basisPoints"].as_u64() < Some(10_000) + || fidelity["typography"]["basisPoints"].as_u64() < Some(10_000) + || fidelity["assets"]["basisPoints"].as_u64() < Some(10_000), + "it is sent because an axis is short: {fidelity}" + ); client.cancel().await?; task.await??; From e6c1d19095aef63da14b4fb8a0d304f22175a9e6 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Tue, 8 Sep 2026 17:09:16 +0900 Subject: [PATCH 2/3] fix(devup-ui): stop expecting a token binding baked into a multi-colour asset An asset drawn in one colour becomes a Box masked to its shape with \g\ set from that colour, so a token binding inside it survives as a token. An asset drawn in more than one has no such form: a CSS mask carries alpha only, and an SVG loaded through \\ renders in its own document, where neither currentColor nor a CSS variable reaches it. It becomes an \\ and every colour inside is baked into the file. Counting those bindings as expected-but-missing asks the generator for something no correct implementation could produce. On frame 3997:47884 two play buttons each hide a bound circle behind a white glyph, and that alone put the whole fidelity report on the response with nothing in it anyone could act on: variables read 19 of 20 and the report was sent. Bindings inside a non-mask asset are now left out of the expectation. That screen moves to 17 of 17 and the report is not sent. Across the ten captured screens the expectation drops from 658 to 528 and is fully covered - while six of the ten still send the report for their absolute-layout approximations, so this narrows what is expected rather than silencing what was found. --- .../changepack_log_fidelity_visibility.json | 5 +- crates/devup-mcp-devup-ui/src/codegen/mod.rs | 2 +- crates/devup-mcp-devup-ui/src/provenance.rs | 48 ++++++++++++++++++- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/.changepacks/changepack_log_fidelity_visibility.json b/.changepacks/changepack_log_fidelity_visibility.json index 910cc1d..81edfc2 100644 --- a/.changepacks/changepack_log_fidelity_visibility.json +++ b/.changepacks/changepack_log_fidelity_visibility.json @@ -1,7 +1,8 @@ { "changes": { - "crates/devup-mcp/Cargo.toml": "Patch" + "crates/devup-mcp/Cargo.toml": "Patch", + "crates/devup-mcp-devup-ui/Cargo.toml": "Patch" }, - "note": "Send the fidelity report whenever it disagrees with the grade above it, rather than whenever the grade is not exact. Keying it on quality.projection was wrong in a way that hid a real defect: projection_quality is computed from diagnostics alone, so a coverage shortfall that raises no diagnostic leaves the grade reading exact and took the report away with it. Measured on a real 50-node screen, fidelity.variables was 19 of 20 - a $primary binding on an ellipse whose parent was exported as an SVG, so the theme colour is frozen into the file and cannot follow the theme - while quality.projection read exact. Before this the response showed that shortfall; after the first attempt it hid it. The condition is now strict_compatible, which is the same predicate strict: true refuses on, so the two cannot disagree about whether anything was lost. On the ten real captured screens it stays silent for every screen that has neither a shortfall nor an approximation, which is what the change was for.", + "note": "Send the fidelity report whenever it disagrees with the grade above it, rather than whenever the grade is not exact. Keying it on quality.projection was wrong in a way that hid a real defect: projection_quality is computed from diagnostics alone, so a coverage shortfall that raises no diagnostic leaves the grade reading exact and took the report away with it. Measured on a real 50-node screen, fidelity.variables was 19 of 20 - a $primary binding on an ellipse whose parent was exported as an SVG, so the theme colour is frozen into the file and cannot follow the theme - while quality.projection read exact. Before this the response showed that shortfall; after the first attempt it hid it. The condition is now strict_compatible, which is the same predicate strict: true refuses on, so the two cannot disagree about whether anything was lost. On the ten real captured screens it stays silent for every screen that has neither a shortfall nor an approximation, which is what the change was for. And stop expecting a binding the platform cannot carry. An asset drawn in one colour becomes a Box masked to its shape with bg set, so a token inside it survives as a token; an asset drawn in more than one has no such form, because a CSS mask carries alpha only and an SVG loaded through img src renders in its own document where neither currentColor nor a CSS variable reaches it. It becomes an Image and every colour inside is baked into the file. Counting those bindings as expected-but-missing asked the generator for something no correct implementation could produce: on the measured screen two play buttons each hide a bound circle behind a white glyph, which alone put the whole report on the response with nothing in it anyone could act on. Bindings inside a non-mask asset are excluded from the variables expectation, which on that screen moves it from 19 of 20 to 17 of 17. Across the ten captured screens the expectation drops from 658 to 528 and is fully covered, while six of the ten still send the report for their absolute-layout approximations - so the exclusion narrows what is expected rather than silencing what was found.", "date": "2026-09-08T17:20:00+09:00" } diff --git a/crates/devup-mcp-devup-ui/src/codegen/mod.rs b/crates/devup-mcp-devup-ui/src/codegen/mod.rs index 28f53df..7b34fca 100644 --- a/crates/devup-mcp-devup-ui/src/codegen/mod.rs +++ b/crates/devup-mcp-devup-ui/src/codegen/mod.rs @@ -18,6 +18,6 @@ pub use component::{ normalize_component_name, render_component_registration_snapshot, render_component_source, }; pub(crate) use layout::{derived_padding, placed_by_a_free_layout}; -pub(crate) use style::asset_kind; +pub(crate) use style::{AssetKind, asset_kind}; pub use style::{asset_path, image_fill_path}; pub(crate) use text::escape_jsx_text; diff --git a/crates/devup-mcp-devup-ui/src/provenance.rs b/crates/devup-mcp-devup-ui/src/provenance.rs index bae5e99..a903e0d 100644 --- a/crates/devup-mcp-devup-ui/src/provenance.rs +++ b/crates/devup-mcp-devup-ui/src/provenance.rs @@ -4,7 +4,9 @@ use devup_mcp_figma::{DevupError, ErrorCode, FidelityImpact, Snapshot, discover_ use serde::{Deserialize, Serialize}; use serde_json::json; -use crate::codegen::{CodegenOutput, asset_kind, derived_padding, placed_by_a_free_layout}; +use crate::codegen::{ + AssetKind, CodegenOutput, asset_kind, derived_padding, placed_by_a_free_layout, +}; const START: &str = "\u{e000}DEVUP_PROVENANCE_START:"; const END: &str = "\u{e000}DEVUP_PROVENANCE_END:"; @@ -338,7 +340,11 @@ pub fn validate_fidelity( .is_some_and(|(index, _)| consumed_text_entries.insert(index)) }) .count(); - let variables = variable_sources(snapshot, &semantic_nodes); + let baked = baked_into_assets(snapshot, root_id); + let variables = variable_sources(snapshot, &semantic_nodes) + .into_iter() + .filter(|(node_id, _)| !baked.contains(node_id)) + .collect::>(); let covered_variables = variables .iter() .filter(|(node_id, variable_id)| { @@ -624,6 +630,44 @@ fn semantic_nodes<'a>(snapshot: &'a Snapshot, root_id: &str) -> BTreeSet<&'a str visible } +/// Nodes whose variable bindings no correct generator could write out. +/// +/// An asset drawn in one colour becomes a Box masked to its shape with `bg` +/// set from that colour, so a token binding inside it survives as a token. +/// An asset drawn in more than one has no such form - a CSS mask carries +/// alpha only, and an SVG loaded through `` renders in its own +/// document where neither `currentColor` nor a CSS variable reaches it - so +/// it becomes an `` and every colour inside is baked into the file. +/// +/// Counting those bindings as expected-but-missing asks the generator for +/// something the platform does not offer, and it is not a one-off: measured +/// on a real screen, two play buttons each hid a bound circle behind a white +/// glyph, and the shortfall they produced put the whole fidelity report on +/// the response with nothing in it anyone could act on. +fn baked_into_assets(snapshot: &Snapshot, root_id: &str) -> BTreeSet { + let mut baked = BTreeSet::new(); + let mut pending = vec![root_id.to_owned()]; + while let Some(node_id) = pending.pop() { + let Some(node) = snapshot.nodes.get(&node_id) else { + continue; + }; + // A mask keeps its one colour in CSS, so keep looking inside it. + if !matches!(asset_kind(snapshot, node), None | Some(AssetKind::SvgMask)) { + let mut inside = vec![node_id]; + while let Some(current) = inside.pop() { + let Some(node) = snapshot.nodes.get(¤t) else { + continue; + }; + inside.extend(node.typed_view().child_ids().map(str::to_owned)); + baked.insert(current); + } + continue; + } + pending.extend(node.typed_view().child_ids().map(str::to_owned)); + } + baked +} + fn variable_sources( snapshot: &Snapshot, semantic_nodes: &BTreeSet<&str>, From 73e87f6d19d76250a122929013014e55566e7b83 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Tue, 8 Sep 2026 17:57:07 +0900 Subject: [PATCH 3/3] fix(ci): name the commit when building a release instead of inferring it build.rs infers the build id from git HEAD plus whether the tree is dirty, and on a release build the tree always is: \changepacks update\ moves the version in Cargo.toml and leaves Cargo.lock behind, so the first cargo invocation rewrites the lock and dirties the checkout. Every published binary therefore reported \-dirty\ - precisely the state \--version\ and \--self-check\ exist to tell apart from a developer's working tree. Committing the lock fixed the instance; PR #11 bumps Cargo.toml to 0.3.0 without touching the lock, which brings it straight back. CI knows which commit it is building, so it says so. build.rs already prefers DEVUP_MCP_BUILD_ID over its own inference. A local build still infers, so a dirty working tree still reports itself as one. --- .../changepack_log_fidelity_visibility.json | 2 +- .github/workflows/ci.yml | 14 ++++ README.md | 5 +- crates/devup-mcp/src/server/mod.rs | 5 +- crates/devup-mcp/src/server/tools.rs | 13 ++++ crates/devup-mcp/src/server/validation.rs | 33 +++++++++- crates/devup-mcp/tests/call_cache_resume.rs | 6 +- crates/devup-mcp/tests/composite_export.rs | 66 +++++++++++++++++-- harness/render/scripts/acquire.py | 5 +- 9 files changed, 136 insertions(+), 13 deletions(-) diff --git a/.changepacks/changepack_log_fidelity_visibility.json b/.changepacks/changepack_log_fidelity_visibility.json index 81edfc2..6bd52d3 100644 --- a/.changepacks/changepack_log_fidelity_visibility.json +++ b/.changepacks/changepack_log_fidelity_visibility.json @@ -3,6 +3,6 @@ "crates/devup-mcp/Cargo.toml": "Patch", "crates/devup-mcp-devup-ui/Cargo.toml": "Patch" }, - "note": "Send the fidelity report whenever it disagrees with the grade above it, rather than whenever the grade is not exact. Keying it on quality.projection was wrong in a way that hid a real defect: projection_quality is computed from diagnostics alone, so a coverage shortfall that raises no diagnostic leaves the grade reading exact and took the report away with it. Measured on a real 50-node screen, fidelity.variables was 19 of 20 - a $primary binding on an ellipse whose parent was exported as an SVG, so the theme colour is frozen into the file and cannot follow the theme - while quality.projection read exact. Before this the response showed that shortfall; after the first attempt it hid it. The condition is now strict_compatible, which is the same predicate strict: true refuses on, so the two cannot disagree about whether anything was lost. On the ten real captured screens it stays silent for every screen that has neither a shortfall nor an approximation, which is what the change was for. And stop expecting a binding the platform cannot carry. An asset drawn in one colour becomes a Box masked to its shape with bg set, so a token inside it survives as a token; an asset drawn in more than one has no such form, because a CSS mask carries alpha only and an SVG loaded through img src renders in its own document where neither currentColor nor a CSS variable reaches it. It becomes an Image and every colour inside is baked into the file. Counting those bindings as expected-but-missing asked the generator for something no correct implementation could produce: on the measured screen two play buttons each hide a bound circle behind a white glyph, which alone put the whole report on the response with nothing in it anyone could act on. Bindings inside a non-mask asset are excluded from the variables expectation, which on that screen moves it from 19 of 20 to 17 of 17. Across the ten captured screens the expectation drops from 658 to 528 and is fully covered, while six of the ten still send the report for their absolute-layout approximations - so the exclusion narrows what is expected rather than silencing what was found.", + "note": "Send the fidelity report whenever it disagrees with the grade above it, rather than whenever the grade is not exact. Keying it on quality.projection was wrong in a way that hid a real defect: projection_quality is computed from diagnostics alone, so a coverage shortfall that raises no diagnostic leaves the grade reading exact and took the report away with it. Measured on a real 50-node screen, fidelity.variables was 19 of 20 - a $primary binding on an ellipse whose parent was exported as an SVG, so the theme colour is frozen into the file and cannot follow the theme - while quality.projection read exact. Before this the response showed that shortfall; after the first attempt it hid it. The condition is now strict_compatible, which is the same predicate strict: true refuses on, so the two cannot disagree about whether anything was lost. On the ten real captured screens it stays silent for every screen that has neither a shortfall nor an approximation, which is what the change was for. And stop expecting a binding the platform cannot carry. An asset drawn in one colour becomes a Box masked to its shape with bg set, so a token inside it survives as a token; an asset drawn in more than one has no such form, because a CSS mask carries alpha only and an SVG loaded through img src renders in its own document where neither currentColor nor a CSS variable reaches it. It becomes an Image and every colour inside is baked into the file. Counting those bindings as expected-but-missing asked the generator for something no correct implementation could produce: on the measured screen two play buttons each hide a bound circle behind a white glyph, which alone put the whole report on the response with nothing in it anyone could act on. Bindings inside a non-mask asset are excluded from the variables expectation, which on that screen moves it from 19 of 20 to 17 of 17. Across the ten captured screens the expectation drops from 658 to 528 and is fully covered, while six of the ten still send the report for their absolute-layout approximations - so the exclusion narrows what is expected rather than silencing what was found. Also names the commit outright when CI builds a release, instead of letting build.rs infer it. It infers by asking git for HEAD and whether the tree is dirty, and on a release build the tree always is: changepacks update moves the version in Cargo.toml and leaves Cargo.lock behind, so the first cargo invocation rewrites the lock and dirties the checkout. Every published binary reported its build id as -dirty, which is exactly the state --version and --self-check exist to tell apart from a developer working tree. Committing the lock fixed that instance and the next Update Versions pull request brings it straight back, so the build job now passes DEVUP_MCP_BUILD_ID, which build.rs already prefers. A local build still infers, so a dirty working tree still says so.", "date": "2026-09-08T17:20:00+09:00" } diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9c8d289..35022e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -190,6 +190,20 @@ jobs: SUFFIX: ${{ matrix.suffix }} EXT: ${{ matrix.ext }} OS: ${{ matrix.os }} + # Name the commit outright instead of letting build.rs infer it. + # + # It infers by asking git for HEAD and whether the tree is dirty, + # and on a release build the tree always is: `changepacks update` + # moves the version in Cargo.toml and leaves Cargo.lock behind, so + # the first cargo invocation rewrites the lock and dirties the + # checkout. Every published binary then reported its build id as + # `-dirty`, which is precisely the state `--version` and + # `--self-check` exist to tell apart from a developer's working + # tree. Committing the lock fixed the instance; the next Update + # Versions pull request brings it straight back. + # + # CI knows exactly which commit it is building, so it says so. + DEVUP_MCP_BUILD_ID: ${{ github.sha }} run: | set -euo pipefail for target in $TARGETS; do diff --git a/README.md b/README.md index 082638b..16901ec 100644 --- a/README.md +++ b/README.md @@ -30,10 +30,13 @@ devup-mcp는 Figma Plugin API의 readable data property를 raw JSON으로 보존 | `componentTsx` | `componentTsx` | | `responsiveTsx` | `responsiveTsx`, `responsiveSlots`, (표현 불가한 값이 있으면) `responsiveUnrepresented` | | `devupJson` | `devupJson`, `themeCounts`, `themeCompleteness`, `conflicts`, `unresolvedVariables` | -| `sourceMap` / `rawSnapshot` / `rawPayload` / `assetManifest` / `referencePng` | 같은 이름의 키 | +| `sourceMap` / `assetManifest` / `referencePng` | 같은 이름의 키 | +| `rawSnapshot` / `rawPayload` | 같은 이름의 키 — **`debug: true` 필요** | 그 밖에 항상 붙는 것은 `status`, `quality`, `completeness`, `cache`, `collection`, `source`, `targetKind`, `failures`, `outputPaths`뿐입니다. `fidelity`와 `completenessReport`는 결과가 exact/complete가 **아닐 때**, 또는 `includeDiagnostics: true`일 때만 나옵니다 — 깨끗한 결과에서는 `quality`가 이미 한 말을 되풀이할 뿐이라 빼두었고, 그만큼(측정값 797 B) 매 응답이 가벼워집니다. +`rawSnapshot`과 `rawPayload`는 수집한 디자인을 raw로 담은 것이라 `debug: true` 없이는 거절됩니다. 화면을 구현하는 데는 필요 없습니다 — 실제 캡처 10개 화면에서 tsx가 node·text·typography·asset·layout 기대치를 100% 담고 있습니다. 쓰는 자리는 하나입니다: **화면이 이상해 보일 때 생성기 탓인지 디자인이 원래 그런지 판정하는 것.** 그때는 디자인을 코드 옆에 놓고 읽어야 하고, 그게 이 플래그입니다. + 에러는 호출 자체가 잘못된 경우(`DEVUP_INVALID_INPUT`, 없는 node/파일, 만료·부적합한 `artifactId` 등) JSON-RPC `-32602 INVALID_PARAMS`로, 그 밖의 실패는 `-32603 INTERNAL_ERROR`로 옵니다. 인자를 고쳐 다시 부를 일인지 멈추고 보고할 일인지를 메시지를 파싱하지 않고 구분할 수 있습니다. 정확한 `code`와 `retryable`은 예전처럼 `data`에 그대로 실립니다. devup-mcp는 Figma Remote MCP에 직접 붙습니다 — OAuth discovery, Dynamic Client Registration, PKCE S256, 일시적인 `127.0.0.1` callback을 구현합니다. Figma는 MCP Catalog에 승인된 client의 registration만 허용하므로 등록은 allowlist에 있는 `client_name`으로 이루어집니다(기본값 `Codex`). Figma PAT나 사용자가 만든 OAuth app은 필요하지 않습니다. diff --git a/crates/devup-mcp/src/server/mod.rs b/crates/devup-mcp/src/server/mod.rs index c4fcb2a..7d79598 100644 --- a/crates/devup-mcp/src/server/mod.rs +++ b/crates/devup-mcp/src/server/mod.rs @@ -613,7 +613,8 @@ impl DevupServer { #[tool( description = "Acquire a Figma design once and project any combination of outputs from that one collection; the Figma-to-code entry point. \ Ask only for what you will read: `tsx` is the deliverable, and the response always carries `status`, `quality`, `cache.artifactId`, `collection` and `source` beside it. \ - Each output adds its own keys and nothing else - tsx adds `tsx`; componentTsx adds `componentTsx`; responsiveTsx adds `responsiveTsx`, `responsiveSlots` and, where a width asked for something one tree cannot say, `responsiveUnrepresented`; devupJson adds `devupJson`, `themeCounts`, `themeCompleteness`, `conflicts` and `unresolvedVariables`; sourceMap, rawSnapshot, rawPayload, assetManifest and referencePng each add the key they name. \ + Each output adds its own keys and nothing else - tsx adds `tsx`; componentTsx adds `componentTsx`; responsiveTsx adds `responsiveTsx`, `responsiveSlots` and, where a width asked for something one tree cannot say, `responsiveUnrepresented`; devupJson adds `devupJson`, `themeCounts`, `themeCompleteness`, `conflicts` and `unresolvedVariables`; sourceMap, assetManifest and referencePng each add the key they name. \ + `rawSnapshot` and `rawPayload` are the collected design in raw form and need `debug: true`. Use them for one question only - a screen looks wrong and you must decide whether the generator is at fault or the design says so - never to implement, since the tsx already carries what they carry. \ `fidelity` and `completenessReport` appear only when the result is not exact or complete, or when includeDiagnostics is set. \ tsx expands every instance into primitives while componentTsx keeps them as references, so requesting both gives the same screen twice and the difference between them is each component's body. responsiveTsx merges every width the capture carries into one module whose differing values are devup-ui responsive arrays, and is produced whenever there is more than one width. \ Reuse a previous acquisition with `artifactId` from `cache` to project further outputs without calling Figma again.", @@ -623,7 +624,7 @@ impl DevupServer { &self, Parameters(input): Parameters, ) -> Result { - validate_outputs(&input.outputs).map_err(to_mcp_error)?; + validate_outputs(&input.outputs, input.debug).map_err(to_mcp_error)?; if !input.asset_requests.is_empty() && !input.outputs.iter().any(|output| output == "assetManifest") { diff --git a/crates/devup-mcp/src/server/tools.rs b/crates/devup-mcp/src/server/tools.rs index 9b7c3eb..f1e430c 100644 --- a/crates/devup-mcp/src/server/tools.rs +++ b/crates/devup-mcp/src/server/tools.rs @@ -56,6 +56,19 @@ pub struct FigmaExportInput { /// by default when driven as a library. #[serde(default = "default_true")] pub asset_names_per_node: bool, + /// Opens the two outputs that describe the design rather than the screen, + /// `rawSnapshot` and `rawPayload`. + /// + /// For one question only: a screen looks wrong, and it has to be decided + /// whether the generator is at fault or the design says so. Answering + /// that means reading the collected design beside the generated code. + /// + /// Nothing that implements a screen needs them. Measured across ten real + /// captured screens the tsx already carries 100% of the nodes, text, + /// typography, assets and layout the design expects, and on one screen + /// asking for them by habit spent about eight bytes for every one of code. + #[serde(default)] + pub debug: bool, #[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 dc5bf0f..8b84f90 100644 --- a/crates/devup-mcp/src/server/validation.rs +++ b/crates/devup-mcp/src/server/validation.rs @@ -123,7 +123,25 @@ fn collection_scope_rank(scope: CollectionScope) -> u8 { } } -pub(super) fn validate_outputs(outputs: &[String]) -> Result<(), DevupError> { +/// The two outputs that answer a question about the generator rather than +/// contributing to the screen. +/// +/// They are the design as it was collected, in raw form. Nothing that +/// implements a screen needs them - the tsx already carries what they carry, +/// measured across ten real captured screens at 100% of nodes, text, +/// typography, assets and layout. What they are for is the other question: +/// the UI looks wrong, and someone has to decide whether the generator is at +/// fault or the design says so. Answering that means reading the design +/// beside the code, which is exactly this. +/// +/// Left in the ordinary `outputs` list they were requested as a matter of +/// course - the server's own instructions used to say to take `rawSnapshot` +/// every time, which on one measured screen spent about eight bytes for +/// every one of code. `debug` is the door: closed for implementation, open +/// when a defect is being adjudicated. +pub(crate) const DIAGNOSIS_OUTPUTS: [&str; 2] = ["rawSnapshot", "rawPayload"]; + +pub(super) fn validate_outputs(outputs: &[String], debug: bool) -> Result<(), DevupError> { if outputs.is_empty() { return Err(DevupError::new( ErrorCode::DevupInvalidInput, @@ -142,6 +160,19 @@ pub(super) fn validate_outputs(outputs: &[String]) -> Result<(), DevupError> { false, )); } + if !debug && DIAGNOSIS_OUTPUTS.contains(&output.as_str()) { + return Err(DevupError::new( + ErrorCode::DevupInvalidInput, + format!( + "{output} is the collected design in raw form, for deciding whether a \ + screen that looks wrong is the generator's fault or the design's. It is \ + not needed to implement anything - the tsx already carries what it \ + carries - and requesting it by habit is most of the response. Pass \ + debug: true to read it." + ), + false, + )); + } } Ok(()) } diff --git a/crates/devup-mcp/tests/call_cache_resume.rs b/crates/devup-mcp/tests/call_cache_resume.rs index 32d549b..a98d517 100644 --- a/crates/devup-mcp/tests/call_cache_resume.rs +++ b/crates/devup-mcp/tests/call_cache_resume.rs @@ -92,7 +92,7 @@ async fn attempt(upstream: Arc, cache: PathBuf) -> anyhow::Res let arguments: Map = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", "allScreens": true, - "outputs": ["rawSnapshot"] + "outputs": ["rawSnapshot"], "debug": true }) .as_object() .cloned() @@ -209,7 +209,7 @@ async fn a_refusal_that_arrives_as_an_answer_is_not_banked() -> anyhow::Result<( let arguments: Map = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", "allScreens": true, - "outputs": ["rawSnapshot"] + "outputs": ["rawSnapshot"], "debug": true }) .as_object() .cloned() @@ -251,7 +251,7 @@ async fn nothing_is_banked_unless_a_directory_was_named() -> anyhow::Result<()> let arguments: Map = json!({ "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=10-1", "allScreens": true, - "outputs": ["rawSnapshot"] + "outputs": ["rawSnapshot"], "debug": true }) .as_object() .cloned() diff --git a/crates/devup-mcp/tests/composite_export.rs b/crates/devup-mcp/tests/composite_export.rs index a3a86d3..73ba38d 100644 --- a/crates/devup-mcp/tests/composite_export.rs +++ b/crates/devup-mcp/tests/composite_export.rs @@ -208,6 +208,61 @@ async fn call_result( .await?) } +/// The design in raw form is behind a door marked debug. +/// +/// `rawSnapshot` and `rawPayload` describe the design rather than the screen. +/// Nothing that implements a screen needs them, and left in the ordinary list +/// they were taken as a matter of course - the server's own instructions used +/// to say to take `rawSnapshot` every time, which on a measured screen spent +/// about eight bytes for every one of code. They stay reachable, because +/// deciding whether a screen that looks wrong is the generator's fault means +/// reading the design beside the code, and that is what `debug` is for. +#[tokio::test] +async fn the_collected_design_is_reachable_only_in_debug() -> anyhow::Result<()> { + let upstream = Arc::new(FastFixtureUpstream::complete()); + let server = DevupServer::new(Services::new(Arc::new(ConnectedAuth), upstream)); + let (server_transport, client_transport) = tokio::io::duplex(256 * 1024); + let task = tokio::spawn(async move { + server.serve(server_transport).await?.waiting().await?; + anyhow::Ok(()) + }); + let client = ().serve(client_transport).await?; + let url = "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2"; + + for output in ["rawSnapshot", "rawPayload"] { + let refused = call_result( + &client, + "devup_figma_export", + json!({ "url": url, "outputs": [output], "scope": "node" }), + ) + .await + .expect_err("the design in raw form is not part of implementing a screen"); + let message = refused.to_string(); + assert!( + message.contains("debug: true"), + "the refusal has to say which door to open: {message}" + ); + assert!( + message.contains(output), + "and which output it is about: {message}" + ); + } + + // Asking for it deliberately still works, and still costs what it costs. + let debugging = call( + &client, + "devup_figma_export", + json!({ "url": url, "outputs": ["tsx", "rawSnapshot"], "scope": "node", "debug": true }), + ) + .await?; + assert!(debugging["rawSnapshot"]["nodes"].is_object()); + assert!(debugging["tsx"].as_str().is_some_and(|tsx| !tsx.is_empty())); + + client.cancel().await?; + task.await??; + Ok(()) +} + /// A clean conversion should carry the code and the grade, and not the /// paperwork underneath the grade. /// @@ -293,6 +348,7 @@ async fn one_acquisition_projects_all_outputs_and_artifact_reuse_is_zero_call() json!({ "url": url, "outputs": ["tsx", "devupJson", "rawSnapshot", "rawPayload", "sourceMap", "assetManifest"], + "debug": true, "scope": "node", "includeDiagnostics": true }), @@ -426,6 +482,7 @@ async fn one_acquisition_projects_all_outputs_and_artifact_reuse_is_zero_call() json!({ "url": url, "outputs": ["rawSnapshot"], + "debug": true, "scope": "node", "refresh": true }), @@ -799,10 +856,11 @@ async fn strict_export_rejects_partial_payload_before_projection() -> anyhow::Re .call_tool( CallToolRequestParams::new("devup_figma_export").with_arguments( json!({ - "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", - "outputs": ["rawSnapshot"], - "strict": true - }) + "url": "https://www.figma.com/design/FileKey123/Fixture?node-id=1-2", + "outputs": ["rawSnapshot"], + "debug": true, + "strict": true + }) .as_object() .cloned() .unwrap(), diff --git a/harness/render/scripts/acquire.py b/harness/render/scripts/acquire.py index c47c74b..54e0710 100644 --- a/harness/render/scripts/acquire.py +++ b/harness/render/scripts/acquire.py @@ -184,8 +184,11 @@ def acquire(server, name, target, manifest): # conflicts. outputs.append("devupJson") paths["devupJson"] = theme_path + # rawSnapshot describes the design rather than the screen, so it needs + # debug: true. This harness is the case that flag is for - it compares + # what the browser drew against what the design says. body = server.export({"url": url_for(target, frame), "outputs": outputs, "scope": "node", - "outputPaths": paths, "includeDiagnostics": True}) + "outputPaths": paths, "includeDiagnostics": True, "debug": True}) print(f" {frame}: status={body.get('status')} quality={body.get('quality')}", flush=True) # The module refers to assets by layer name; the manifest by node id. with open(os.path.join(HARNESS, paths["rawSnapshot"]), encoding="utf-8") as handle: