From 546996b713b1aef66dadabd9ebc27ad6560e1d24 Mon Sep 17 00:00:00 2001 From: Valentino Zegna Date: Fri, 8 May 2026 16:03:13 -0700 Subject: [PATCH] chore: track plans/ in repo and add INDEX Promote plans/ from gitignored scratch space to a tracked, indexed folder so design notes are visible to reviewers. Migrates seven existing plans (cloud-storage-readiness, run_erc, xnet-depth-limit, power-net-stop-pattern, relative_path, altium-visual-data, dsn-parser-audit), drops the shelved schematic-renderer experiment, and adds INDEX.md as the entry point. --- .gitignore | 1 - plans/INDEX.md | 24 ++ plans/altium-visual-data.md | 37 ++ plans/cloud-storage-readiness.md | 576 +++++++++++++++++++++++++++++++ plans/dsn-parser-audit.md | 193 +++++++++++ plans/power-net-stop-pattern.md | 54 +++ plans/relative_path.md | 114 ++++++ plans/run_erc.md | 91 +++++ plans/xnet-depth-limit.md | 109 ++++++ 9 files changed, 1198 insertions(+), 1 deletion(-) create mode 100644 plans/INDEX.md create mode 100644 plans/altium-visual-data.md create mode 100644 plans/cloud-storage-readiness.md create mode 100644 plans/dsn-parser-audit.md create mode 100644 plans/power-net-stop-pattern.md create mode 100644 plans/relative_path.md create mode 100644 plans/run_erc.md create mode 100644 plans/xnet-depth-limit.md diff --git a/.gitignore b/.gitignore index 8ffb9ed..25bad62 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,5 @@ telemetry-*.zip # Private files and plans private/ references/ -plans/ .plans/ tmp/ \ No newline at end of file diff --git a/plans/INDEX.md b/plans/INDEX.md new file mode 100644 index 0000000..bac4300 --- /dev/null +++ b/plans/INDEX.md @@ -0,0 +1,24 @@ +# Plans Index + +Living docs for upcoming or in-flight work, plus reference audits. Each +entry lists status against the codebase as of the last review. + +| Plan | Topic | Status | +|---|---|---| +| [cloud-storage-readiness.md](./cloud-storage-readiness.md) | Decouple file I/O from parsers; add `Storage` interface + GCS adapter so `gs://` URIs work alongside local paths | Proposed | +| [run_erc.md](./run_erc.md) | New `run_erc` MCP tool for net-level electrical rule checks (single_pin, testpoint_only, unnamed, testpoint_stub) | Proposed | +| [xnet-depth-limit.md](./xnet-depth-limit.md) | Add `max_depth` parameter to `query_xnet_*` tools, plus `max_depth_reached` and `frontier_nets` response metadata | Proposed | +| [power-net-stop-pattern.md](./power-net-stop-pattern.md) | Switch `POWER_NET_PATTERN` / `STOP_NET_PATTERN` keywords (VCC/VDD/VBAT/VBUS/VSYS) from anchored prefix to substring matching so prefixed rails like `CC1310_VDD` stop traversal | Proposed | +| [relative_path.md](./relative_path.md) | Auto-discovery + ID-based design access: `list_designs` returns relative-path IDs that all other tools accept instead of absolute paths | Proposed | +| [altium-visual-data.md](./altium-visual-data.md) | Extract non-electrical visual/drawing data (lines, polylines, junctions, sheet metadata, etc.) from `.SchDoc` for future schematic rendering | Proposed | +| [dsn-parser-audit.md](./dsn-parser-audit.md) | Audit comparing our DSN TypeScript parser against the C++ `OpenOrCadParser` reference. Documents semantic extensions, gaps, and risks | Reference | + +## Conventions + +- One file per plan, named with kebab-case where practical. +- Each plan should open with a `## Context` section explaining *why* the + change is needed before describing *what* to build. +- Plans stay in this folder until landed. After implementation, either + delete the file (if the plan is fully captured by the code + commit + history) or move to `docs/` if the writeup has long-term reference value + (the DSN audit is an example of the latter). diff --git a/plans/altium-visual-data.md b/plans/altium-visual-data.md new file mode 100644 index 0000000..93fc669 --- /dev/null +++ b/plans/altium-visual-data.md @@ -0,0 +1,37 @@ +# Altium Schematic Visual Data Extraction + +**Goal**: Extract non-electrical visual/drawing data from `.SchDoc` files for future schematic rendering. + +**Context**: The current Altium parser (`src/parsers/altium/`) only extracts electrical data (components, pins, nets) into the unified `ParsedNetlist` model. However, `.SchDoc` files contain rich visual information that could enable schematic rendering in a browser (e.g., in `pcb-viewer` or a new `schematic-viewer`). + +**Reference**: The [Altium-Schematic-Parser](https://github.com/a3ng7n/Altium-Schematic-Parser) project demonstrates raw record-level extraction of all 51 record types, including drawing primitives. It can serve as a reference for what data is available. + +**Data to extract** (record types not currently used): + +| Record | Type | Visual Data | +|--------|------|-------------| +| 4 | Annotation | Text boxes with position, color, font | +| 5 | Bezier | Bezier curve coordinates | +| 6 | Polyline | Line segments with X/Y coordinates, color, width | +| 7 | Polygon | Filled polygonal regions | +| 8 | Ellipse | Elliptical shapes | +| 10 | Round Rectangle | Rounded rectangles | +| 11 | Elliptical Arc | Arc segments | +| 12 | Arc | Circular arcs | +| 13 | Line | Simple lines | +| 14 | Rectangle | Rectangular shapes | +| 28 | Text Frame | Text with bounding box | +| 29 | Junction | Wire junction dots | +| 30 | Image | Embedded images | +| 31 | Sheet | Page size, grid, title block, border | +| 32 | Sheet Name | Sheet title | +| 33 | Sheet File Name | File reference | +| 34 | Designator | Component designator text with position/font | +| 39 | Template | Title block template reference | +| 44/45 | Model Container/Reference | Symbol geometry from library | + +**Notes**: +- This data is NOT for MCP tool responses; it is for rendering schematics visually. +- Parsing should produce a separate visual model alongside `ParsedNetlist`, not pollute the electrical data. +- Pin coordinates, wire coordinates, and component positions are already partially available but discarded after connectivity resolution. +- The standalone parser uses geometry-based connectivity (bounding box intersection), while universal-netlist uses Union-Find with spatial grid indexing. The visual extraction should reuse the existing OLE/record parsing infrastructure. diff --git a/plans/cloud-storage-readiness.md b/plans/cloud-storage-readiness.md new file mode 100644 index 0000000..ba9cbc5 --- /dev/null +++ b/plans/cloud-storage-readiness.md @@ -0,0 +1,576 @@ +# Plan: Decouple file I/O from parsers (cloud-storage readiness) + +## Context + +Today every parser entry point eventually pulls bytes off the local disk via +`fs`/`fs/promises`. The user wants to deploy a Cloud Run / AWS variant of +`universal-netlist` that reads `.DSN`, `.dat` (Cadence), and `.SchDoc` / +`.PrjPcb` (Altium) files directly from cloud storage buckets (GCS/S3) without +forking the parsers. The actual binary/text parsing is already pure (operates +on `Buffer`/`string`), so the question is: are the I/O seams thin enough to +swap? + +**Answer: yes, with one focused refactor.** This plan introduces a `Storage` +interface, ships only the local implementation, and routes every existing +file-system call through it. Cloud later becomes a single new file +(`GcsStorage` / `S3Storage`) and one line in a URI router. No parser logic +needs to change again. + +Scope confirmed with the user: + +- **Abstraction + GCS adapter + end-to-end test against a real bucket.** + S3 adapter is deferred (user has GCP, not AWS). +- All three supported formats (`.DSN`, `.dat`, `.SchDoc`) must work in cloud. + `pstswp.exe` (used by `export_cadence_netlist`) stays local-only and is + out of scope, since the cloud path will consume pre-exported `.dat` or + the binary `.DSN`/`.SchDoc` directly. +- Cloud paths are addressed via URI scheme on existing path arguments + (`gs://bucket/key`, `s3://bucket/key`). MCP tool signatures do not change. +- The MCP binary still runs locally on the agent's machine. It just makes + outbound HTTPS to GCS when given a `gs://` URI. Cloud Run deployment is + a separate future concern (same code, different host). +- E2E test uses your real GCP project, gated by an env var so CI without + creds skips it. No service-account JSON files committed. + +## Current I/O surface (verified by exploration) + +Three categories of disk access exist today: + +1. **Whole-file reads** — load a complete file into a `Buffer`/`string`, + then parse purely. + - `src/parsers/ole-reader/ole-reader.ts:50-58` — `OleReader` ctor calls + `readFileSync(filePath)`. Used by both DSN and Altium pipelines. + - `src/parsers/ole-reader/ole-reader.ts:452-455` — `readOleStream(filePath)` + thin wrapper around `OleReader`. + - `src/parsers/cadence/dat/pstxnet-parser.ts:13`, + `pstxprt-parser.ts:32`, `pstchip-parser.ts:22` — each reads the file + then delegates to a pure `parse*Content(string)` function. + - `src/parsers/altium/discovery.ts:52` — reads `.PrjPcb` config text. + - `src/parsers/altium/discovery.ts:29-31` — `open()` + 8-byte read for + OLE magic check (cheap to convert to a full read since these files + are small and we always end up loading them anyway). + - `src/parsers/cadence/discovery.ts:280` — reads `pstxprt.dat` to + extract `ROOT_DRAWING`. + +2. **Directory listings (recursive)** — for design discovery. + - `src/parsers/cadence/discovery.ts:69` (`walkForCadenceFiles`). + - `src/parsers/altium/discovery.ts:108` (recursive walker). + +3. **Local-only side-effects** — out of scope for cloud. + - `src/service/tools/cadence-export.ts` — shells out to `pstswp.exe`, + creates output dirs, renames lock files. Stays Windows + local. Cloud + deployments must accept that this tool is a no-op in their environment. + +No parser performs random-access seeks against a file handle. `BinaryReader` +seeks are in-memory `Buffer` offsets, so cloud blobs (one fetch → one Buffer) +work without changes. + +## Design + +### 1. New `Storage` abstraction + +Create `src/storage/`: + +- **`types.ts`** — interface and shared types: + + ```ts + export interface DirEntry { + name: string; // basename + path: string; // full storage-native path/URI + isDirectory: boolean; + isFile: boolean; + } + + export interface ListOptions { + maxDepth?: number; // matches DiscoverDesignsOptions semantics + } + + export interface Storage { + readFile(path: string): Promise; + readTextFile(path: string, encoding?: BufferEncoding): Promise; + listDirectory(path: string, options?: ListOptions): Promise; + exists(path: string): Promise; + } + ``` + + `listDirectory` returns the full recursive tree (with depth control) + rather than one level. The current Cadence/Altium walkers already do + recursive traversal; centralizing recursion in `Storage` lets cloud + implementations use bucket prefix listing efficiently (one paginated + listObjects call vs. N recursive readdirs). + +- **`local.ts`** — `LocalStorage` implements the interface using + `fs/promises`. Behaviour must match today's discovery semantics exactly + (EACCES is swallowed, separator normalization preserved). + +- **`index.ts`** — URI router: + + ```ts + export const getStorage = (pathOrUri: string): Storage => { + if (pathOrUri.startsWith("gs://")) return gcsStorage; + if (pathOrUri.startsWith("s3://")) + throw new Error("S3 storage backend not registered"); + return localStorage; + }; + ``` + +- **`local.test.ts`** — verify recursive walk, EACCES handling, + read/exists semantics against a tmp dir of fixtures. + +### 2. Refactor the I/O seams + +Each refactor is mechanical and additive — keep existing public signatures +intact, add overloads/factories that take a `Storage`. + +**`src/parsers/ole-reader/ole-reader.ts`** + +- Change the constructor to take a `Buffer` directly (the existing pure + build steps already operate on `this.buffer`). Make it `private`. +- Add static factories: + + ```ts + static fromBuffer(buffer: Buffer): OleReader; + static async from(path: string, storage?: Storage): Promise; + ``` + + `from` calls `(storage ?? getStorage(path)).readFile(path)` then + `OleReader.fromBuffer(buf)`. +- Update `readOleStream` to async: + + ```ts + export const readOleStream = async ( + filePath: string, + streamName = "FileHeader", + storage?: Storage, + ): Promise; + ``` + + Drop `readFileSync` import. All call sites are already in `async` + contexts. + +**`src/parsers/cadence/dsn/dsn-parser.ts:19-20`** + +Replace `new OleReader(dsnPath)` with `await OleReader.from(dsnPath)`. + +**`src/parsers/altium/index.ts`** (line ~414, `parse(schdocPath)`) + +Replace `readOleStream(schdocPath)` callsite with the async version. +Already in an `async` function. + +**`src/parsers/cadence/dat/pstxnet-parser.ts`, `pstxprt-parser.ts`, +`pstchip-parser.ts`** + +Replace `import { readFile } from "fs/promises"` with +`import { getStorage } from "../../../storage/index.js"` and use +`getStorage(filePath).readTextFile(filePath, "utf-8")`. The +`parse*Content(string)` pure functions stay untouched. + +**`src/parsers/cadence/discovery.ts`** + +- Inject storage: change `walkForCadenceFiles` to call + `storage.listDirectory(rootDir, { maxDepth })` and consume the flat + recursive listing. Group `.dat` matches by directory in JS. The + matching/scoring logic (`matchDatSetsToDesigns`, etc.) is pure — it + stays unchanged. +- Replace `readFile(pstxprtPath, "utf-8")` in `extractRootDrawing` with + `storage.readTextFile(...)`. +- `discoverCadenceDesigns` and `findCadenceDatFiles` resolve the storage + via `getStorage(rootDir)` once, pass it through. + +**`src/parsers/altium/discovery.ts`** + +- Replace `readdir` recursion with `storage.listDirectory(...)` (single + source of truth for separator/EACCES handling). +- Replace `open()` + 8-byte magic read with `storage.readFile(path)` + followed by an in-memory magic comparison. These files are small, and + we already read them in full immediately afterwards — no perf + regression. +- Replace project config `readFile` with `storage.readTextFile`. + +**`src/service/load-netlist.ts`** + +`resolvePath` currently calls `path.resolve(...)`. Update so URI strings +are passed through unchanged (`gs://...`, `s3://...`); only normalize when +the string is a local path. The existing local-resolution behaviour must +stay identical. + +**Out of scope (do not touch)** + +- `src/service/tools/cadence-export.ts` — Windows shell-out, never cloud. +- `src/types.ts` — `EDAProjectFormatHandler.parse(designPath: string)` + signature is unchanged. Cloud URIs are valid string paths. +- All `parse*Content` pure functions, `BinaryReader`, `OleReader`'s + internal parsing methods, MCP tool wiring. + +### 3. Testability + +- `LocalStorage` has its own unit tests (`src/storage/local.test.ts`). +- Existing parser/discovery tests continue running unchanged because + `getStorage(localPath)` returns `LocalStorage`. No fixture path changes. +- Optional: a small `MemoryStorage` test helper (in + `src/storage/memory.ts`) backed by a `Map` makes future + cloud-adapter tests trivial. Add only if existing tests benefit; not + required for this PR. + +### 4. GCS adapter (in this PR) + +New file: `src/storage/gcs.ts`. New runtime dep: `@google-cloud/storage`. +Registered in `getStorage`: + +```ts +if (pathOrUri.startsWith("gs://")) return gcsStorage; +``` + +Implementation surface for `GcsStorage` (lazy-init the SDK client so users +who never use cloud don't pay the import cost): + +```ts +import type { Storage as GcsClient } from "@google-cloud/storage"; + +let cachedClient: GcsClient | null = null; +const getClient = async (): Promise => { + if (!cachedClient) { + const { Storage } = await import("@google-cloud/storage"); + cachedClient = new Storage(); // ADC: no args needed + } + return cachedClient; +}; + +const parseUri = (uri: string): { bucket: string; key: string } => { + const m = uri.match(/^gs:\/\/([^/]+)\/?(.*)$/); + if (!m) throw new Error(`Invalid GCS URI: ${uri}`); + return { bucket: m[1], key: m[2] }; +}; + +export const gcsStorage: Storage = { + async readFile(uri) { + const { bucket, key } = parseUri(uri); + const [buf] = await (await getClient()).bucket(bucket).file(key).download(); + return buf; + }, + async readTextFile(uri, encoding = "utf-8") { + return (await this.readFile(uri)).toString(encoding); + }, + async exists(uri) { + const { bucket, key } = parseUri(uri); + const [ok] = await (await getClient()).bucket(bucket).file(key).exists(); + return ok; + }, + async listDirectory(uri, options) { + const { bucket, key } = parseUri(uri); + const prefix = key === "" ? "" : key.endsWith("/") ? key : key + "/"; + const [files] = await (await getClient()).bucket(bucket).getFiles({ + prefix, + autoPaginate: true, + }); + return files + .map((f) => buildDirEntry(`gs://${bucket}/${f.name}`, prefix)) + .filter((e) => withinMaxDepth(e, prefix, options?.maxDepth)); + }, +}; +``` + +Key implementation notes: + +- **Authentication is implicit.** No code touches credentials. ADC chain: + `GOOGLE_APPLICATION_CREDENTIALS` env → `gcloud auth application-default + login` creds → metadata server. Same code works on a laptop (user creds) + and on Cloud Run (service-account creds). +- **No directories in object stores.** GCS lists flat keys; "depth" is + derived from `/` count in the suffix after the prefix. The Cadence/Altium + walkers use `listDirectory` for full recursive listings anyway, so this + shape is a natural fit. +- **No `isDirectory: true` entries from GCS.** The walkers must not rely on + directory entries — only file entries matter. Verify Cadence and Altium + discovery code paths after the LocalStorage refactor by checking that + they only consume `entry.isFile` items. +- **Path joining stays string-based** in discovery code. We never call + `path.join` on URIs — instead, the walkers receive absolute paths/URIs + back from `Storage.listDirectory` and use them as-is. The existing + `normalizeSeparators` logic only applies when `getStorage` returns + `LocalStorage`. + +S3 (deferred): when needed, `src/storage/s3.ts` follows the exact same +pattern with `@aws-sdk/client-s3` (`GetObjectCommand`, `ListObjectsV2Command`). +Cost ~80 LOC. No parser changes. + +## Critical files to modify + +New: + +- `src/storage/types.ts` +- `src/storage/local.ts` +- `src/storage/gcs.ts` +- `src/storage/index.ts` +- `src/storage/local.test.ts` +- `src/storage/gcs.e2e.test.ts` (gated by env var, see verification) + +Modified: + +- `src/parsers/ole-reader/ole-reader.ts` +- `src/parsers/cadence/dsn/dsn-parser.ts` +- `src/parsers/altium/index.ts` +- `src/parsers/cadence/dat/pstxnet-parser.ts` +- `src/parsers/cadence/dat/pstxprt-parser.ts` +- `src/parsers/cadence/dat/pstchip-parser.ts` +- `src/parsers/cadence/discovery.ts` +- `src/parsers/altium/discovery.ts` +- `src/service/load-netlist.ts` (only `resolvePath` URI passthrough) +- `src/paths.ts` (if `resolvePath` lives there — confirm during edit) + +## Verification + +End-to-end checks before considering done. + +### Local (no creds needed) + +1. `npm run type-check && npm run lint && npm test` — full suite green. + Existing parser/discovery tests pass unchanged because `LocalStorage` + is the default and matches today's behaviour. +2. `npm run dev` and exercise via MCP tools against existing fixtures: + - `mcp__universal-netlist__list_designs` against the test fixtures + directory must return identical results to `main`. + - `mcp__universal-netlist__query_xnet_by_net_name` on a DSN fixture and + a `.dat`-based fixture must produce byte-identical JSON to `main`. + - `mcp__universal-netlist__query_xnet_by_pin_name` on an Altium + `.SchDoc` fixture must produce byte-identical JSON to `main`. +3. Confirm no remaining `import ... from "fs"` / `"fs/promises"` exists + under `src/parsers/**` (excluding `cadence-export.ts` which legitimately + needs local fs for `pstswp.exe` invocation). Grep is sufficient. + +### GCS end-to-end (real bucket) + +This is the proof that the abstraction holds and IAM auth works. + +**One-time setup (user runs):** + +```bash +gcloud auth application-default login +gcloud config set project +gsutil mb gs://val-netlist-e2e/ +gsutil -m cp -r tests/fixtures/cadence-cis-sample \ + gs://val-netlist-e2e/cadence-cis-sample/ +gsutil -m cp -r tests/fixtures/altium-sample \ + gs://val-netlist-e2e/altium-sample/ +gsutil -m cp -r tests/fixtures/cadence-dat-sample \ + gs://val-netlist-e2e/cadence-dat-sample/ +export NETLIST_GCS_E2E_BUCKET=val-netlist-e2e +``` + +**E2E test file: `src/storage/gcs.e2e.test.ts`** + +Skips automatically if `NETLIST_GCS_E2E_BUCKET` is unset. For each of the +three formats (cadence-cis, cadence-dat, altium) it runs the same query +twice — once against the local fixture path, once against the +`gs://${bucket}/...` URI — and `expect(localResult).toEqual(gcsResult)`. + +```ts +const bucket = process.env.NETLIST_GCS_E2E_BUCKET; +const skip = !bucket ? describe.skip : describe; + +skip("gcs e2e parity", () => { + it("list_designs: local and gs:// match", async () => { + const local = await discoverDesigns("tests/fixtures/cadence-cis-sample"); + const cloud = await discoverDesigns(`gs://${bucket}/cadence-cis-sample`); + expect(stripPaths(cloud)).toEqual(stripPaths(local)); + }); + + it("query_xnet_by_net_name: DSN local vs gs:// match", async () => { /* ... */ }); + it("query_xnet_by_net_name: dat-only local vs gs:// match", async () => { /* ... */ }); + it("query_xnet_by_pin_name: Altium local vs gs:// match", async () => { /* ... */ }); +}); +``` + +`stripPaths` normalizes the `path` field across the two backends (one is a +filesystem path, the other a `gs://` URI) so deep-equal can succeed on +everything else. Every other field — names, formats, nets, pins, +components — must match exactly. + +**Manual smoke from an agent:** + +After the test passes, do one live MCP-tool call to confirm: + +```jsonc +{ "tool": "mcp__universal-netlist__query_xnet_by_net_name", + "arguments": { + "design": "gs://val-netlist-e2e/cadence-cis-sample/.dsn", + "net": "" } } +``` + +Output should match the local equivalent byte-for-byte. + +**IAM negative test (manual, ~2 min):** + +```bash +gcloud storage buckets remove-iam-policy-binding gs://val-netlist-e2e \ + --member="user:$(gcloud config get-value account)" \ + --role="roles/storage.objectViewer" +# Re-run the e2e test → must surface a clean "permission denied" error, +# not a crash. Restore the binding afterwards. +``` + +This proves the SDK's auth chain is what enforces access, and that errors +propagate cleanly through our abstraction. + +## Practical examples: same agent, three backends + +The point of the abstraction is that **the MCP tool call is identical +across backends — only the path string changes**. + +### Example 1: discovering designs + +**Local laptop** — exactly today's behaviour: + +```jsonc +{ + "tool": "mcp__universal-netlist__list_designs", + "arguments": { "path": "/Users/val/projects/board-rev-c" } +} +``` + +Internal dispatch: + +```ts +getStorage("/Users/val/projects/board-rev-c") // → LocalStorage +// LocalStorage.listDirectory recursively walks via fs/promises.readdir +// LocalStorage.readFile pulls bytes via fs/promises.readFile +``` + +**Cloud Run + GCS** — supply a `gs://` URI: + +```jsonc +{ + "tool": "mcp__universal-netlist__list_designs", + "arguments": { "path": "gs://acme-eda-uploads/board-rev-c" } +} +``` + +Internal dispatch: + +```ts +getStorage("gs://acme-eda-uploads/board-rev-c") // → GcsStorage +// bucket("acme-eda-uploads") +// .getFiles({ prefix: "board-rev-c/", autoPaginate: true }) +// → flatten to DirEntry[], filter by maxDepth, return. +// +// bucket("acme-eda-uploads").file("board-rev-c/top.dsn").download() +// → returns Buffer, identical shape to fs/promises.readFile +``` + +**Lambda / Fargate + S3** — same tool, `s3://` URI (future): + +```jsonc +{ + "tool": "mcp__universal-netlist__list_designs", + "arguments": { "path": "s3://acme-eda-uploads/board-rev-c" } +} +``` + +```ts +getStorage("s3://acme-eda-uploads/board-rev-c") // → S3Storage +// ListObjectsV2Command({ Bucket: "acme-eda-uploads", +// Prefix: "board-rev-c/" }) +// → paginate, build DirEntry[] from Contents[]. +// +// GetObjectCommand({ Bucket, Key: "board-rev-c/top.dsn" }) +// → stream to Buffer. +``` + +The agent sees identical JSON in all three cases: + +```jsonc +{ + "designs": [ + { "name": "top", + "format": "cadence-cis", + "path": "/top.dsn", + "datFiles": { "pstxnet": "/netlist/pstxnet.dat", "...": "..." } } + ] +} +``` + +### Example 2: querying a net (the full read pipeline) + +``` +agent → MCP tool: query_xnet_by_net_name({ design: "/top.dsn", + net: "PCIE_RX0_P" }) + │ + ▼ + loadNetlist(designPath) + │ + ▼ + cadenceHandler.parse(designPath) + │ + ▼ + OleReader.from(designPath, getStorage(designPath)) + │ + ┌─────────────────────┼─────────────────────┐ + ▼ ▼ ▼ + LocalStorage GcsStorage S3Storage + fs.readFile(path) bucket.file(key) GetObjectCommand + .download() + │ │ │ + └─────────────────────┴─────────────────────┘ + │ + ▼ + Buffer (whole file) + │ + ▼ + OleReader.fromBuffer(buf) ← pure, identical + │ + ▼ + parsePage / parseCache / ... ← pure, identical + │ + ▼ + ParsedNetlist → JSON to agent +``` + +Every box below "Buffer" is the **same code path** for all three backends. +That is the whole guarantee of this refactor. + +### Example 3: mixed local + cloud in the same session + +Because dispatch is per-call and based on the path string, an agent can +mix backends within a single conversation: + +```jsonc +// Compare a local in-progress design to the canonical version in GCS +{ "tool": "mcp__universal-netlist__query_xnet_by_net_name", + "arguments": { "design": "/Users/val/wip/top.dsn", "net": "DDR4_CK_P" } } + +{ "tool": "mcp__universal-netlist__query_xnet_by_net_name", + "arguments": { "design": "gs://acme-eda-archive/board-rev-b/top.dsn", + "net": "DDR4_CK_P" } } +``` + +No server restart, no config flag. Each call routes through `getStorage` +independently. + +### Authentication + +- **GCS**: `@google-cloud/storage` picks up Application Default Credentials + from the Cloud Run service account, or `gcloud auth application-default + login` creds locally, or `GOOGLE_APPLICATION_CREDENTIALS` for service + account JSON. `GcsStorage` constructor takes no creds — the SDK handles it. +- **S3** (future): `@aws-sdk/client-s3` picks up the IAM role on + Lambda/ECS/EC2, or `AWS_*` env vars locally. Same story. +- **LocalStorage**: no auth, just filesystem permissions. + +The `Storage` interface stays auth-agnostic. Each implementation owns its +own credential plumbing. + +## Out of scope / explicit non-goals + +- **S3 adapter** — deferred. User has GCP, not AWS. Skeleton documented + above (~80 LOC follow-up PR). +- **Cloud Run deployment artefacts** — Dockerfile, service.yaml, IaC. + This PR makes Cloud Run *possible*; actually deploying is a separate + task. +- Changing MCP tool argument shapes — `designPath` stays a string. +- Refactoring `cadence-export.ts` for cloud — it remains a local-Windows + tool. Cloud users export `.dat` locally and upload, or use `.DSN` / + `.SchDoc` directly. +- Streaming reads. Every parser already loads whole files; GCS + `download()` returns a Buffer just fine. Streaming can be added later + if a parser ever needs it. +- GCS e2e tests in CI — gated by env var, run on user's laptop only. diff --git a/plans/dsn-parser-audit.md b/plans/dsn-parser-audit.md new file mode 100644 index 0000000..569523a --- /dev/null +++ b/plans/dsn-parser-audit.md @@ -0,0 +1,193 @@ +# DSN Parser Audit: TypeScript vs C++ Reference + +**Date**: 2026-03-08 +**Scope**: All files under `src/parsers/cadence/dsn/` compared against `references/OpenOrCadParser/` + +## Module Mapping + +| TypeScript | C++ Reference | +|---|---| +| `cache-parser.ts` | `src/Streams/StreamCache.cpp` | +| `page-parser.ts` | `src/Streams/StreamPage.cpp` | +| `page-parser.ts` (parsePackageStream) | `src/Streams/StreamPackage.cpp` | +| `library-parser.ts` | `src/Streams/StreamLibrary.cpp` | +| `page-parser.ts` (parseHierarchyNetNames) | `src/Streams/StreamHierarchy.cpp` | +| `generic-parser.ts` | `src/GenericParser.cpp` | +| `structures.ts` | `src/Structures/Struct*.cpp` (12 covered) | + +--- + +## 1. What We Do That C++ Does Not + +### 1.1 Semantic Field Extraction + +| Feature | Location | Detail | +|---|---|---| +| **GraphicInst.pairingId** | `structures.ts` parseGraphicInstBase | First uint32 of body is strLst index for OPC net name. C++ skips this as "unknown data" (`printUnknownData(8)`). We correctly identified its semantic meaning, enabling OPC net resolution without cross-page wire matching. | +| **PlacedInstance.partValueIdx** | `structures.ts` parsePlacedInstance | Extracted from the 14-byte block that C++ skips entirely. Used for Value resolution (3-source priority: prefix "Value" property, partValueIdx, LibraryPart defaultValue). | +| **PlacedInstance prefix properties** | `structures.ts` parsePlacedInstance | Extract (nameIdx, valIdx) pairs from short prefix. C++ reads prefixes but doesn't expose property key/value pairs. Used for MPN and Value extraction via Library strLst lookup. | +| **T0x10.pinIndex computation** | `structures.ts` parseT0x10 | Immediately compute `sth < 32768 ? sth : 65536 - sth`. C++ stores raw `sth` field without transformation. | + +### 1.2 Error Recovery + +| Feature | Location | Detail | +|---|---|---| +| **Cache brute-force recovery** | `cache-parser.ts` scanForStructures | Scan for preamble magic `FF E4 5C 39` when sequential metadata parsing fails. Checks 3 bytes before magic for valid structure type (Package=0x1F, LibraryPart=0x18). C++ `discard_until_preamble()` re-reads linearly from a checkpoint. | +| **LibraryPart try-catch recovery** | `page-parser.ts` parsePackageStream | If `parseLibraryPart()` fails, seek back and `skipStructure()`. C++ has no equivalent; exceptions propagate up. | +| **GeneralProperties optional parsing** | `structures.ts` parseLibraryPart | Try-catch around 4 string reads + 2-byte skip. C++ uses conditional checkpoint position logic (less forgiving). | + +### 1.3 Pin Resolution Pipeline + +| Feature | Location | Detail | +|---|---|---| +| **Cache dual pin map indexing** | `cache-parser.ts` indexCachePackage | `pinMaps` (primary) + `cachePinMaps` (fallback when Packages/ pinMap has more entries than T0x10 count, i.e. physical pads > schematic pins). No C++ equivalent. | +| **Multi-section component keying** | `cache-parser.ts` indexCachePackage | Index cache packages with `baseName + unitRef` compound keys for multi-section components (e.g., dual op-amps). C++ doesn't track unitRef. | +| **Multi-fallback pin resolution** | `pin-resolver.ts` | Packages/ -> Cache -> brute-force -> positional device -> unitRef fallback chain. C++ reference shows no pin resolution strategy at all. | +| **Device.pinMap null entries** | `structures.ts` parseDevice | Store `null` for `-1` strLen pins, preserving positional alignment. C++ skips `-1` entries (vector shrinks, losing index correspondence). | + +### 1.4 Net Connectivity + +| Feature | Location | Detail | +|---|---|---| +| **Net table storage + uppercasing** | `page-parser.ts` parsePage | Store `Map` with uppercased names (matching Cadence Allegro DAT export convention). C++ reads the net table but only logs it. | +| **Hierarchy stream parsing** | `page-parser.ts` parseHierarchyNetNames | Dedicated parser with 0x43 magic marker scanning for cross-page net name aliases. C++ has `StreamHierarchy.cpp` but no equivalent net name extraction. | + +### 1.5 Encoding + +| Feature | Location | Detail | +|---|---|---| +| **Latin-1 string decoding** | `library-parser.ts` readStringLatin1 | Explicit `toString('latin1')` for Library strLst entries. C++ uses implicit std::string encoding. | + +--- + +## 2. What C++ Does That We Do Not + +### 2.1 Structural Validation (Systematic Gap) + +| Gap | C++ Behavior | Our Behavior | Risk | +|---|---|---|---| +| **`sanitizeCheckpoints()`** | Validates ALL FutureData checkpoints were visited; throws if any remain unparsed. Called in every structure parser. | Never called. `checkpoint()` is best-effort. | **Medium-High**. Binary format changes could silently shift structure boundaries without error. | +| **`assumeData()` validation** | Validates specific bytes match expected values (e.g., SymbolDisplayProp trailing 0x00). | `skip()` without checking. | **Low**. Would only catch rare format corruption. | +| **Twin ID validation in Cache** | Compares id0 vs id1 and logs ERROR on mismatch. | Uses equality test for branching only, no validation logging. | **Low**. | +| **EOF validation** | `sanitizeEoF()` called after every stream parse; throws if extra data exists. | Never checked. | **Low**. Extra trailing data is harmless for netlist extraction. | + +### 2.2 Skipped Fields + +These fields are extracted by C++ but skipped by us. All are rendering/graphical data, not needed for netlist connectivity. + +| Field | Structure | C++ Extracts | We Do | +|---|---|---|---| +| pinIgnore (bit 7) / pinGroup (bits 0-6) | Device | `GetBit(7, cfg)` / `cfg & 0x7f` | `skip(1)` | +| Coordinates, pinShape, portType | SymbolPin | 28 bytes of geometric data | `skip(28)` | +| color, lineWidth, lineStyle | Wire | `ToColor()`, `ToLineWidth()`, `ToLineStyle()` | `skip()` each | +| color | GraphicInst | `ToColor(readUint8())` | `skip(1)` | +| sourceLibrary | Package, LibraryPart | Stored as member | Read and discarded | +| unknownStr1 | Package | Stored, logged | Read and discarded | + +### 2.3 Entire Page Sections Skipped + +Our page parser stops after OffPageConnectors. C++ continues with: + +| Section | What C++ Does | Netlist Relevance | +|---|---|---| +| ERCObjects | Reads N structures | None (design rule checks) | +| BusEntries | Reads N structures | Low (bus segment entries) | +| GraphicInsts (post-OPC) | Reads N structures | None (schematic graphics) | +| Structure10 / Structure11 | Reads N unknown structures | Unknown | + +### 2.4 Package Stream + +| Gap | Detail | +|---|---| +| **PartCell structures** | C++ fully parses and stores PartCell structures in Package streams. We `skipStructure()` them entirely. PartCells contain cell-level metadata, not directly needed for pin mapping. | + +### 2.5 LibraryPart Primitives + +| Gap | Detail | +|---|---| +| **Primitive iteration** | C++ iterates through primitives with complex discard logic (up to 64-byte trailing data detection per primitive, boundary validation). We call `skipToNextBoundary()` to jump past the entire primitive block. Primitives are graphical shapes (lines, arcs, rectangles) inside symbol definitions. | +| **Pin 0x00 skip marker** | C++ peeks at first byte of each SymbolPin; if 0x00, skips that pin and doesn't add to symbolPins array. ~~We parse all pins unconditionally.~~ **Fixed** (2026-03-10): we now peek for 0x00 and push empty string to preserve index alignment. | + +### 2.6 Library Stream + +| Gap | Detail | Risk | +|---|---|---| +| **Version-dependent strLstLen** | C++ reads `uint16` for version A, `uint32` for B+. We always read `uint32`. | **Medium**. Version A DSN files would read garbage for strLstLen. Unknown how common version A files are in the wild. | +| **Format version prediction** | C++ tries 16 format versions (A-P) via `predictVersion()`. We have no version detection. | **Low** if version A files are rare. | +| **Database type detection** | C++ reads introduction string to distinguish Design vs Library files. We skip entirely. | **None** for netlist use case. | +| **Part aliases table** | C++ reads alias->package name pairs after strLst. We stop at strLst. | **Low**. Could matter if a design uses part aliases for package references. | +| **someLen validation** | C++ enforces `someLen == 24` and throws otherwise. We skip without validation. | **Low**. | +| **Timestamps** | C++ extracts createDate / modifyDate. We skip. | **None**. | +| **LOGFONTA parsing** | C++ parses font structures. We calculate byte offsets and skip. | **None**. | +| **Design footer** | C++ reads schematicName and validation bytes for Design-type files. We skip. | **None**. | + +### 2.7 Logging and Debugging + +C++ has comprehensive `spdlog` logging at debug/trace/error levels throughout all parsers. We have zero logging in the DSN parser. Format deviations, unexpected bytes, and structural anomalies are invisible. + +### 2.8 Uncovered C++ Streams (10) + +These stream parsers exist in the C++ reference but have no TypeScript equivalent. None are needed for netlist extraction: + +`StreamAdminData`, `StreamBOMDataStream`, `StreamDTypeD`, `StreamDirectoryStruct`, `StreamDsnStream`, `StreamERC`, `StreamHSObjects`, `StreamNetBundleMapData`, `StreamSchematic`, `StreamSymbol` + +### 2.9 Uncovered C++ Structures (~39) + +All graphic rendering structures (10 files: lines, arcs, boxes, polygons, beziers, bitmaps, ellipses, polylines, OLE embeds, comment text), symbol definitions (9 files), ERC objects (2 files), title blocks (2 files), hierarchy sub-records (4 files), and miscellaneous metadata structures. None are needed for connectivity. + +--- + +## 3. Key Implementation Differences + +### 3.1 Error Handling Philosophy + +| Aspect | C++ | TypeScript | +|---|---|---| +| Checkpoint enforcement | Strict: `sanitizeCheckpoints()` throws | Lenient: silent no-ops | +| Unknown flags | Exception on unmatched values (GraphicInst switch default) | Silent pass-through | +| EOF management | Clears EOF flag during prefix guessing | No EOF concept in BinaryReader | +| Structure type check | Three overloads: no type, single type, multiple types | Single function with optional type | + +### 3.2 Architecture + +| Aspect | C++ | TypeScript | +|---|---|---| +| Goal | Reverse-engineer and document the full DSN format | Extract netlist connectivity data | +| Data retention | All structures parsed and stored in object tree | Only netlist-critical data retained | +| Output | In-memory object tree with logging | Typed interfaces (ParsedNetlist) | +| Recovery | Linear re-read from checkpoints | Magic-based brute-force scanning | +| Extensibility | Factory pattern for new structure types | Direct function dispatch | + +### 3.3 Sub-Loop Exit in Cache + +- C++ breaks from sub-loop and continues parsing the rest of the Cache +- TS returns from entire function, abandoning Cache processing + +--- + +## 4. Actionable Risks + +Ranked by likelihood of causing real-world issues: + +### High Priority + +- [x] **SymbolPin 0x00 skip marker** (fixed 2026-03-10): Added 0x00 peek-and-skip in `parseLibraryPart()`, pushing empty string to maintain pinIndex alignment. No existing fixtures were affected (CutiePi/LAUNCHXL gaps confirmed as absent LibraryPart data, not parsing failures). + +### Medium Priority + +- [ ] **No `sanitizeCheckpoints()`**: Silent boundary misalignment if binary format changes. Consider adding at least a warning-level check (non-throwing) to detect when checkpoints are missed. +- [ ] **Library version A (uint16 strLstLen)**: We always read uint32. Determine if version A DSN files exist in the wild. If so, add version detection or at least a heuristic (check if uint32 value is unreasonably large). + +### Low Priority + +- [ ] **Port trailing bytes**: Page parser comment says 5 bytes; StructPort.cpp shows 9 bytes. Verify consistency across both call sites. +- [ ] **Part aliases table**: Could matter if designs use part aliases for package references. Monitor for unresolved package names. +- [ ] **Cache sub-loop exit scope**: TS returns from entire function vs C++ breaks from loop. Could miss structures after the metadata sub-loop in edge cases. +- [ ] **pinIgnore flags**: If a design marks pins as "ignored", we'd still include them in the netlist. Likely benign for connectivity checking. + +### No Action Needed + +- Graphic rendering fields (colors, line widths, coordinates) +- Uncovered streams and structures (all non-netlist) +- Logging infrastructure (nice-to-have, not a correctness issue) +- Timestamps, fonts, database type detection diff --git a/plans/power-net-stop-pattern.md b/plans/power-net-stop-pattern.md new file mode 100644 index 0000000..07021bd --- /dev/null +++ b/plans/power-net-stop-pattern.md @@ -0,0 +1,54 @@ +# Fix: Power net pattern to use substring matching + +## Context + +The xnet traversal correctly stops at power/ground nets during traversal (line 346 of circuit-traversal.ts). However, the `POWER_NET_PATTERN` regex uses anchored patterns (`^VDD\w*$`) that only match nets starting with VDD, VCC, VBUS, etc. Prefixed power nets like `CC1310_VDD`, `USB_VBUS`, `XDS_VCC` are not recognized as power nets, so the traversal walks right through them into signal nets, eventually visiting half the board. + +## Root cause + +`POWER_NET_PATTERN` and `STOP_NET_PATTERN` in `circuit-traversal.ts` use `^VDD\w*$` style patterns. A net like `CC1310_VDD` doesn't start with `VDD`, so it's not matched. + +## Fix + +Change the power keyword patterns (VCC, VDD, VIN, VOUT, VBAT, VBUS, VSYS) from anchored start-of-string (`^VDD\w*`) to substring/contains matching. If the net name contains `VDD` anywhere, it's a power net. + +Keep the other patterns (PP\w*, RAIL_\w+, PWR_\w+, voltage patterns like `+5V`) as-is since they work correctly with anchored matching. + +### File: `src/circuit-traversal.ts` (lines 10-15) + +Change `POWER_NET_PATTERN` from: +``` +/^(VCC\w*|VDD\w*|VIN\w*|VOUT\w*|VBAT\w*|VBUS\w*|VSYS\w*|PWR_\w+|RAIL_\w+|PP\w*|PN\w*|LD_PP\w*|LD_PN\w*|[+-]?\d+V\d*\w*|[+-].+)$/i +``` + +To something like: +``` +/VCC|VDD|VBAT|VBUS|VSYS|^(VIN\w*|VOUT\w*|PWR_\w+|RAIL_\w+|PP\w*|PN\w*|LD_PP\w*|LD_PN\w*|[+-]?\d+V\d*\w*|[+-].+)$/i +``` + +The keywords VCC, VDD, VBAT, VBUS, VSYS become unanchored substring matches. VIN/VOUT stay anchored to avoid false positives (e.g. a signal containing "VIN" as part of another word is less likely but worth being cautious). Same split for STOP_NET_PATTERN. + +Note: `GROUND_NET_PATTERN` stays as-is (exact match for GND, VSS, etc.). + +### File: `src/circuit-traversal.test.ts` + +Add tests for: +- `isPowerNet('CC1310_VDD')` returns true +- `isPowerNet('USB_VBUS')` returns true +- `isPowerNet('XDS_VCC')` returns true +- `isPowerNet('AVDD')` returns true +- `isStopNet('CC1310_VDD')` returns true +- Signal nets like `CC1310_TXD`, `USB_DP`, `DIO4_SCL` are NOT matched + +### File: `src/server.ts` (lines 271, 295) + +Update tool descriptions for `query_xnet_by_net_name` and `query_xnet_by_pin_name` to mention that traversal stops at power nets too, not just ground. + +## Verification + +1. `npm run type-check && npm run lint && npm test` +2. MCP testing on LAUNCHXL-CC1310: + - `query_xnet_by_net_name VDDS`: should now return a contained result (U1 power pins, decoupling caps, FL1 ferrite bead, and stop at CC1310_VDD) + - `query_xnet_by_net_name CC1310_VDD`: should show components on that rail (pull-ups, decoupling cap, ICs) but NOT traverse into JTAG/debug subsystem + - `query_xnet_by_net_name CC1310_TXD`: should still work normally (signal net) + - `query_xnet_by_net_name GND`: should still be rejected diff --git a/plans/relative_path.md b/plans/relative_path.md new file mode 100644 index 0000000..4fae04e --- /dev/null +++ b/plans/relative_path.md @@ -0,0 +1,114 @@ +Universal Netlist MCP Server: Auto-Discovery & ID-Based Access +Problem +All tools currently require absolute paths to design files: +javascriptlist_nets({ design: "/Users/valentino/Developer/westworld/reference-designs/Altium-STM32-PCB/STM32_PCB_Design.PrjPcb" }) +This is cumbersome and breaks when Claude runs in environments with different filesystem views. +Solution +Implement auto-discovery so list_designs builds a registry, then all other tools work with a design identifier (relative path from search root): +javascriptlist_designs({ path: "/Users/valentino/Developer" }) +// Returns: +// [ +// { "id": "project-a/STM32_PCB_Design", "path": "/Users/.../project-a/STM32_PCB_Design.PrjPcb" }, +// { "id": "project-b/rev1/board", "path": "/Users/.../project-b/rev1/board.PrjPcb" }, +// { "id": "project-b/rev2/board", "path": "/Users/.../project-b/rev2/board.PrjPcb" } +// ] + +list_nets({ design: "project-b/rev1/board" }) +// Unambiguous - uses relative path as identifier +Handling Duplicate Names +When multiple designs have the same filename (e.g., board.PrjPcb in different folders), use the relative path from the search root as the design identifier: +Search root: /Users/valentino/Developer +Found files: + /Users/valentino/Developer/project-b/rev1/board.PrjPcb → id: "project-b/rev1/board" + /Users/valentino/Developer/project-b/rev2/board.PrjPcb → id: "project-b/rev2/board" +This ensures every design has a unique identifier. +Implementation Tasks +Task 1: Add Design Registry +Create an in-memory registry that maps design IDs to their full paths: +javascript// registry.js +const designRegistry = new Map(); + +function registerDesign(id, fullPath, type) { + designRegistry.set(id, { path: fullPath, type }); +} + +function getDesignPath(id) { + if (!designRegistry.has(id)) { + throw new Error(`Design "${id}" not found. Run list_designs first.`); + } + return designRegistry.get(id).path; +} + +function clearRegistry() { + designRegistry.clear(); +} + +// Helper: compute relative path ID from search root and full path +function computeDesignId(searchRoot, fullPath) { + const relativePath = path.relative(searchRoot, fullPath); + // Remove file extension + return relativePath.replace(/\.(PrjPcb|kicad_pro|dsn|cpm)$/i, ''); +} +Task 2: Update list_designs to Populate Registry +When list_designs scans a directory, register each design with its relative path ID: +javascriptasync function listDesigns({ path: searchRoot, pattern }) { + clearRegistry(); // Fresh scan + + const designPaths = await scanForDesigns(searchRoot, pattern); + + const designs = designPaths.map(fullPath => { + const id = computeDesignId(searchRoot, fullPath); + const type = detectDesignType(fullPath); + registerDesign(id, fullPath, type); + return { id, path: fullPath, type }; + }); + + return designs; +} +Task 3: Update All Other Tools +Add path resolution at the start of each tool. Change from: +javascriptasync function listNets({ design }) { + const projectPath = design; // Currently expects full path + // ... +} +To: +javascriptasync function listNets({ design }) { + const projectPath = getDesignPath(design); // Resolves ID to full path + // ... +} +Apply this change to: + +list_components +list_nets +search_nets +search_components_by_refdes +search_components_by_mpn +search_components_by_description +query_xnet_by_net_name +query_xnet_by_pin_name +query_component +export_cadence_netlist + +Task 4: Update Tool Descriptions +Update the design parameter description in all tools: +"design": "Design ID from list_designs results (relative path without extension)" +Example Workflow +javascript// Step 1: Discover designs (user provides their path once) +list_designs({ path: "/Users/valentino/Developer" }) +// Response: +// [ +// { "id": "westworld/Altium-STM32-PCB/STM32_PCB_Design", "path": "/Users/.../STM32_PCB_Design.PrjPcb" }, +// { "id": "power-supplies/rev1/board", "path": "/Users/.../rev1/board.PrjPcb" }, +// { "id": "power-supplies/rev2/board", "path": "/Users/.../rev2/board.PrjPcb" } +// ] + +// Step 2: Use any tool with the design ID +list_nets({ design: "westworld/Altium-STM32-PCB/STM32_PCB_Design" }) +query_component({ design: "power-supplies/rev1/board", refdes: "U2" }) +search_nets({ design: "power-supplies/rev2/board", pattern: "VCC.*" }) +Acceptance Criteria + +list_designs({ path }) scans recursively and registers all found designs +Design IDs are relative paths from search root without file extension (e.g., "project/rev1/board") +All other tools only accept design IDs +Clear error if design ID not found: "Design 'X' not found. Run list_designs first." diff --git a/plans/run_erc.md b/plans/run_erc.md new file mode 100644 index 0000000..1706a71 --- /dev/null +++ b/plans/run_erc.md @@ -0,0 +1,91 @@ +Universal Netlist MCP Server: run_erc (v1) Plan + +Goal +Add a new `run_erc` MCP tool that runs deterministic ERC checks on the universal netlist JSON and returns concise, LLM-friendly results. + +Scope (v1) +- Net-level ERC only (no consistency/inverse checks between `nets` and `components`) +- Severity model: only `error` and `warning` (no `info`) +- Runtime output: high-level summary metadata + grouped findings +- Keep payload lean; avoid noisy/duplicated fields + +v1 Rules (Final) +- `net.single_pin` -> `error` +- `net.testpoint_only` -> `error` +- `net.unnamed` -> `warning` +- `net.testpoint_stub` -> `warning` + +Input Arguments (v1) +- `design` (required) +- `include_dns` (optional, default `false`) +- `include_rules` (optional) +- `exclude_rules` (optional) +- `max_findings` (optional, default `100`) +- `max_findings_per_rule` (optional, default `25`) + +Output Principles (v1) +- Keep the high-level metadata simple and counter-based: + - `design` + - `summary.total_findings` + - `summary.by_severity` (`error`, `warning`) + - `summary.by_rule` + - `summary.scanned` (`nets`, `components`, `pins`) + - `summary.limits` (`max_findings`, `max_findings_per_rule`) +- No rule catalog. +- No `info` output. +- No cursor/paging in v1 unless we later prove truncation is a real problem. + +Data Structures Workstream +- We still need to design most rule payload shapes. +- Only `net.single_pin` is finalized for now. +- Other rule payload schemas are intentionally TBD and will be finalized after first implementation/output review. + +Zod Schema (Finalized for `net.single_pin`) +```ts +import { z } from "zod"; + +export const NetSinglePinFindingSchema = z.object({ + id: z.string().regex(/^[a-f0-9]{10,12}$/i), // deterministic truncated hash + net: z.string().min(1), + endpoint: z.string().regex(/^[A-Za-z0-9_]+\.[A-Za-z0-9_]+$/), // REFDES.PIN +}); + +export const NetSinglePinGroupSchema = z.object({ + severity: z.literal("error"), + findings: z.array(NetSinglePinFindingSchema), +}); +``` + +Hash ID Strategy +- Deterministic hash-based IDs (not sequential IDs). +- Canonical input for this rule: `net.single_pin||`. +- Truncate hash to 10-12 hex chars (default to 12 unless changed later). + +Implementation Steps +1. Server Wiring + - Register `run_erc` in `src/server.ts` with v1 input schema. +2. Service Entrypoint + - Add `runErc(...)` in `src/service.ts`. +3. ERC Engine Skeleton + - Build shared scan context from parsed netlist. + - Add rule runner framework (filtering via include/exclude lists). +4. Implement v1 Rules + - Implement four net rules listed above. +5. Summary + Limits + - Compute summary counters and enforce max limits. +6. Documentation + - Add `docs/tools/run_erc.md`. + - Update `docs/README.md` tool list. + - Update `manifest.json` tools list. +7. Tests + - Unit tests for each rule trigger. + - Unit tests for deterministic hash IDs. + - Unit tests for summary counters. + - Unit tests for include/exclude filtering and limits. + +Acceptance Criteria +- `run_erc` works on existing universal netlist designs. +- Output contains only `error`/`warning` severities. +- Rule counts and severity counts are correct. +- `net.single_pin` output matches finalized schema above. +- Result stays concise under configured caps. diff --git a/plans/xnet-depth-limit.md b/plans/xnet-depth-limit.md new file mode 100644 index 0000000..6afe851 --- /dev/null +++ b/plans/xnet-depth-limit.md @@ -0,0 +1,109 @@ +# Xnet Depth Limit + +## Context + +The xnet traversal (`traverseCircuitFromNet`) uses BFS with no depth bound. It terminates only when it hits stop nets (power/ground), skip_types, DNS filtering, or exhaustion. On designs with broad power rails that don't match the stop-net regex (e.g., `CC1310_VDD`, `USB_VBUS`), the traversal can fan out excessively. + +Instead of expanding the stop-net regex (fragile, false-positive-prone), add a `max_depth` parameter that caps the number of series-passive hops. This gives the calling agent deterministic control over traversal scope. + +## Depth Semantics + +Depth = number of series-passive component hops from the starting net. + +- `max_depth=0`: No traversal. Return only components directly connected to the starting net. +- `max_depth=1`: Traverse through one series passive. Shows components on the starting net and the next net segment. +- `max_depth=5` (default): Up to 5 hops. + +Ground nets (GND, AGND, DGND, etc.) are still rejected at the service layer regardless of depth. Power/ground stop-net behavior during traversal still applies within the depth limit. + +## Response Metadata + +Three fields, no redundancy: + +- `max_depth: number` - the configured depth limit (input parameter, default 5) +- `max_depth_reached: number` - the deepest passive hop actually taken during traversal; always present, purely informational about circuit topology +- `frontier_nets?: Array<{ net: string; depth: number }>` - nets at the depth boundary that were not traversed further; only present when non-empty; this is the authoritative truncation signal + +Note: `max_depth_reached === max_depth` does NOT imply truncation. The traversal may naturally end at exactly the limit (e.g., hit an active component or stop net on the last hop). Only a non-empty `frontier_nets` means the depth limit actually cut a branch short. + +## Implementation + +### File 1: `src/circuit-traversal.ts` + +**`TraversalOptions`** (line 181-184): Add `maxDepth?: number` field. + +**`TraversalResult`** (line 175-179): Add `max_depth_reached: number` and `frontier_nets: Array<{ net: string; depth: number }>`. + +**`traverseCircuitFromNet`** (line 269-413): + +1. Read `maxDepth` from options, default to `5`. +2. Change BFS queue from `string[]` to `Array<{ net: string; depth: number }>`. Seed with `{ net: startNet, depth: 0 }`. +3. Track `maxDepthReached = 0`, updated as each net is dequeued. +4. When processing a passive and enqueueing the other-side net, compute `nextDepth = currentDepth + 1`. +5. If `nextDepth > maxDepth`, do NOT enqueue. Instead, record `{ net, depth: nextDepth }` in a `frontierNets` collector. +6. At `maxDepth=0`: the starting net is processed (all components on it are collected), but no passives are followed through. Passives on the starting net appear in results as leaf components (their other-side pins/nets are visible but not traversed). +7. Return `max_depth_reached` and `frontier_nets` in the result. + +### File 2: `src/types.ts` + +**`AggregatedCircuitResult`** (line 144-153): Add: +- `max_depth: number` +- `max_depth_reached: number` +- `frontier_nets?: Array<{ net: string; depth: number }>` (omit when empty) + +### File 3: `src/service.ts` + +**`queryXnetByNetName`** (line 745-793): Add `maxDepth: number` parameter. Pass to `traverseCircuitFromNet` via options. Forward `max_depth_reached`, `frontier_nets`, `max_depth` into the `AggregatedCircuitResult`. + +**`queryXnetByPinName`** (line 803-889): Same changes. + +### File 4: `src/server.ts` + +**Both tool schemas** (lines 265-290 and 292-314): Add input parameter: +``` +max_depth: z.number().int().min(0).optional() + .describe("Max series-passive hops to traverse (0 = direct connections only, default 5)") +``` + +Pass `max_depth` through to the service function calls. + +Update tool descriptions to mention depth limiting and the response metadata (`max_depth_reached`, `frontier_nets`). + +### File 5: `src/circuit-traversal.test.ts` + +Add tests within the `traverseCircuitFromNet` describe block: + +**depth limit behavior:** +- `max_depth=0` returns only components on the starting net, no passive follow-through +- `max_depth=1` follows through one passive, stops at the next +- `max_depth=2` follows through two passives in a chain +- Default (no maxDepth) uses 5 + +**max_depth_reached:** +- Reports actual depth when traversal completes within the limit +- Reports max_depth when traversal is truncated +- Reports 0 when `max_depth=0` + +**frontier_nets:** +- Populated with correct net names and depths when depth limit is hit +- Empty/absent when traversal completes naturally +- Multiple frontier nets when traversal fans out at the boundary +- Not populated when traversal stops due to stop nets at the boundary (stop net is not a frontier) + +**interaction with other options:** +- Depth limit + skip_types: both apply independently +- Depth limit + stop nets: stop nets still stop within the depth limit +- Depth limit + DNS filtering: both apply + +## Verification + +```bash +npm run type-check && npm run lint && npm test +``` + +Manual MCP testing on a real design: +- `query_xnet_by_net_name` with `max_depth=0`: returns only direct connections +- `query_xnet_by_net_name` with `max_depth=1`: returns one hop +- `query_xnet_by_net_name` with default depth: behaves like current behavior but capped at 5 +- Verify `frontier_nets` appears only when traversal is truncated +- Verify `max_depth_reached` reflects actual circuit depth in all cases