feat(surfpool,skill): auto-discover preset IDLs and fix solana-add-idl template gaps

.claude/skills/solana-add-idl/SKILL.md

@@ -1,13 +1,40 @@
 ---
 name: solana-add-idl
-description: Add a new Solana program IDL-based visualizer preset. Fetches IDL on-chain or accepts user-provided IDL, then scaffolds config.rs, mod.rs, and registers the preset.
+description: Scaffold a structural IDL-driven Solana visualizer preset. Fetches the IDL (on-chain or user-provided), drops it into the preset directory, and writes a generic decoder. Registration is fully reflective via build.rs. Semantic refinement (domain labels, token resolution) is a follow-up workflow.
 user-invocable: true
 ---
 
 # Add Solana IDL Visualizer Preset
 
 You are scaffolding a new Solana program visualizer preset from an Anchor IDL.
 
+## Scope: what this skill produces and doesn't produce
+
+This skill scaffolds a **structurally correct, semantically generic** preset.
+
+What you get:
+
+- Binary instruction decoded against the IDL via `parse_instruction_with_idl`
+- Each on-chain account paired with its IDL-declared name
+- Each instruction argument shown as a `text` field with the raw decoded value
+- Auto-registered in `available_visualizers()` and `PRESET_IDLS` by `build.rs` reflection — no edits to `presets/mod.rs` or any test file
+- Crash-safety auto-covered by `tests/fuzz_idl_parsing.rs` (proptest, generative), `fuzz/fuzz_targets/` (cargo-fuzz, generative), and `tests/surfpool_fuzz.rs::surfpool_preset_idls` (reflective)
+
+What it deliberately does **not** produce:
+
+- Domain-specific labels — e.g. `"Swap 1.5 USDC for 0.001 SOL"` rather than `in_token=Pubkey(...), amount_in=1500000, ...`
+- Token metadata resolution (mint decimals, symbol lookups) — amounts render as raw integers
+- Per-instruction display logic — every instruction goes through the same generic path
+- Cross-instruction correlation (e.g. CPI inner-instruction handling)
+- Account-role disambiguation beyond IDL parameter names
+- Semantic correctness assertions in `tests/semantic_pipeline.rs` — those are program-specific and hand-written
+
+The skill's output is the equivalent of a typed-decoder dump: correct, but not yet wallet-readable.
+
+For a **fully semantic** preset to model after, read `src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs`. It hand-rolls a `JupiterSwapInstruction` enum, resolves token metadata via `get_token_info`, and uses format strings like `"Swap {amount} {in_token} for {amount} {out_token}"`. That's the destination; this skill produces the starting point.
+
+Semantic refinement is intended as a separate workflow (planned: `solana-refine-idl-preset` skill). Until that exists, contributors who want wallet-readable output extend the generated `mod.rs` by hand using `jupiter_swap` as the reference. See **Step 8: What's next** at the end of this skill.
+
 ## Step 1: Gather Information
 
 Ask the user for:
@@ -86,29 +113,30 @@ impl SolanaIntegrationConfig for {PascalName}Config {
 
 ### File: `mod.rs`
 
-Use the squads_multisig preset as a template: `src/chain_parsers/visualsign-solana/src/presets/squads_multisig/mod.rs`
+Use `src/chain_parsers/visualsign-solana/src/presets/unknown_program/mod.rs` as the working reference for the IDL parsing pattern — it is the preset that actually exercises `parse_instruction_with_idl` against a runtime-supplied IDL today. Your preset is the same pattern with the IDL **embedded at compile time** and the program ID hardcoded.
+
+Substitutions to make when adapting it:
+- **Hardcode the program ID const** at the top of `mod.rs`:
+  ```rust
+  pub(crate) const {SCREAMING_SNAKE}_PROGRAM_ID: &str = "{base58_program_id}";
+  ```
+  This is what `config.rs` resolves via `use super::{SCREAMING_SNAKE}_PROGRAM_ID;`. See `presets/jupiter_swap/mod.rs` line ~24 for the canonical placement.
+- **Embed the IDL** via `const IDL_JSON: &str = include_str!("{snake_name}.json");` and replace any runtime `idl_registry.get_idl(...)` lookup with `decode_idl_data(IDL_JSON)?`.
+- **Rename the visualizer/config/static**: `UnknownProgramVisualizer` → `{PascalName}Visualizer`, `UnknownProgramConfig` → `{PascalName}Config`, `UNKNOWN_PROGRAM_CONFIG` → `{SCREAMING_SNAKE}_CONFIG`.
+- **`kind()`** returns your chosen `VisualizerKind` variant: `VisualizerKind::{Kind}("{display_name}")`.
+- **Drop the no-IDL fallback path** (`create_unknown_program_preview_layout`) — for an IDL-driven preset, return `Err(VisualSignError::DecodeError(...))` if parsing fails. Do not display raw bytes as a substitute.
 
-Read that file for the exact structure, then generate a generic version with these substitutions:
-- Replace `SquadsMultisig` / `squads_multisig` / `SQUADS_MULTISIG` with the appropriate casing of the new program name
-- Replace the program ID string with the new program address
-- Replace `"Squads Multisig"` display strings with `{display_name}`
-- Replace IDL file reference: `include_str!("{snake_name}.json")`
-- Keep the `kind()` method returning the user's chosen `VisualizerKind` variant with `display_name` as the `&'static str` argument
+**Building `named_accounts` — what the IDL gives you and what you build manually**
 
-**Important — generic IDL pattern only:**
-- DO NOT copy any squads-specific code (e.g. `VaultTransactionMessage` decoding, inner instruction handling)
-- The generic scaffold uses `build_named_accounts`, `build_parsed_fields`, `build_fallback_fields`, `append_raw_data`, `format_arg_value` — all of which work with any IDL
-- The parse function should: check `data.len() < 8`, load IDL, call `parse_instruction_with_idl`, call `build_named_accounts`, return a struct with parsed data + named accounts
+`parse_instruction_with_idl` returns a `SolanaParsedInstructionData` whose `named_accounts` field is empty by default. There is no `build_named_accounts` helper in `solana_parser`; you build the map yourself by matching the on-chain instruction's accounts against the IDL instruction's account list, in order. The reference loop is in `unknown_program::try_parse_with_idl` (search for `named_accounts` in that file). Copy that loop verbatim — it is the supported pattern.
 
-**Required imports** (at top of module, NOT inside functions):
+**Required imports** (at top of module, NOT inside functions; only symbols that actually exist in the current dependency graph):
 ```rust
 use crate::core::{
     InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind,
 };
 use config::{PascalName}Config;
-use solana_parser::{
-    Idl, SolanaParsedInstructionData, decode_idl_data, parse_instruction_with_idl,
-};
+use solana_parser::{SolanaParsedInstructionData, decode_idl_data, parse_instruction_with_idl};
 use std::collections::HashMap;
 use visualsign::errors::VisualSignError;
 use visualsign::field_builders::{create_raw_data_field, create_text_field};
@@ -119,20 +147,37 @@ use visualsign::{
 ```
 
 **Required tests** (in `#[cfg(test)] mod tests`):
-- `test_{snake_name}_idl_loads` — IDL loads and has instructions
-- `test_{snake_name}_idl_has_discriminators` — every instruction has an 8-byte discriminator
-- `test_unknown_discriminator_returns_error` — garbage 9-byte data returns error
-- `test_short_data_returns_error` — 3-byte data returns error
+- `test_{snake_name}_idl_loads` — `decode_idl_data(IDL_JSON)` succeeds and `instructions` is non-empty
+- `test_{snake_name}_idl_has_discriminators` — every instruction in the IDL has an 8-byte discriminator
+
+Crash-safety against unknown discriminators / short data is **already covered**: by `tests/fuzz_idl_parsing.rs` (proptest, generative — exercises arbitrary discriminator/data combinations) and by `tests/surfpool_fuzz.rs::surfpool_preset_idls` (auto-iterates `PRESET_IDLS`). Do not duplicate those assertions in the preset's own test module.
+
+## Step 4: Registration is automatic — nothing to edit
+
+`presets/mod.rs` is generated by `build.rs` (it `include!`s a file that emits `#[path = ...] pub mod <name>;` per direct subdirectory of `src/presets/` containing a `mod.rs`). Drop your preset directory in place; the next build picks it up.
+
+`build.rs` also discovers your `{PascalName}Visualizer` for `available_visualizers()` and (because Step 2 saved an IDL JSON) adds an entry to `PRESET_IDLS`.
 
-## Step 4: Register in presets/mod.rs
+Skip ahead — there's no edit to make in this step.
 
-Add `pub mod {snake_name};` to `src/chain_parsers/visualsign-solana/src/presets/mod.rs`.
+## Step 5: Test coverage — what's auto-discovered, what's program-specific
 
-**Keep entries in alphabetical order.** The existing entries are sorted — insert the new module in the correct position.
+You do **not** need to edit any test file for crash-safety coverage. The harness picks up the new IDL by reflection:
 
-No other registration is needed. `build.rs` auto-discovers `{PascalName}Visualizer` from any directory under `src/presets/`.
+- **`build.rs`** emits `pub const PRESET_IDLS: &[(&str, &str)]` from `src/presets/<name>/<name>.json` (saved in Step 2). The slice is re-exported from the library.
+- **`tests/surfpool_fuzz.rs::surfpool_preset_idls`** iterates `PRESET_IDLS` and runs each through `run_idl_roundtrip` against a `surfpool` mainnet fork (decode IDL → build synthetic tx with the first instruction's discriminator → convert → assert non-empty payload). Picked up on every run with no test-file edit.
+- **Proptest (`tests/fuzz_idl_parsing.rs`)** is *generative*: strategies in `solana_parser_fuzz_core::proptest` synthesize arbitrary IDL shapes and feed them through `decode_idl_data` / `parse_instruction_with_idl`. Covers your IDL structurally without registration.
+- **cargo-fuzz (`fuzz/fuzz_targets/`)** is also *generative* — random byte streams through `transaction_string_to_visual_sign`. No per-IDL registration.
 
-## Step 5: Code Quality
+CI: `surfpool_preset_idls` is `#[ignore]`. It runs when the PR carries the `surfpool` label (see `.github/workflows/surfpool-solana.yml`); local runs need `HELIUS_API_KEY`.
+
+### Semantic correctness is NOT auto-covered
+
+The auto-roundtrip only asserts the converter doesn't crash. It does **not** assert the displayed fields look correct semantically — that's by design (this skill produces a generic decoder, not a semantic one).
+
+If the preset needs CI-level semantic guarantees (specific label text, amount formatting, multi-instruction flows, fixture-based snapshot expectations), add a hand-written test in `tests/semantic_pipeline.rs` modelled after the existing `RAYDIUM_IDL` / `ORCA_IDL` blocks. Otherwise, ship as-is — semantic refinement is a separate workflow (see Step 8).
+
+## Step 6: Code Quality
 
 Follow these rules in all generated code:
 - `use` statements at top of module, never inside functions
@@ -141,7 +186,7 @@ Follow these rules in all generated code:
 - ASCII only in user-visible strings: `>=` not `≥`, `->` not `→`
 - Rust edition 2024 on nightly
 
-## Step 6: Verify
+## Step 7: Verify
 
 Run these commands and fix any issues:
 
@@ -153,3 +198,30 @@ make -C src test
 ```
 
 All must pass before the task is complete.
+
+To confirm the surfpool roundtrip picked up the new preset's IDL via auto-discovery, run:
+
+```bash
+cargo build -p visualsign-solana
+grep -- '"{snake_name}"' src/chain_parsers/visualsign-solana/target/debug/build/visualsign-solana-*/out/preset_idls.rs
+```
+
+The `PRESET_IDLS` slice should contain a `("{snake_name}", include_str!(...))` entry. If it doesn't, the IDL JSON file is at the wrong path — `build.rs` looks for exactly `src/presets/{snake_name}/{snake_name}.json`.
+
+## Step 8: What's next — semantic refinement (optional, follow-up)
+
+Your preset compiles, registers, and survives a roundtrip. A wallet user signing one of these transactions will, however, see raw arg names and integer values, not a recognizable summary. The skill's scope ends here. To make the preset wallet-readable, three options:
+
+1. **Ship as-is.** For low-traffic programs or where structural display is enough, this is acceptable — the new preset is strictly better than the `unknown_program` fallback.
+
+2. **Hand-extend the generated `mod.rs`**, modelled after `presets/jupiter_swap/mod.rs`. The patterns to copy:
+   - Replace the wildcard `"*": ["*"]` in `config.rs` with explicit instruction names so each instruction can be dispatched separately.
+   - Introduce a `{PascalName}Instruction` enum with one variant per IDL instruction you care about. See `JupiterSwapInstruction` for the shape (named fields like `in_token`, `out_token`, `slippage_bps`).
+   - Add a `parse_{snake_name}_instruction` helper that dispatches on the 8-byte discriminator and decodes args into the enum.
+   - Add a `format_{snake_name}_instruction` helper that turns the enum into a human string. Use `get_token_info` from `crate::utils` to resolve mint decimals and symbols for amount fields.
+   - Replace generic `create_text_field` calls with semantic ones — `create_amount_field` for token quantities, `create_address_field` for accounts you want clickable in the UI.
+   - Add a fixture test in `tests/semantic_pipeline.rs` asserting the formatted output for one or two real on-chain transactions.
+
+3. **Wait for `solana-refine-idl-preset`** — a planned follow-up skill that automates the structural-to-semantic transition. Tracked as future work; not yet available.
+
+Until option 3 exists, option 2 is the path. The structural decoder this skill produced is the scaffolding the semantic layer goes on top of, not a replacement for it.

src/chain_parsers/visualsign-solana/build.rs

@@ -24,6 +24,29 @@ fn main() {
     );
 
     fs::write(out_dir.join("generated_visualizers.rs"), code).unwrap();
+
+    let preset_idls = collect_preset_idls();
+    let preset_idls_code = format!(
+        "/// Preset IDLs auto-discovered from `src/presets/<name>/<name>.json`.\n\
+         ///\n\
+         /// Generated by `build.rs`. Drop a JSON file at the matching path and\n\
+         /// it appears here on the next build -- no manual registration. Used\n\
+         /// by the surfpool roundtrip test to exercise every embedded IDL.\n\
+         pub const PRESET_IDLS: &[(&str, &str)] = &[\n    {}\n];\n",
+        preset_idls.join(",\n    ")
+    );
+    fs::write(out_dir.join("preset_idls.rs"), preset_idls_code).unwrap();
+
+    let preset_mods = collect_preset_mods();
+    let preset_mods_code = format!(
+        "// Auto-generated by `build.rs`. One `pub mod` per directory under\n\
+         // `src/presets/` that contains a `mod.rs`. Drop in a new preset\n\
+         // directory and it shows up here on the next build -- no manual\n\
+         // edit of `presets/mod.rs` required.\n\
+         {}\n",
+        preset_mods.join("\n")
+    );
+    fs::write(out_dir.join("presets_mod.rs"), preset_mods_code).unwrap();
 }
 
 fn collect_visualizers() -> Vec<String> {
@@ -68,6 +91,71 @@ fn collect_visualizers() -> Vec<String> {
         .collect()
 }
 
+fn collect_preset_mods() -> Vec<String> {
+    let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap();
+    let presets_dir = PathBuf::from(&manifest_dir).join("src/presets");
+    let Ok(entries) = fs::read_dir(&presets_dir) else {
+        return Vec::new();
+    };
+    let mut mods: Vec<String> = entries
+        .filter_map(|entry| {
+            let path = entry.ok()?.path();
+            if !path.is_dir() {
+                return None;
+            }
+            // Only emit `pub mod` for directories that actually contain a
+            // mod.rs. An empty subdirectory would otherwise produce a
+            // confusing compile error pointing at generated code.
+            let mod_rs = path.join("mod.rs");
+            if !mod_rs.exists() {
+                return None;
+            }
+            let dir_name = path.file_name()?.to_str()?.to_string();
+            // The generated file lives in OUT_DIR; `mod` resolution is
+            // relative to *its* location, so without `#[path]` rustc looks
+            // for `OUT_DIR/<name>.rs`. The absolute path attribute pins
+            // resolution to the real source tree.
+            Some(format!(
+                "#[path = \"{path}\"]\npub mod {dir_name};",
+                path = mod_rs.display()
+            ))
+        })
+        .collect();
+    mods.sort();
+    mods
+}
+
+fn collect_preset_idls() -> Vec<String> {
+    let manifest_dir = env::var("CARGO_MANIFEST_DIR").unwrap();
+    let presets_dir = PathBuf::from(&manifest_dir).join("src/presets");
+    let Ok(entries) = fs::read_dir(&presets_dir) else {
+        return Vec::new();
+    };
+    let mut idls: Vec<String> = entries
+        .filter_map(|entry| {
+            let path = entry.ok()?.path();
+            if !path.is_dir() {
+                return None;
+            }
+            let dir_name = path.file_name()?.to_str()?.to_string();
+            let json_path = path.join(format!("{dir_name}.json"));
+            if !json_path.exists() {
+                return None;
+            }
+            // include_str! takes a literal; emit the absolute path as-is. The
+            // `cargo:rerun-if-changed=src/presets` directive triggers a rebuild
+            // when the JSON file is added, removed, or modified.
+            Some(format!(
+                "(\"{name}\", include_str!(\"{path}\"))",
+                name = dir_name,
+                path = json_path.display()
+            ))
+        })
+        .collect();
+    idls.sort();
+    idls
+}
+
 fn to_pascal_case(s: &str) -> String {
     s.split('_')
         .map(|w| {
@@ -117,4 +205,68 @@ mod tests {
             "Should have at least one visualizer"
         );
     }
+
+    #[test]
+    fn test_collect_preset_mods_emits_path_attribute_per_preset() {
+        // Every preset directory with a mod.rs should produce one
+        // `#[path = ...] pub mod <name>;` line. Without the path
+        // attribute, `include!`'d `pub mod` would resolve relative to
+        // OUT_DIR and break the build.
+        let mods = collect_preset_mods();
+        assert!(!mods.is_empty(), "Should discover at least one preset");
+        for line in &mods {
+            assert!(
+                line.starts_with("#[path = \""),
+                "Each entry must pin the source path: {line}"
+            );
+            assert!(
+                line.contains("pub mod "),
+                "Each entry must declare a module: {line}"
+            );
+        }
+        // Sorted so diffs stay stable as presets are added.
+        let mut sorted = mods.clone();
+        sorted.sort();
+        assert_eq!(mods, sorted, "collect_preset_mods output must be sorted");
+    }
+
+    #[test]
+    fn test_collect_preset_mods_skips_dirs_without_mod_rs() {
+        // The current preset tree has a mod.rs in every directory.
+        // If any future scaffolding leaves a JSON-only directory behind,
+        // this guard prevents it from producing a confusing build break.
+        let mods = collect_preset_mods();
+        for line in &mods {
+            // Extract the path between #[path = "..."]
+            let start = line.find('"').unwrap() + 1;
+            let end = line[start..].find('"').unwrap() + start;
+            let path = &line[start..end];
+            assert!(
+                std::path::Path::new(path).exists(),
+                "Generated #[path] points at a non-existent file: {path}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_collect_preset_idls_returns_only_dirs_with_matching_json() {
+        // Currently no preset ships an embedded IDL JSON, so the slice
+        // is expected to be empty. When the first preset adds one, this
+        // test will need updating -- which is the point: it locks in
+        // the discovery contract.
+        let idls = collect_preset_idls();
+        for entry in &idls {
+            assert!(
+                entry.starts_with('('),
+                "Entries must be tuple-shaped: {entry}"
+            );
+            assert!(
+                entry.contains("include_str!"),
+                "Entries must embed JSON: {entry}"
+            );
+        }
+        let mut sorted = idls.clone();
+        sorted.sort();
+        assert_eq!(idls, sorted, "collect_preset_idls output must be sorted");
+    }
 }