diff --git a/docs/SPEC.md b/docs/SPEC.md index af87586a..0389e2dc 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -1446,6 +1446,25 @@ resource-inventory prescription. Capability map + build plan: goal 0011; evidence gaps (lifecycle/versioning, fuzzy semantics, first-match schema behavior) recorded there, not guessed at. +**Update — goal 0011 delivered the core of this review's model, +`LOCKED`/built; a few items stay named-and-deferred, not guessed +at.** Typed column schema (reusing ADR-0029's canonical +`typedfield.Field`, never a fifth vocabulary), system-managed audit +columns (`Row.CreatedAt`/`UpdatedAt`/`Status`, platform-owned, +excluding `CreatedBy`/`UpdatedBy` — Mill is single-user forever, +§3.7), a schema-generated row editor, `list-search` as the workflow +step (multiple match parameters, exact/fuzzy via an adopted matching +library — `github.com/hbollon/go-edlib`, never invented — a typed +Object output, Expired excluded from matching by default with a +per-step opt-in), and in-place migration of pre-existing key/value +Lists (`list.MigrateLegacyEntries`) are all built — see §3.3's List +row for the full writeup. Still open, deliberately deferred: CSV/ +JSON/JSON-Schema row+schema import, a first-match-only toggle's exact +schema behavior, a per-column Jaro-Winkler override, and full +per-execution dataset-version snapshotting (today's `list_id` on the +output Object is the goal's own named minimum evidence bar, not the +full snapshot this review calls for). + ### 3.2.3 Home/landing-dashboard reference review — design input, `OPEN` **Fifth owner-supplied reference review (2026-08-10, five screenshots, @@ -1632,7 +1651,7 @@ Plan step for this as a standing rule. | **Parallel Steps** | Fan out to multiple steps concurrently, then join | Graph/fan-in semantics: build. Concurrency execution: DBOS's `Queue`/`WithWorkerConcurrency` (§7) is a plausible real backing mechanism once designed, not hand-rolled goroutine management | ADR-0005 names it, deferred | | **Child Workflow** | One workflow invokes another as a step | Graph/node semantics: build. Execution: **adopt** — DBOS (already adopted, §7) has real, native parent/child primitives (`RunWorkflow` called from inside a running workflow auto-tracks `ParentWorkflowID`; a workflow ID is DBOS's own idempotency key), corrected from ADR-0005's original "no library has an opinion" verdict | `LOCKED` — [ADR-0010](adr/0010-child-workflow.md), built | | **Integration / Connector node** | Call an external HTTP API, auth'd | Wire protocol: adopt (stdlib `net/http`, via `internal/adapters/httpconnector`). Connector config/credential model: build (`internal/domain/connector`) + adopt (`zalando/go-keyring` via `internal/adapters/credential`) | `LOCKED` (execution) — `internal/domain/connector`'s `Connector{ID, Label, Type, BaseURL, AuthType, Headers}` + a new `integration-http` `NodeType` (`KindProcess`) execute real HTTP calls, resolving `AuthType`/secret into the right header (`X-Api-Key` or `Authorization: Bearer`) via `composition.SetConnectorLookup`'s injected seam (mirrors `TriggerService`'s `Syncer` pattern — the domain package doesn't own connector storage). §4 stays `OPEN` on the Configure-surface UI to author a Connector; see §3.5's own row | -| **List** (a reusable lookup/reference dataset) | Look up an Attributes value against a named, Configure-authored table, write the match back into Attributes | Build (core domain — no library has an opinion on Mill's own List model; the lookup itself is a plain map read) | `LOCKED` (execution) — `internal/domain/list.List{ID, Label, Entries}` + a new `list-lookup` `NodeType` (`KindProcess`) resolve a `listId` via `composition.SetListLookup` (same injected-seam pattern as Integration/Connector's `SetConnectorLookup`) and write the matched entry into `ExecContext.Attributes[outputKey]`. Not in ADR-0005's original taxonomy at all (a real gap flagged in §3.5) — added here as the first thing built against it. §3.5 stays `OPEN` on the Configure-surface UI to author a List | +| **List** (a reusable typed tabular dataset) | Look up an Attributes value against a named, Configure-authored table, write the match back into Attributes — either a single exact key (`list-lookup`) or multiple exact/fuzzy match parameters against typed columns (`list-search`) | Build (core domain — no library has an opinion on Mill's own List model; matching itself is a plain map read or, for fuzzy, an adopted library) | `LOCKED` (execution), grown from a flat key/value map to typed columns + rows by goal 0011: `internal/domain/list.List{ID, Label, Description, Columns []typedfield.Field, Rows []Row}` — `Row{ID, Values, CreatedAt, UpdatedAt, Status}` (`Status` is `Active`/`Expired`, a platform-owned audit field, never a user-declared Column; no `CreatedBy`/`UpdatedBy` — Mill is single-user forever, §3.7). `list-lookup` (`KindProcess`) keeps working completely unchanged against a typed List via `list.DeriveEntries` (a flat key/value view over the first two Columns). `list-search` (`KindProcess`) is the richer successor: multiple match parameters (JSON-encoded in one `matchParams` ConfigField, the `inputBindings`/`argumentsJSON` precedent), each a column + a literal-or-`attr:` value + exact/fuzzy match type, AND'd together; fuzzy matching adopts `github.com/hbollon/go-edlib` (MIT) behind `internal/adapters/fuzzymatch`, Damerau-Levenshtein by default (industry research: the most explainable algorithm, and Elasticsearch's/OpenRefine's own default); exact match is always plain string equality, never routed through the fuzzy library. Expired rows are excluded from matching by default, uniform across exact and fuzzy (industry research: the soft-delete/OFAC-sanctions-screening/Informatica-MDM convention), with a per-step `includeExpired` opt-in. Output is a typed Object Attribute (`{results, matched, first_match, match_count, list_id}`) — `list_id` is the goal's own minimum execution-evidence bar (full per-run dataset-version snapshotting stays deferred). Both nodes resolve a `listId` via `composition.SetListLookup` (unchanged seam, now returning `Entries`+`Columns`+`Rows`). Configure's Lists tab (`ConfigureLists.tsx`) authors the typed schema (a flat column editor mirroring `ConfigureAttributes.tsx`) and rows (a schema-generated row editor, type-aware inputs); pre-existing key/value Lists migrate in place on first load (`list.MigrateLegacyEntries`, synthesized `key`/`value` Columns) — was previously `OPEN` on the Configure-surface UI, now closed. CSV/JSON row import, per-column Jaro-Winkler override, and full per-run dataset snapshot/versioning are named, deliberately deferred future work. | | **MCP tool call** (§3.6's extension point — call a tool on a Configure-authored MCP server) | Call one tool on a locally-configured MCP server over stdio, replace the payload with its text result | Wire protocol: adopt (`modelcontextprotocol/go-sdk`'s client role, via `internal/adapters/mcpclient`). Server config/CRUD: build, same shape as Connector | `LOCKED` (execution + authoring, end-to-end) — `internal/domain/mcpserver.MCPServer{ID, Label, Command, Args}` + a new `mcp-tool-call` `NodeType` (`KindProcess`) resolve an `mcpServerId` via `composition.SetMCPServerLookup` and call `toolName` with `argumentsJSON`. Verified against a real spawned subprocess (an official MCP reference server via `npx`), not just unit tests — see §3.6 for the full writeup. This is the "add a new capability without a core code change" answer §3.6 set out to find | | **AI completion (local Ollama / BYO endpoint)** | Send a configured prompt + the running payload to a user-configured LLM endpoint, write the completion back into the payload — one deterministic call per step, never a loop (§1.1's owner-confirmed invariant, 2026-08-11) | Transport: adopt/reuse (Ollama and OpenAI-compatible endpoints are plain HTTP — candidate is the existing `httpconnector` path or a small dedicated adapter; research pass owed before building). Node/config model: build (the stamped Configure-entity recipe, same shape as MCP Server) | `OPEN` — invariant locked, nothing built; the named next capability after capture (ADR-0030). Local-Ollama variant is zero-egress and works at the bank | | **Durable step execution / retry / resume** | Survive the process dying mid-workflow, checkpoint per step, retry transient failures | Adopt (DBOS-Go) | `LOCKED` — ADR-0004 `accepted`, `internal/adapters/execution` + `executionservice.go` built and e2e-verified; a real regression test (`TestResumeAfterFailure_DoesNotReExecuteCheckpointedStep`) proves a checkpointed step doesn't re-execute on resume against a real DBOS SQLite runtime. Since [ADR-0008](adr/0008-single-execution-path.md), this is the *only* execution path — every run is durable, not an opt-in alternative to a plain in-memory Run | @@ -1972,7 +1991,7 @@ true and isn't what was asked for. | **Input / Attributes** | **Configure** | **1:1** — scoped to the one workflow that declares it, per §3.2's original cardinality note | `LOCKED` end-to-end — `ConfigureView.tsx`'s Attributes tab (`ConfigureAttributes.tsx`) picks a workflow and edits its declared schema (key/label/type rows, `FieldOptions` excluded — see §3.3's rule-builder Update note for why), calling `ConfigureService.UpdateWorkflowAttributes` | | **Branch** (routing — UI-renamed from "Decision: route" by [ADR-0027](adr/0027-decision-terminal-outcome.md); code IDs `KindDecision`/`decision-route` unchanged) | Canvas — conditions live on edges, authored via the rule builder | **1:1** — a workflow's routing logic is that workflow's own; §3.2's "cardinality unconfirmed" flag is now resolved by the split below, not by promoting routing to Configure | `LOCKED` end-to-end — see §3.3's rule-builder writeup (`react-querybuilder` + `ruleTranslate.ts`, one-way translation only) | | **Decision** (a reusable, typed **terminal outcome** — a genuinely new concept, not the routing node matured; the reference platform's own semantics: "rulesets route; Decisions terminate") | **Configure**, a Decisions tab — category (approve/deny/manual-review/action-needed/uncategorized, **immutable** after create, server-enforced with Duplicate as the migration path), typed output schema, optional webhook-by-HTTPRequest-reference | **1:many** — one configured Decision referenced by many workflows' terminal nodes via the ADR-0009 picker (`RefKind: "decision"`, quick-create included) | `LOCKED`, built end-to-end — [ADR-0027](adr/0027-decision-terminal-outcome.md) `accepted` 2026-08-10, three owner calls decided directly (Branch rename; webhook reuses the HTTPRequest capability by reference, never a second outbound-HTTP surface; manual-review parks into the existing Review queue). See §3.3's row for the build details | -| **List** (a reusable lookup/reference dataset) | **Configure** | **1:many** recommended, same shape as Integration — a shared lookup table is the kind of thing multiple workflows would plausibly reference | `LOCKED` end-to-end — `ConfigureView.tsx`'s Lists tab (`ConfigureLists.tsx`) is a real page: create/edit/delete a List and its key/value entries, calling `ConfigureService`'s `Lists`/`CreateList`/`UpdateList`/`DeleteList` | +| **List** (a reusable, typed tabular dataset) | **Configure** | **1:many** — a shared dataset multiple workflows plausibly reference | `LOCKED` end-to-end, grown from key/value to typed by goal 0011 — `ConfigureView.tsx`'s Lists tab (`ConfigureLists.tsx`) authors a Column schema (a flat key/label/type editor, `ConfigureAttributes.tsx`'s own style) and Rows (a schema-generated, type-aware row editor), calling `ConfigureService`'s `Lists`/`CreateList`/`UpdateList`/`DeleteList`/`AddListRow`/`UpdateListRow`/`DeleteListRow`. See §3.3's List row for the full execution-side writeup | **What Configure is *not*: a plugin system for user-defined node kinds.** Worth being explicit about, since "define a dedicated thing in Configure" diff --git a/docs/goals/BACKLOG.md b/docs/goals/BACKLOG.md index 12101104..bac965fa 100644 --- a/docs/goals/BACKLOG.md +++ b/docs/goals/BACKLOG.md @@ -44,6 +44,10 @@ this pipeline and on this code)** backend-side `isAway`), alert-style authorization request (notify.Start), cross-device forward (`composition.SendJSONWebhook`, `ForwardPendingApproval`) — see ADR-0032's Update note +4. [ ] [0026 — Request lifecycle honesty](0026-request-lifecycle-honesty.md) + — withdrawal verb (`cancel_write`, the MCP Tasks `tasks/cancel` + precedent ADR-0032 already mirrors) + staleness/expiry presentation + (owner-observed 2026-08-11: a 4h-old stale ask reads as breakage) **Ratified 2026-08-10 (owner): three groups, A→B→C. 0001 stays standing live-review material, interleaved during owner reviews, not a lane.** @@ -63,7 +67,7 @@ live-review material, interleaved during owner reviews, not a lane.** **Unscheduled (reorder into a group when prioritized)** 7. [x] [0012 — Authoring hot-exit](archive/0012-authoring-hot-exit.md) — canvas half delivered 2026-08-10 (scratch persistence + restored-unsaved banner + dirty dots; Configure forms recorded-remaining in the archived file) 8. [x] [0013 — Canonical type system](archive/0013-canonical-type-system.md) — COMPLETE 2026-08-10 (typedfield leaf pkg; all 4 vocabularies converged incl. openapispec Phase 3; the #1 kernel investment) -9. [ ] [0011 — Lists maturation](0011-lists-maturation.md) (typed datasets + List Search per SPEC §3.2.2's reference review; evidence-gap research first) +9. [x] [0011 — Lists maturation](archive/0011-lists-maturation.md) — DELIVERED 2026-08-12 (harvested from a parallel owner session + reconciled onto main: typed Columns/Rows against ADR-0029's canonical typedfield, system-managed audit columns w/ Expired-excluded-by-default, `list-search` node w/ go-edlib fuzzy matching, in-place legacy-List migration; CSV import + full per-run dataset snapshot named-deferred) 10. [x] [0014 — Home dashboard / value mirror](archive/0014-home-dashboard.md) — delivered 2026-08-10 (Recharts, industry-decided metric semantics, editable minutes-saved, default landing) 11. [ ] [0015 — Summon quick-invoke](0015-summon-quick-invoke.md) — CORE delivered 2026-08-11 (⌘K palette: commands with inline shortcuts, workflow run, tab jump/close; delegated build); PHASE 2 delivered same day (ADR-0033: the summon hotkey opens a dedicated floating Quick Panel — frameless, floats over fullscreen, Esc/blur dismiss, focus-yield; supersedes "summon opens the main window"). Remainder open: frecency/pins (needs the 0014 usage substrate), Configure entities, pending-review count, ⌘?/⌘/ alias (needs multi-binding registry support) 12. [x] [0022 — Workflow view mode](archive/0022-workflow-view-mode.md) — delivered 2026-08-11 (row click → read-only canvas w/ Run+step-debug; Edit explicit in-place mode switch; breakpoint dot moved onto the node card, both modes; fixed a latent bug where a policy deny could hide a breakpoint's existence) diff --git a/docs/goals/0011-lists-maturation.md b/docs/goals/archive/0011-lists-maturation.md similarity index 62% rename from docs/goals/0011-lists-maturation.md rename to docs/goals/archive/0011-lists-maturation.md index 6ebccae7..9dc04215 100644 --- a/docs/goals/0011-lists-maturation.md +++ b/docs/goals/archive/0011-lists-maturation.md @@ -83,6 +83,61 @@ workflow) proven per the layered-coverage model; the List-as-database boundary documented; evidence gaps resolved by research or explicitly deferred with reasons. +## Delivered (2026-08-12) + +Harvested from a parallel owner session's in-progress worktree +(`wt-lists`) and landed on `goal/0011-lists`, reconciled against main +(typedfield's Phase 1/2 convergence, entity-level `CreatedAt`/ +`UpdatedAt`, ADR-0028 validation, confirmed-delete/`InventoryList` +conventions, and SPEC §3.2.4 all landed on main after the worktree +branched). + +- Item 1 (typed columns + rows): `internal/domain/list.List.Columns + []typedfield.Field` / `Rows []Row`, built directly against + ADR-0029's canonical vocabulary from day one — no parallel schema + system. +- Item 2 (system-managed columns): `Row{ID, Values, CreatedAt, + UpdatedAt, Status}` — `Status` (`Active`/`Expired`) is a + platform-owned struct field, never a user-declared Column; + `CreatedBy`/`UpdatedBy` deliberately NOT modeled (Mill is + single-user forever, §3.7) — the goal's own open call, resolved. + Expired rows excluded from matching by default, uniform across + exact and fuzzy, with a per-step `includeExpired` opt-in (the + industry-research verdict this file recorded). +- Item 3 (schema/row import): NOT built — CSV/JSON row+schema import + stays named, deliberately deferred future work (recorded in + SPEC.md §3.2.2's Update note). +- Item 4 (`list-search`): built as a new `NodeType` alongside + `list-lookup` (kept, unchanged, via `list.DeriveEntries`'s + first-two-columns view) — multiple match parameters (column + + literal-or-`attr:` value + exact/fuzzy, AND'd), fuzzy via + `github.com/hbollon/go-edlib` (MIT) behind + `internal/adapters/fuzzymatch`, Damerau-Levenshtein default; exact + match stays plain equality, never routed through the fuzzy lib. + Output is the fixed-by-construction typed Object `{results, + matched, first_match, match_count, list_id}`. +- Item 5 (execution evidence): minimum bar only — `list_id` recorded + inline on every `list-search` output. Full per-run dataset-version + snapshotting stays deferred, named in SPEC.md, not silently dropped. +- Item 6 (migration): `list.MigrateLegacyEntries` converts a + pre-0011 flat key/value List into synthesized `key`/`value` typed + Columns + Rows in place, on first load, idempotently. + +Proof: `internal/domain/list`, `internal/domain/typedfield`, +`internal/domain/composition`, `internal/adapters/fuzzymatch`, +`internal/services/configuresvc`, `internal/services/executionsvc` +Go suites green (race + cover); two seeded workflows +(`example-list-lookup-workflow`, `example-list-search-workflow`) +against the shared seeded "Example: Country codes" List (typed +code/name columns, one deliberately Expired row), each proven via a +real-DBOS Go test AND a `seed-completeness.spec.ts` e2e case, +registered in `seedproof_test.go`; a dedicated +`configure-lists.spec.ts` e2e exercises the Configure Column/Row +editors and the `list-search` node's Inspector +(`ListSearchParamsEditor.tsx`) live through the canvas; `configure- +export-import.spec.ts` round-trips a List's typed columns/rows. +SPEC.md §3.2.2 (Update note), §3.3 (List row), and §3.5 (Configure +table) all updated in the same change. ## Design section (research pass delivered 2026-08-12 — full report in session record) @@ -111,9 +166,14 @@ Key verdicts, primary-sourced: consistency); SQLite-via-DBOS is the named future trigger the day a real four-digit-row List exists — not before. -**GATING DECISION, owner-owned:** `.claude/worktrees/wt-lists` holds a -near-complete uncommitted implementation of this goal from a parallel -session, on a branch diverged behind main (pre-§3.2.4, pre-goal-0018). -Rebase-and-land vs treat-as-scratch must be decided by the owner before -any build starts — another session's live workspace is never touched -from this one. +**GATING DECISION, owner-owned — resolved.** `.claude/worktrees/wt-lists` +held a near-complete uncommitted implementation of this goal from a +parallel session, on a branch diverged behind main (pre-§3.2.4, +pre-goal-0018). Rebase-and-land was the path taken (this file's own +"Delivered" section above): the worktree's uncommitted work was +harvested by diff (never checked out or mutated directly) onto a fresh +`goal/0011-lists` branch off current `main`, then reconciled against +everything that landed on `main` after the worktree branched — every +verdict in this Design section (fuzzy library, Row shape, output +stability, storage, the snapshot reframing) was independently +cross-checked against the delivered build and matched. diff --git a/frontend/bindings/github.com/alicoding/mill/internal/domain/list/index.ts b/frontend/bindings/github.com/alicoding/mill/internal/domain/list/index.ts index ceb966d9..0304877c 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/domain/list/index.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/domain/list/index.ts @@ -1,6 +1,11 @@ // Cynhyrchwyd y ffeil hon yn awtomatig. PEIDIWCH Â MODIWL // This file is automatically generated. DO NOT EDIT +export { + RowStatus +} from "./models.js"; + export type { - List + List, + Row } from "./models.js"; diff --git a/frontend/bindings/github.com/alicoding/mill/internal/domain/list/models.ts b/frontend/bindings/github.com/alicoding/mill/internal/domain/list/models.ts index 1badf45f..13c3bebe 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/domain/list/models.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/domain/list/models.ts @@ -1,23 +1,42 @@ // Cynhyrchwyd y ffeil hon yn awtomatig. PEIDIWCH Â MODIWL // This file is automatically generated. DO NOT EDIT +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore: Unused imports +import * as typedfield$0 from "../typedfield/models.js"; + /** - * List is one reusable, named lookup table. Entries maps an input key - * (whatever a workflow's list-lookup node is configured to look up) to - * the value that gets written back into the workflow's Attributes. + * List is one reusable, named typed dataset. Columns declares its + * schema (typedfield.Field, ADR-0029); Rows carries its data. + * + * Entries is the PRE-0011 flat key/value shape, kept on the struct + * for wire/backward compatibility only -- MigrateLegacyEntries + * (migrate.go) converts any list still carrying it (Columns empty, + * Entries non-empty) into the typed Columns+Rows shape the first time + * it's loaded (internal/services/configuresvc's restore()), so + * list-search/list-lookup execution logic only ever deals with + * Columns+Rows, never a third code path for the legacy shape. A list + * persisted before this goal, loaded once, re-persists in the typed + * shape; Entries is never populated by any code path after that first + * load -- new lists never populate it at all. DeriveEntries below is + * the read-side mirror: list-lookup's own execution keeps reading a + * flat map, computed from any 2+-column typed list's first two + * columns, so it never needed to change at all. */ export interface List { "ID": string; "Label": string; + "Description": string; + "Columns": typedfield$0.Field[] | null; + "Rows": Row[] | null; "Entries": { [_ in string]?: string } | null; /** - * BuiltIn marks a seeded example list (BuiltIn() below) -- purely - * informational, same as httprequest.HTTPRequest.BuiltIn/ - * decision.Decision.BuiltIn: drives a "built-in" badge only, never - * gates Edit/Delete. A seeded example is an ordinary, fully- - * editable/deletable list from the moment it exists (docs/SPEC.md - * §2.2's Update note). + * BuiltIn marks a seeded example list -- purely informational, + * same as httprequest.HTTPRequest.BuiltIn/decision.Decision.BuiltIn: + * drives a "built-in" badge only, never gates Edit/Delete. A seeded + * example is an ordinary, fully-editable list from the moment it + * exists (docs/SPEC.md §2.2's Update note). */ "BuiltIn": boolean; @@ -30,3 +49,36 @@ export interface List { "CreatedAt": string; "UpdatedAt": string; } + +/** + * Row is one typed record in a List. Values maps a declared Column's + * Key to its string value -- the same "every value stays a plain + * string on the wire" discipline typedfield.Field itself documents; + * a Column's Type only governs validation/rendering/matching, never + * the wire shape. CreatedAt/UpdatedAt/Status are platform-owned audit + * fields, set by the owning service (internal/services/configuresvc), + * never a user-declared Column. + */ +export interface Row { + "ID": string; + "Values": { [_ in string]?: string } | null; + "CreatedAt": string; + "UpdatedAt": string; + "Status": RowStatus; +} + +/** + * RowStatus is a Row's platform-owned lifecycle state -- never a + * TypedField/user-declared column (goal 0011's audit-column + * decision: system-managed fields are Go struct fields, not entries + * in List.Columns). + */ +export enum RowStatus { + /** + * The Go zero value for the underlying type of the enum. + */ + $zero = "", + + RowActive = "active", + RowExpired = "expired", +}; diff --git a/frontend/bindings/github.com/alicoding/mill/internal/services/configuresvc/configureservice.ts b/frontend/bindings/github.com/alicoding/mill/internal/services/configuresvc/configureservice.ts index fc7a1897..18480f9b 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/services/configuresvc/configureservice.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/services/configuresvc/configureservice.ts @@ -44,11 +44,24 @@ import * as list$0 from "../../domain/list/models.js"; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore: Unused imports import * as mcpserver$0 from "../../domain/mcpserver/models.js"; +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore: Unused imports +import * as typedfield$0 from "../../domain/typedfield/models.js"; // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore: Unused imports import * as $models from "./models.js"; +/** + * AddListRow appends a new, Active row to a List, minting its ID here + * (row-ID generation stays a service-layer concern, same as List IDs + * themselves via seeding.NewSlugID -- internal/domain/list stays pure + * per .claude/rules/backend.md). + */ +export function AddListRow(listID: string, values: { [_ in string]?: string } | null): $CancellablePromise { + return $Call.ByID(197919857, listID, values); +} + /** * CaptureShellPath returns the user's real login-shell $PATH -- the * ExecEnv form's "Capture from my shell" affordance (ADR-0026's @@ -81,8 +94,8 @@ export function CreateHTTPRequest(label: string, baseURL: string, method: string return $Call.ByID(2634895949, label, baseURL, method, body, authType, headers, openAPISpec, auth, jose, description); } -export function CreateList(label: string, entries: { [_ in string]?: string } | null): $CancellablePromise { - return $Call.ByID(1760985996, label, entries); +export function CreateList(label: string, description: string, columns: typedfield$0.Field[] | null): $CancellablePromise { + return $Call.ByID(1760985996, label, description, columns); } export function CreateMCPServer(label: string, command: string, args: string[] | null): $CancellablePromise { @@ -132,6 +145,10 @@ export function DeleteList(id: string): $CancellablePromise { return $Call.ByID(1223896803, id); } +export function DeleteListRow(listID: string, rowID: string): $CancellablePromise { + return $Call.ByID(2135971241, listID, rowID); +} + export function DeleteMCPServer(id: string): $CancellablePromise { return $Call.ByID(3847603582, id); } @@ -309,8 +326,16 @@ export function UpdateHTTPRequest(id: string, label: string, baseURL: string, me return $Call.ByID(248664070, id, label, baseURL, method, body, authType, headers, openAPISpec, auth, jose, description); } -export function UpdateList(id: string, label: string, entries: { [_ in string]?: string } | null): $CancellablePromise { - return $Call.ByID(437956429, id, label, entries); +export function UpdateList(id: string, label: string, description: string, columns: typedfield$0.Field[] | null): $CancellablePromise { + return $Call.ByID(437956429, id, label, description, columns); +} + +/** + * UpdateListRow replaces one row's Values/Status (its ID/CreatedAt + * stay put; UpdatedAt is stamped here, not client-supplied). + */ +export function UpdateListRow(listID: string, rowID: string, values: { [_ in string]?: string } | null, status: list$0.RowStatus): $CancellablePromise { + return $Call.ByID(783553507, listID, rowID, values, status); } export function UpdateMCPServer(id: string, label: string, command: string, args: string[] | null): $CancellablePromise { diff --git a/frontend/e2e/composition.spec.ts b/frontend/e2e/composition.spec.ts index c9e8c28b..f6500c0a 100644 --- a/frontend/e2e/composition.spec.ts +++ b/frontend/e2e/composition.spec.ts @@ -190,8 +190,10 @@ test('Composition page lists built-in workflows; node primitives live in a colla // node, docs/adr/0027) + code-execution (docs/adr/0026's code // execution capability, goal 0004b) + capture-file, // process-extract-html, capture-clipboard-info (the save-page - // capture floor + clipboard inspector, docs/adr/0030 / SPEC.md §5). - await expect(activePanel(page).getByTestId('palette-item')).toHaveCount(24) + // capture floor + clipboard inspector, docs/adr/0030 / SPEC.md §5) + + // list-search (docs/goals/0011-lists-maturation.md's richer, typed + // successor to list-lookup). + await expect(activePanel(page).getByTestId('palette-item')).toHaveCount(25) }) test('A new workflow starts with a starter node placed, not a blank canvas', async ({ page }) => { diff --git a/frontend/e2e/configure-export-import.spec.ts b/frontend/e2e/configure-export-import.spec.ts index 2aa85cfd..83a146bf 100644 --- a/frontend/e2e/configure-export-import.spec.ts +++ b/frontend/e2e/configure-export-import.spec.ts @@ -57,19 +57,23 @@ test('Importing a Request file adds a new, independent request', async ({ page } await expect(importedRow).toHaveCount(0) }) -test('Exporting and importing a List round-trips its entries', async ({ page }) => { +test('Exporting and importing a List round-trips its typed columns and rows', async ({ page }) => { await page.goto('/') await page.getByRole('link', { name: 'Configure' }).click() await page.getByRole('tab', { name: 'Lists' }).click() await page.getByTestId('new-list').click() await page.getByLabel('Label').fill('E2E export list') - await page.getByPlaceholder('key').fill('color') - await page.getByPlaceholder('value').fill('blue') + await page.getByTestId('list-column-key').fill('color') await page.getByRole('button', { name: 'Save list' }).click() + await page.getByTestId('add-list-row').click() + await page.getByTestId('list-row').getByRole('textbox').fill('blue') + await page.getByTestId('save-list-row').click() + const originalRow = page.locator('[data-testid="inventory-row"][data-entity="list"]', { has: page.getByText('E2E export list', { exact: true }) }) await expect(originalRow).toBeVisible() + await expect(originalRow).toContainText('1 columns, 1 rows') const downloadPromise = page.waitForEvent('download') await clickRowAction(page, originalRow, 'Export') @@ -78,7 +82,16 @@ test('Exporting and importing a List round-trips its entries', async ({ page }) const chunks: Buffer[] = [] for await (const chunk of stream) chunks.push(chunk as Buffer) const json = Buffer.concat(chunks).toString('utf-8') - expect(JSON.parse(json).entries).toEqual({ color: 'blue' }) + const parsed = JSON.parse(json) + // internal/domain/typedfield.Field and internal/domain/list.Row carry + // no json struct tags of their own, so their fields marshal under + // their real Go names (Key, Values, ...) even though the top-level + // exportedList wrapper fields do (columns/rows, configureservice_ + // export.go's own json tags). + expect(parsed.columns).toHaveLength(1) + expect(parsed.columns[0].Key).toBe('color') + expect(parsed.rows).toHaveLength(1) + expect(parsed.rows[0].Values.color).toBe('blue') await page.getByTestId('import-list').click() await page.getByTestId('import-list-input').setInputFiles({ diff --git a/frontend/e2e/configure-lists.spec.ts b/frontend/e2e/configure-lists.spec.ts new file mode 100644 index 00000000..a7dfdbdc --- /dev/null +++ b/frontend/e2e/configure-lists.spec.ts @@ -0,0 +1,160 @@ +import { test, expect } from './fixtures/server' +import { clickRowAction } from './inventoryRow' + +// docs/goals/0011-lists-maturation.md: exercises the typed List +// Configure UI (column schema editor + schema-generated row editor, +// ConfigureLists.tsx) and the list-search node's own Inspector +// (ListSearchParamsEditor.tsx, a column picker + literal-or-attribute +// value + exact/fuzzy match type) built live through the canvas -- +// not just against the hardcoded seed (seed-completeness.spec.ts +// already covers running the seed). Deletes the workflow it creates, +// same shared-settings-file discipline every other spec here follows; +// reuses the seeded "Example: Country codes" List rather than +// creating a second one, so there's nothing List-shaped to clean up. + +function workflowRow(page: import('@playwright/test').Page, label: string) { + return page.locator('[data-testid="inventory-row"][data-entity="workflow"]', { has: page.getByText(label, { exact: true }) }) +} + +function activePanel(page: import('@playwright/test').Page) { + return page.locator('[role="tabpanel"]:not([hidden])').last() +} + +async function dragPaletteItemToCanvas(page: import('@playwright/test').Page, nodeTypeID: string) { + await page.evaluate((id) => { + const panel = document.querySelector('[role="tabpanel"]:not([hidden])') + if (!panel) throw new Error('no active tabpanel') + const palette = panel.querySelector(`[data-node-type-id="${id}"]`) + const canvas = panel.querySelector('.react-flow__pane') + if (!palette || !canvas) throw new Error(`drag setup failed: palette found=${!!palette} canvas found=${!!canvas}`) + const dataTransfer = new DataTransfer() + const rect = canvas.getBoundingClientRect() + const clientX = rect.x + rect.width / 2 + const clientY = rect.y + rect.height / 2 + palette.dispatchEvent(new DragEvent('dragstart', { bubbles: true, cancelable: true, dataTransfer })) + canvas.dispatchEvent(new DragEvent('dragover', { bubbles: true, cancelable: true, dataTransfer, clientX, clientY })) + canvas.dispatchEvent(new DragEvent('drop', { bubbles: true, cancelable: true, dataTransfer, clientX, clientY })) + }, nodeTypeID) +} + +async function connectNodes(page: import('@playwright/test').Page, sourceLabel: string, targetLabel: string) { + const panel = activePanel(page) + await panel.getByRole('button', { name: 'Fit View' }).click() + await page.waitForTimeout(300) + const sourceHandle = panel.locator('.react-flow__node').filter({ hasText: sourceLabel }).locator('.react-flow__handle.source') + const targetHandle = panel.locator('.react-flow__node').filter({ hasText: targetLabel }).locator('.react-flow__handle.target') + const sourceBox = await sourceHandle.boundingBox() + const targetBox = await targetHandle.boundingBox() + if (!sourceBox || !targetBox) throw new Error('connectNodes: handle bounding box not found') + await page.mouse.move(sourceBox.x + sourceBox.width / 2, sourceBox.y + sourceBox.height / 2) + await page.mouse.down() + await page.mouse.move(targetBox.x + targetBox.width / 2, targetBox.y + targetBox.height / 2, { steps: 10 }) + await page.mouse.up() +} + +async function clickCanvasNode(page: import('@playwright/test').Page, panel: import('@playwright/test').Locator, label: string) { + const node = panel.locator('.react-flow__node').filter({ hasText: label }) + const box = await node.boundingBox() + if (!box) throw new Error(`clickCanvasNode: node "${label}" has no bounding box`) + const candidates = [ + { x: box.x + 10, y: box.y + 10 }, + { x: box.x + box.width - 10, y: box.y + 10 }, + { x: box.x + box.width / 2, y: box.y + box.height / 2 }, + { x: box.x + 10, y: box.y + box.height - 10 }, + ] + for (const point of candidates) { + const insideNode = await page.evaluate(({ x, y }) => { + const el = document.elementFromPoint(x, y) + return !!el?.closest('.react-flow__node') + }, point) + if (insideNode) { + await page.mouse.click(point.x, point.y) + return + } + } + throw new Error(`clickCanvasNode: no point for node "${label}" resolved inside its own card`) +} + +test('Configuring a typed List: add a column, add a row, both persist', async ({ page }) => { + await page.goto('/') + await page.getByRole('link', { name: 'Configure' }).click() + await page.getByRole('tab', { name: 'Lists' }).click() + + await page.getByTestId('new-list').click() + await page.getByLabel('Label').fill('E2E typed list UI') + await page.getByTestId('list-column-key').fill('sku') + await page.getByRole('button', { name: 'Save list' }).click() + + await expect(page.getByTestId('list-rows-editor')).toBeVisible() + await page.getByTestId('add-list-row').click() + const row = page.getByTestId('list-row') + await expect(row).toBeVisible() + await row.getByRole('textbox').fill('SKU-1') + await row.getByTestId('save-list-row').click() + + await page.getByRole('button', { name: 'Close' }).click() + const listRow = page.locator('[data-testid="inventory-row"][data-entity="list"]', { has: page.getByText('E2E typed list UI', { exact: true }) }) + await expect(listRow).toBeVisible() + await expect(listRow).toContainText('1 columns, 1 rows') + + // Clean up. + await clickRowAction(page, listRow, 'Delete') + await expect(listRow).toHaveCount(0) +}) + +test('list-search node: configuring a real match parameter through the Inspector, then running it end to end', async ({ page }) => { + await page.goto('/') + await page.getByRole('link', { name: 'Workflows' }).click() + await page.getByTestId('new-workflow').click() + const panel = activePanel(page) + await panel.getByLabel('Label').fill('E2E list-search config test') + + // Deliberately no apply-clipboard-write-text terminal node -- this + // test proves the list-search Inspector's own authoring/execution, + // not clipboard I/O, and a clipboard apply step has no clipboard on + // a headless Linux CI runner (docs/SPEC.md §1.3; the same fix + // applied to the seeded "Example: Country lookup (search)" workflow + // after a real CI failure, builtinworkflows_list.go). Ending at + // list-search itself is an accepted, warn-only Process leaf + // (ADR-0028). + await panel.getByTestId('toggle-palette').click() + await dragPaletteItemToCanvas(page, 'list-search') + await expect(panel.locator('.react-flow__node')).toHaveCount(2) + await connectNodes(page, 'Trigger: manual', 'List: search') + + await clickCanvasNode(page, panel, 'List: search') + const inspector = panel.getByTestId('composition-inspector') + + // Pick the seeded typed List via the live entity picker (ADR-0009). + await inspector.getByTestId('entity-ref-field').selectOption({ label: 'Example: Country codes' }) + + const editor = inspector.getByTestId('list-search-params-editor') + await expect(editor).toBeVisible() + await editor.getByTestId('add-list-search-param').click() + // The column Select now offers the real List's own columns (code, name). + await editor.getByTestId('list-search-param-column').selectOption({ label: 'Code' }) + await editor.getByLabel('Value literal value').fill('US') + + // outputAttribute is a plain generic ConfigField (not owned by the + // bespoke editor) -- filled via its own normal labeled text input. + await inspector.getByLabel('Output attribute').fill('searchResult') + + await panel.getByTestId('save-workflow').click() + + const row = workflowRow(page, 'E2E list-search config test') + await expect(row).toBeVisible() + await row.click() + + // Manual trigger, no declared Attributes -- Run fires immediately, + // no test-input dialog (docs/adr/0008 only opens one when Attributes + // are declared). + await activePanel(page).getByTestId('canvas-run').click() + const bar = activePanel(page).getByTestId('current-step-bar') + await expect(bar).toContainText('SUCCESS', { timeout: 15_000 }) + + // Clean up. + await page.getByRole('link', { name: 'Workflows' }).click() + const wfRow = workflowRow(page, 'E2E list-search config test') + await clickRowAction(page, wfRow, 'Delete') + await expect(wfRow).toHaveCount(0) +}) diff --git a/frontend/e2e/seed-completeness.spec.ts b/frontend/e2e/seed-completeness.spec.ts index 6be33092..603935b3 100644 --- a/frontend/e2e/seed-completeness.spec.ts +++ b/frontend/e2e/seed-completeness.spec.ts @@ -37,7 +37,10 @@ test('Seeded List "Example: Country codes" is present, built-in-badged, with its const row = page.locator('[data-testid="inventory-row"][data-entity="list"]').filter({ has: page.getByText('Example: Country codes', { exact: true }) }) await expect(row).toBeVisible() await expect(row.getByText('built-in', { exact: true })).toBeVisible() - await expect(row).toContainText('3 entries') + // docs/goals/0011-lists-maturation.md: the seed grew typed + // code/name columns + 5 rows (4 Active + 1 deliberately Expired), + // replacing the old flat key/value "N entries" description. + await expect(row).toContainText('2 columns, 5 rows') }) test('Seeded MCP Server "Example: Reference server (npx)" is present, built-in-badged, pointed at the real reference server', async ({ page }) => { @@ -76,6 +79,28 @@ test('Example: Country code lookup runs a real match through the seeded List', a await expect(bar).toContainText('SUCCESS', { timeout: 15_000 }) }) +test('Example: Country lookup (search) runs a real exact match through list-search', async ({ page }) => { + await page.goto('/') + await page.getByRole('link', { name: 'Workflows' }).click() + + const row = workflowRow(page, 'Example: Country lookup (search)') + await expect(row).toBeVisible() + await row.click() + await expect(activePanel(page).locator('.react-flow__node').first()).toBeVisible() + + await activePanel(page).getByTestId('canvas-run').click() + + // The workflow declares 'code'/'searchResult' Attributes -- only + // 'code' matters for this run (list-search always overwrites + // 'searchResult' with its own typed result, docs/goals/0011). + const dialog = page.getByRole('dialog') + await dialog.getByLabel('Code').fill('US') + await dialog.getByRole('button', { name: 'Run' }).click() + + const bar = activePanel(page).getByTestId('current-step-bar') + await expect(bar).toContainText('SUCCESS', { timeout: 15_000 }) +}) + test('Example: MCP echo call workflow is present with the real mcp-tool-call node on canvas', async ({ page }) => { await page.goto('/') await page.getByRole('link', { name: 'Workflows' }).click() diff --git a/frontend/src/composition/ListSearchParamsEditor.tsx b/frontend/src/composition/ListSearchParamsEditor.tsx new file mode 100644 index 00000000..81c9fe23 --- /dev/null +++ b/frontend/src/composition/ListSearchParamsEditor.tsx @@ -0,0 +1,139 @@ +import { useEffect, useState } from 'react' +import { Button, FormControl, IconButton, Select, Stack, Text, TextInput } from '@primer/react' +import { PlusIcon, TrashIcon } from '@primer/octicons-react' +import { ConfigureService } from '../shared/bindings' +import type { List } from '../../bindings/github.com/alicoding/mill/internal/domain/list/models' +import type { AttributeDef } from '../../bindings/github.com/alicoding/mill/internal/domain/composition/models' +import { LiteralOrAttributeField } from '../shared/LiteralOrAttributeField' +import styles from '../shared/ListCard.module.css' + +// The list-search node's match-parameter editor (docs/goals/0011- +// lists-maturation.md item 4): once a List is picked, fetches its +// real Columns (ConfigureService.Lists(), the same data ConfigureLists +// itself edits) and renders one row per match parameter -- a column +// picker from the List's own declared columns, a literal-or-attribute +// value binding (the shared LiteralOrAttributeField every other +// binding editor in this folder already uses), an exact/fuzzy match +// type, and a threshold input shown only when fuzzy. Owns matchParams +// entirely -- NodeInspector.tsx skips it in its generic ConfigFields +// loop (same reasoning as MCPToolArgsEditor owning toolName/ +// argumentsJSON) and renders this component instead. +interface MatchParam { + column: string + value: string + matchType: 'exact' | 'fuzzy' + threshold?: number +} + +function parseParams(raw: string): MatchParam[] { + if (!raw) return [] + try { + const parsed: unknown = JSON.parse(raw) + return Array.isArray(parsed) ? (parsed as MatchParam[]) : [] + } catch { + return [] + } +} + +export function ListSearchParamsEditor({ + listId, matchParamsRaw, attrs, onChangeMatchParams, +}: { + listId: string + matchParamsRaw: string + attrs: AttributeDef[] + onChangeMatchParams: (raw: string) => void +}) { + const [lists, setLists] = useState(null) + + useEffect(() => { + ConfigureService.Lists().then((l) => setLists(l ?? [])).catch(() => setLists([])) + }, []) + + const selectedList = lists?.find((l) => l.ID === listId) + const columns = selectedList?.Columns ?? [] + + const params = parseParams(matchParamsRaw) + const writeParams = (next: MatchParam[]) => onChangeMatchParams(JSON.stringify(next)) + const updateParam = (i: number, patch: Partial) => + writeParams(params.map((p, idx) => (idx === i ? { ...p, ...patch } : p))) + const removeParam = (i: number) => writeParams(params.filter((_, idx) => idx !== i)) + const addParam = () => + writeParams([...params, { column: columns[0]?.Key ?? '', value: '', matchType: 'exact' }]) + + return ( + + Match parameters + {!listId && ( + Pick a List above first. + )} + {listId && columns.length === 0 && ( + + That List has no declared columns yet -- add some in Configure > Lists. + + )} + {params.map((p, i) => ( + + + + Column + + + + Match type + + + removeParam(i)} + /> + + updateParam(i, { value: v })} + /> + {p.matchType === 'fuzzy' && ( + + Threshold + 0..1 similarity (Damerau-Levenshtein); defaults to 0.7 if left blank. + { + const v = e.target.value + updateParam(i, { threshold: v === '' ? undefined : parseFloat(v) }) + }} + /> + + )} + + ))} + + + ) +} diff --git a/frontend/src/composition/NodeInspector.tsx b/frontend/src/composition/NodeInspector.tsx index 53fdc59b..cf7867d3 100644 --- a/frontend/src/composition/NodeInspector.tsx +++ b/frontend/src/composition/NodeInspector.tsx @@ -13,6 +13,7 @@ import { IntegrationBindingsEditor } from './IntegrationBindingsEditor' import { ChildWorkflowBindingsEditor } from './ChildWorkflowBindingsEditor' import { DecisionOutcomeBindingsEditor } from './DecisionOutcomeBindingsEditor' import { MCPToolArgsEditor } from './MCPToolArgsEditor' +import { ListSearchParamsEditor } from './ListSearchParamsEditor' import { WorkflowHoverPreview } from './WorkflowHoverPreview' import { NodeGuardrailSection } from './NodeGuardrailSection' import { NodeExecutionSection } from './NodeExecutionSection' @@ -204,6 +205,7 @@ export function NodeInspector({ node, workflowId, attrs, nodeType, sameKindNodeT // below instead of this generic loop -- a schema-driven tool // picker and typed-argument fields, not a raw text box. .filter((field) => !(node.data.nodeTypeID === 'mcp-tool-call' && (field.Key === 'toolName' || field.Key === 'argumentsJSON'))) + .filter((field) => !(node.data.nodeTypeID === 'list-search' && field.Key === 'matchParams')) .map((field) => ( {field.Label} @@ -353,6 +355,15 @@ export function NodeInspector({ node, workflowId, attrs, nodeType, sameKindNodeT /> )} + {node.data.nodeTypeID === 'list-search' && ( + onConfigChange('matchParams', raw)} + /> + )} + {node.data.nodeTypeID === 'decision-outcome' && ( = { + [ConfigFieldType.TypeText]: 'Text', + [ConfigFieldType.TypeNumber]: 'Number', + [ConfigFieldType.TypeBoolean]: 'Boolean', } -function entriesToRows(entries: { [key: string]: string | undefined } | null | undefined): EntryRow[] { - return Object.entries(entries ?? {}).map(([key, value]) => ({ key, value: value ?? '' })) -} - -function rowsToEntries(rows: EntryRow[]): Record { - const out: Record = {} - for (const r of rows) { - if (r.key.trim() !== '') out[r.key] = r.value +function emptyColumn(): Field { + return { + Key: '', Label: '', Type: ConfigFieldType.TypeText, Required: false, Default: '', Description: '', + Options: null, Suggestions: null, Secret: false, RefKind: '', Multiline: false, SystemManaged: false, } - return out } -// Configure's Lists section (docs/SPEC.md §3.5): CRUD over -// ConfigureService's Lists, each a named key/value lookup table a -// workflow's list-lookup node can resolve against (composition.go's -// SetListLookup seam). -// -// Rows are the DEFAULT view (docs/goals/0007): InventoryList's shared -// row replaces the old hand-rolled card branch. Row click edits -// (today's only real per-row interaction, same as before this goal); -// Export/Delete move into the trailing ⋯ menu. +// Configure's Lists section (docs/SPEC.md §3.5, docs/goals/0011-lists- +// maturation.md): CRUD over ConfigureService's typed Lists -- a +// key/label/type Column-schema editor mirroring ConfigureAttributes. +// tsx's own flat style (the goal's own instruction), plus a schema- +// generated row editor once a list's columns are saved. Both a +// list-lookup and a list-search workflow node resolve against these +// same Columns/Rows. export function ConfigureLists() { const [lists, setLists] = useState(null) const [editingID, setEditingID] = useState(null) const [label, setLabel] = useState('') - const [rows, setRows] = useState([]) + const [description, setDescription] = useState('') + const [columns, setColumns] = useState([]) const [formOpen, setFormOpen] = useState(false) const [error, setError] = useState('') + const [rowError, setRowError] = useState('') const [importError, setImportError] = useState(null) const importInputRef = useRef(null) const [viewMode, setViewMode] = useViewMode('mill-lists-view-mode') const refetch = () => { - ConfigureService.Lists().then((list) => setLists(list ?? [])).catch(console.error) + ConfigureService.Lists().then((l) => setLists(l ?? [])).catch(console.error) } const exportList = (id: string, label: string) => { @@ -79,32 +78,48 @@ export function ConfigureLists() { useEffect(refetch, []) + const editingList = lists?.find((l) => l.ID === editingID) ?? null + const startCreate = () => { setEditingID(null) setLabel('') - setRows([{ key: '', value: '' }]) + setDescription('') + setColumns([emptyColumn()]) setFormOpen(true) setError('') + setRowError('') } const startEdit = (l: List) => { setEditingID(l.ID) setLabel(l.Label) - setRows(entriesToRows(l.Entries).length > 0 ? entriesToRows(l.Entries) : [{ key: '', value: '' }]) + setDescription(l.Description) + setColumns(l.Columns && l.Columns.length > 0 ? l.Columns : [emptyColumn()]) setFormOpen(true) setError('') + setRowError('') + } + + const updateColumn = (i: number, field: keyof Field, value: string) => { + setColumns((prev) => prev.map((c, idx) => (idx === i ? { ...c, [field]: value } : c))) } - const save = async () => { + const saveSchema = async () => { setError('') try { - const entries = rowsToEntries(rows) + // Drop any never-touched blank column row (the default starting + // state for a new list, and what's left if the user deletes down + // to nothing) -- same "an empty draft row isn't a real column" + // filtering the old key/value editor's rowsToEntries applied, so + // Save still works with zero columns declared, not just a full one. + const nonEmptyColumns = columns.filter((c) => c.Key.trim() !== '') + let saved: List if (editingID) { - await ConfigureService.UpdateList(editingID, label, entries) + saved = await ConfigureService.UpdateList(editingID, label, description, nonEmptyColumns) } else { - await ConfigureService.CreateList(label, entries) + saved = await ConfigureService.CreateList(label, description, nonEmptyColumns) } - setFormOpen(false) + setEditingID(saved.ID) refetch() } catch (err) { setError(String(err)) @@ -124,8 +139,39 @@ export function ConfigureLists() { onConfirm: (l) => remove(l.ID), }) - const updateRow = (i: number, field: 'key' | 'value', value: string) => { - setRows((prev) => prev.map((r, idx) => (idx === i ? { ...r, [field]: value } : r))) + const addRow = async () => { + if (!editingID) return + setRowError('') + const values: Record = {} + for (const c of columns) values[c.Key] = '' + try { + await ConfigureService.AddListRow(editingID, values) + refetch() + } catch (err) { + setRowError(String(err)) + } + } + + const updateRow = async (rowID: string, values: Record, status: RowStatus) => { + if (!editingID) return + setRowError('') + try { + await ConfigureService.UpdateListRow(editingID, rowID, values, status) + refetch() + } catch (err) { + setRowError(String(err)) + } + } + + const deleteRow = async (rowID: string) => { + if (!editingID) return + setRowError('') + try { + await ConfigureService.DeleteListRow(editingID, rowID) + refetch() + } catch (err) { + setRowError(String(err)) + } } // Last-updated-first, applied once so both view modes render the @@ -142,7 +188,7 @@ export function ConfigureLists() { // fully editable/deletable (docs/SPEC.md §2.2's Update note), same // as ConfigureRequests.tsx's identical badge. labelBadges: l.BuiltIn ? : undefined, - description: `${Object.keys(l.Entries ?? {}).length} entries`, + description: `${(l.Columns ?? []).length} columns, ${(l.Rows ?? []).length} rows`, onOpen: () => startEdit(l), menuActions: [ { label: 'Export', onClick: () => exportList(l.ID, l.Label) }, @@ -183,36 +229,74 @@ export function ConfigureLists() { {formOpen && ( -
- - - Label - setLabel(e.target.value)} block /> - - Entries - {rows.map((row, i) => ( - - updateRow(i, 'key', e.target.value)} /> - updateRow(i, 'value', e.target.value)} /> - setRows((prev) => prev.filter((_, idx) => idx !== i))} - /> +
+ + + Label + setLabel(e.target.value)} block data-testid="list-label" /> + + + Description + setDescription(e.target.value)} block /> + + + Columns + {columns.map((c, i) => ( + + updateColumn(i, 'Key', e.target.value)} data-testid="list-column-key" /> + updateColumn(i, 'Label', e.target.value)} /> + + setColumns((prev) => prev.filter((_, idx) => idx !== i))} + /> + + ))} + + + {error && {error}} + + + - ))} - - {error && {error}} - - - - -
+
+ + {editingID && editingList && ( +
+ + Rows + {(editingList.Columns ?? []).length === 0 ? ( + Add at least one column, then Save, to start adding rows. + ) : ( + <> + {(editingList.Rows ?? []).map((r) => ( + updateRow(r.ID, values, status)} + onDelete={() => deleteRow(r.ID)} + /> + ))} + + + )} + {rowError && {rowError}} + +
+ )} )} @@ -224,7 +308,8 @@ export function ConfigureLists() { data={sortedLists.map((l) => ({ ...l, id: l.ID }))} columns={[ { header: 'Label', field: 'Label', rowHeader: true, sortBy: 'alphanumeric' }, - { header: 'Entries', id: 'entries', width: 'auto', renderCell: (l) => Object.keys(l.Entries ?? {}).length }, + { header: 'Columns', id: 'columns', width: 'auto', renderCell: (l) => (l.Columns ?? []).length }, + { header: 'Rows', id: 'rows', width: 'auto', renderCell: (l) => (l.Rows ?? []).length }, { header: 'ID', field: 'ID' }, { header: '', id: 'actions', width: 'auto', align: 'end', @@ -247,7 +332,7 @@ export function ConfigureLists() { emptyState={{ icon: ListUnorderedIcon, heading: 'No lists yet', - description: "A reusable key/value lookup table a workflow's List node can resolve against.", + description: "A reusable typed dataset a workflow's List Search (or List Lookup) node can resolve against.", action: , }} /> @@ -256,3 +341,62 @@ export function ConfigureLists() { ) } + +// One row's inline editor -- a type-aware input per declared Column +// (text/number/boolean; TypeOptions renders a Select over the +// column's own Options, same as ConfigField's generic Inspector +// rendering elsewhere) plus an Active/Expired status Select. Local +// draft state with an explicit Save, rather than per-keystroke RPCs. +function RowEditor({ row, columns, onSave, onDelete }: { + row: Row + columns: Field[] + onSave: (values: Record, status: RowStatus) => void + onDelete: () => void +}) { + const [values, setValues] = useState>( + Object.fromEntries(Object.entries(row.Values ?? {}).map(([k, v]) => [k, v ?? ''])), + ) + const [status, setStatus] = useState(row.Status) + + const setValue = (key: string, v: string) => setValues((prev) => ({ ...prev, [key]: v })) + + return ( + + + {columns.map((c) => ( + + {c.Label || c.Key} + {c.Type === ConfigFieldType.TypeBoolean ? ( + setValue(c.Key, String(e.target.checked))} + /> + ) : c.Type === ConfigFieldType.TypeOptions ? ( + + ) : ( + setValue(c.Key, e.target.value)} + /> + )} + + ))} + + + + + + ) +} diff --git a/frontend/src/configure/EntityRefField.tsx b/frontend/src/configure/EntityRefField.tsx index c9f62448..86ed1b55 100644 --- a/frontend/src/configure/EntityRefField.tsx +++ b/frontend/src/configure/EntityRefField.tsx @@ -168,7 +168,7 @@ function QuickCreateDialog({ refKind, onCancel, onCreated }: { refKind: string; break } case 'list': { - const l = await ConfigureService.CreateList(label, null) + const l = await ConfigureService.CreateList(label, '', null) id = l.ID break } diff --git a/go.mod b/go.mod index 2bdc8809..076374d0 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/go-jose/go-jose/v4 v4.1.4 github.com/google/uuid v1.6.0 github.com/hashicorp/go-retryablehttp v0.7.8 + github.com/hbollon/go-edlib v1.7.0 github.com/modelcontextprotocol/go-sdk v1.7.0 github.com/netresearch/go-cron v0.15.1 github.com/wailsapp/wails/v3 v3.0.0-beta.4 diff --git a/go.sum b/go.sum index 2960dfd7..a411b00b 100644 --- a/go.sum +++ b/go.sum @@ -65,6 +65,8 @@ github.com/hashicorp/go-retryablehttp v0.7.8 h1:ylXZWnqa7Lhqpk0L1P1LzDtGcCR0rPVU github.com/hashicorp/go-retryablehttp v0.7.8/go.mod h1:rjiScheydd+CxvumBsIrFKlx3iS0jrZ7LvzFGFmuKbw= github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= +github.com/hbollon/go-edlib v1.7.0 h1:Jt3AtZ+AdgtJhzkrCFvkbdbNL3KCqZlGioLnUfwsxeU= +github.com/hbollon/go-edlib v1.7.0/go.mod h1:wnt6o6EIVEzUfgbUZY7BerzQ2uvzp354qmS2xaLkrhM= github.com/jackc/pgerrcode v0.0.0-20250907135507-afb5586c32a6 h1:D/V0gu4zQ3cL2WKeVNVM4r2gLxGGf6McLwgXzRTo2RQ= github.com/jackc/pgerrcode v0.0.0-20250907135507-afb5586c32a6/go.mod h1:a/s9Lp5W7n/DD0VrVoyJ00FbP2ytTPDVOivvn2bMlds= github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= diff --git a/internal/adapters/fuzzymatch/fuzzymatch.go b/internal/adapters/fuzzymatch/fuzzymatch.go new file mode 100644 index 00000000..9f5e3bdc --- /dev/null +++ b/internal/adapters/fuzzymatch/fuzzymatch.go @@ -0,0 +1,59 @@ +// Package fuzzymatch wraps github.com/hbollon/go-edlib (MIT, actively +// maintained, zero external dependencies -- confirmed directly against +// its go.mod) behind Mill's own names, the same +// internal/adapters/expression precedent (.claude/rules/backend.md): +// keep a commodity matching library behind a small adapter so a +// future swap never touches domain code +// (internal/domain/composition/listsearch.go). +// +// Default algorithm is Damerau-Levenshtein -- docs/goals/0011-lists- +// maturation.md's own decided default, from industry research: the +// most explainable edit-distance algorithm (a human can reason about +// "N character edits including one transposition"), and the default +// both Elasticsearch's fuzzy query and OpenRefine's own key-collision +// clustering ship with. A per-column Jaro-Winkler override (its +// Census name-matching origin makes it a better fit for a "Name"-typed +// column specifically) is real future work, deliberately deferred -- +// it needs a typed Name column subtype that doesn't exist yet. +// +// Exact-match lookups never route through this package at all -- +// plain equality stays plain equality +// (internal/domain/composition/listsearch.go's own exact branch). +package fuzzymatch + +import ( + "sort" + + edlib "github.com/hbollon/go-edlib" +) + +// Match is one candidate string that met the similarity threshold, +// with its similarity score (0..1, edlib's own normalized range: 1.0 +// is an exact match, 0.0 is completely dissimilar). +type Match struct { + Value string + Score float64 +} + +// Search returns every candidate whose Damerau-Levenshtein similarity +// to query is >= threshold (0..1), sorted best-match-first (ties keep +// candidates' original relative order). A candidate edlib can't score +// (its own StringsSimilarity only errors on an empty algorithm-input +// edge case) is skipped rather than failing the whole search -- +// there's no reasonable per-candidate error to surface to a workflow +// author here, and skipping degrades to "that one candidate never +// matches" rather than aborting every other candidate's real result. +func Search(query string, candidates []string, threshold float64) []Match { + var out []Match + for _, c := range candidates { + sim, err := edlib.StringsSimilarity(query, c, edlib.DamerauLevenshtein) + if err != nil { + continue + } + if float64(sim) >= threshold { + out = append(out, Match{Value: c, Score: float64(sim)}) + } + } + sort.SliceStable(out, func(i, j int) bool { return out[i].Score > out[j].Score }) + return out +} diff --git a/internal/adapters/fuzzymatch/fuzzymatch_test.go b/internal/adapters/fuzzymatch/fuzzymatch_test.go new file mode 100644 index 00000000..648d466d --- /dev/null +++ b/internal/adapters/fuzzymatch/fuzzymatch_test.go @@ -0,0 +1,47 @@ +package fuzzymatch + +import "testing" + +func TestSearch_ExactMatch_ScoresOne(t *testing.T) { + got := Search("France", []string{"France"}, 0.5) + if len(got) != 1 || got[0].Value != "France" || got[0].Score != 1.0 { + t.Fatalf("Search(exact) = %+v, want one Match{France, 1.0}", got) + } +} + +func TestSearch_Typo_MatchesAboveThreshold(t *testing.T) { + // One transposition -- Damerau-Levenshtein's whole reason for being + // over plain Levenshtein (which would count it as two edits). + got := Search("Fracne", []string{"France"}, 0.7) + if len(got) != 1 || got[0].Value != "France" { + t.Fatalf("Search(typo) = %+v, want a match against France", got) + } + if got[0].Score <= 0 || got[0].Score >= 1.0 { + t.Errorf("Search(typo) score = %v, want strictly between 0 and 1", got[0].Score) + } +} + +func TestSearch_BelowThreshold_Excluded(t *testing.T) { + got := Search("Zzzzzz", []string{"France"}, 0.9) + if len(got) != 0 { + t.Errorf("Search(dissimilar, high threshold) = %+v, want no matches", got) + } +} + +func TestSearch_MultipleCandidates_SortedBestFirst(t *testing.T) { + got := Search("France", []string{"Franc", "Francia", "Germany"}, 0.3) + if len(got) < 2 { + t.Fatalf("Search = %+v, want at least 2 matches", got) + } + for i := 1; i < len(got); i++ { + if got[i-1].Score < got[i].Score { + t.Errorf("Search results not sorted best-first: %+v", got) + } + } +} + +func TestSearch_EmptyCandidates_ReturnsEmpty(t *testing.T) { + if got := Search("France", nil, 0.5); len(got) != 0 { + t.Errorf("Search(no candidates) = %+v, want empty", got) + } +} diff --git a/internal/domain/composition/builtinworkflows.go b/internal/domain/composition/builtinworkflows.go index 9eaea3fc..2e92b4e4 100644 --- a/internal/domain/composition/builtinworkflows.go +++ b/internal/domain/composition/builtinworkflows.go @@ -4,7 +4,6 @@ import ( "github.com/alicoding/mill/internal/domain/decision" "github.com/alicoding/mill/internal/domain/execenv" "github.com/alicoding/mill/internal/domain/httprequest" - "github.com/alicoding/mill/internal/domain/list" "github.com/alicoding/mill/internal/domain/mcpserver" ) @@ -201,31 +200,6 @@ func BuiltInWorkflows() []Workflow { panic("built-in workflow references an unknown node type: " + err.Error()) } - // List lookup (docs/goals/0010 item 4, docs/SPEC.md §3.3's List - // row): a typed 'code' Attribute is read into the payload via - // capture-attribute, then list-lookup resolves it against the - // seeded "Example: Country codes" List (Configure > Lists), - // writing the match into a second, declared 'countryName' - // Attribute -- the same "typed data flows through Attributes" - // pattern the parent/child example already established. - const ( - listTriggerID = "example-list-trigger" - listCaptureID = "example-list-capture" - listLookupID = "example-list-lookup" - ) - listNodes, err := ResolveNodeDefaults([]Node{ - {ID: listTriggerID, NodeTypeID: "trigger-manual", Position: Position{X: 0, Y: 0}}, - {ID: listCaptureID, NodeTypeID: "capture-attribute", Position: Position{X: 0, Y: 100}, - Config: map[string]string{"attribute": "code"}}, - {ID: listLookupID, NodeTypeID: "list-lookup", Position: Position{X: 0, Y: 200}, - Config: map[string]string{ - "listId": list.ExampleCountryCodesID, "inputKey": "code", "outputKey": "countryName", - }}, - }) - if err != nil { - panic("built-in workflow references an unknown node type: " + err.Error()) - } - // MCP tool call (docs/goals/0010 item 5, docs/SPEC.md §3.6): calls // the seeded "Example: Reference server (npx)" MCP Server's real // "echo" tool -- the exact round trip SPEC.md §3.6 already verified @@ -305,7 +279,7 @@ func BuiltInWorkflows() []Workflow { panic("built-in workflow references an unknown node type: " + err.Error()) } - return []Workflow{ + workflows := []Workflow{ { ID: "load-sample-html-workflow", Label: "Load sample HTML", @@ -421,21 +395,6 @@ func BuiltInWorkflows() []Workflow { }, BuiltIn: true, }, - { - ID: "example-list-lookup-workflow", - Label: "Example: Country code lookup", - Description: "Captures a typed 'code' Attribute and looks it up in the seeded \"Example: Country codes\" List (Configure > Lists), writing the match into a 'countryName' Attribute (docs/SPEC.md §3.3's List row). Run it with code = US, CA, or MX to see a match; any other code fails the run (the List node's own default \"If no match: fail\" behavior).", - Nodes: listNodes, - Attributes: []AttributeDef{ - {Key: "code", Label: "Code", Type: FieldText}, - {Key: "countryName", Label: "Country name", Type: FieldText}, - }, - Edges: []Edge{ - {ID: "example-list-e0", Source: listTriggerID, Target: listCaptureID}, - {ID: "example-list-e1", Source: listCaptureID, Target: listLookupID}, - }, - BuiltIn: true, - }, { ID: "example-mcp-echo-workflow", Label: "Example: MCP echo call", @@ -471,6 +430,16 @@ func BuiltInWorkflows() []Workflow { Disabled: true, }, } + + // List lookup + List search (docs/goals/0010 item 4, docs/goals/ + // 0011-lists-maturation.md item 4): split into their own file + // (builtinworkflows_list.go) once this function crossed the + // 500-line convention -- List was the newest, most self-contained + // addition (neither seed's nodes are referenced anywhere else in + // this file), the same "split along a real seam" discipline + // composition.go's own earlier split already established + // (.claude/rules/architecture.md). + return append(workflows, builtInListWorkflows()...) } // ExampleChildWorkflowID is exported so the parent seed above and any diff --git a/internal/domain/composition/builtinworkflows_list.go b/internal/domain/composition/builtinworkflows_list.go new file mode 100644 index 00000000..0d10276f --- /dev/null +++ b/internal/domain/composition/builtinworkflows_list.go @@ -0,0 +1,110 @@ +package composition + +import ( + "github.com/alicoding/mill/internal/domain/list" + "github.com/alicoding/mill/internal/domain/typedfield" +) + +// builtInListWorkflows returns the two seeded workflows exercising +// List (docs/goals/0010 item 4, docs/goals/0011-lists-maturation.md +// item 4): list-lookup (the original, simpler exact-key lookup) and +// list-search (goal 0011's richer typed-Object successor), both +// against the same seeded "Example: Country codes" List +// (internal/domain/list.BuiltIn). Split out of builtinworkflows.go +// once BuiltInWorkflows() crossed the 500-line convention -- see that +// function's own call site comment for the seam this follows. +func builtInListWorkflows() []Workflow { + // List lookup (docs/goals/0010 item 4, docs/SPEC.md §3.3's List + // row): a typed 'code' Attribute is read into the payload via + // capture-attribute, then list-lookup resolves it against the + // seeded "Example: Country codes" List (Configure > Lists), + // writing the match into a second, declared 'countryName' + // Attribute -- the same "typed data flows through Attributes" + // pattern the parent/child example already established. + const ( + listTriggerID = "example-list-trigger" + listCaptureID = "example-list-capture" + listLookupID = "example-list-lookup" + ) + listNodes, err := ResolveNodeDefaults([]Node{ + {ID: listTriggerID, NodeTypeID: "trigger-manual", Position: Position{X: 0, Y: 0}}, + {ID: listCaptureID, NodeTypeID: "capture-attribute", Position: Position{X: 0, Y: 100}, + Config: map[string]string{"attribute": "code"}}, + {ID: listLookupID, NodeTypeID: "list-lookup", Position: Position{X: 0, Y: 200}, + Config: map[string]string{ + "listId": list.ExampleCountryCodesID, "inputKey": "code", "outputKey": "countryName", + }}, + }) + if err != nil { + panic("built-in workflow references an unknown node type: " + err.Error()) + } + + // List search (docs/goals/0011-lists-maturation.md item 4): the + // richer successor to list-lookup above, against the same seeded + // "Example: Country codes" List (now typed, code/name columns). + // Demonstrates a typed Object result (results/matched/first_match/ + // match_count/list_id) a downstream step could branch on, not just + // a single scalar Attribute -- list-lookup's own seed above stays + // untouched, proving the two coexist. Deliberately ends AT + // list-search itself, mirroring list-lookup's own seed above, + // rather than adding a terminal apply-clipboard-write-text step: + // that step has no clipboard on a headless Linux CI runner + // (docs/SPEC.md §1.3) and would only be exercising clipboard I/O + // this seed isn't actually about -- caught by a real CI failure + // (goal 0011's own PR), not assumed. A Process leaf is an accepted, + // warn-only ending (ADR-0028), the same shape several other seeds + // already use. + const ( + listSearchTriggerID = "example-list-search-trigger" + listSearchCaptureID = "example-list-search-capture" + listSearchStepID = "example-list-search-step" + ) + const listSearchMatchParams = `[{"column":"code","value":"attr:code","matchType":"exact"}]` + listSearchNodes, err := ResolveNodeDefaults([]Node{ + {ID: listSearchTriggerID, NodeTypeID: "trigger-manual", Position: Position{X: 0, Y: 0}}, + {ID: listSearchCaptureID, NodeTypeID: "capture-attribute", Position: Position{X: 0, Y: 100}, + Config: map[string]string{"attribute": "code"}}, + {ID: listSearchStepID, NodeTypeID: "list-search", Position: Position{X: 0, Y: 200}, + Config: map[string]string{ + "listId": list.ExampleCountryCodesID, + "matchParams": listSearchMatchParams, + "outputAttribute": "searchResult", + }}, + }) + if err != nil { + panic("built-in workflow references an unknown node type: " + err.Error()) + } + + return []Workflow{ + { + ID: "example-list-lookup-workflow", + Label: "Example: Country code lookup", + Description: "Captures a typed 'code' Attribute and looks it up in the seeded \"Example: Country codes\" List (Configure > Lists), writing the match into a 'countryName' Attribute (docs/SPEC.md §3.3's List row). Run it with code = US, CA, or MX to see a match; any other code fails the run (the List node's own default \"If no match: fail\" behavior).", + Nodes: listNodes, + Attributes: []AttributeDef{ + {Key: "code", Label: "Code", Type: FieldText}, + {Key: "countryName", Label: "Country name", Type: FieldText}, + }, + Edges: []Edge{ + {ID: "example-list-e0", Source: listTriggerID, Target: listCaptureID}, + {ID: "example-list-e1", Source: listCaptureID, Target: listLookupID}, + }, + BuiltIn: true, + }, + { + ID: "example-list-search-workflow", + Label: "Example: Country lookup (search)", + Description: "Captures a typed 'code' Attribute and searches the seeded \"Example: Country codes\" List (Configure > Lists) via list-search -- an exact match on its 'code' column, writing a typed Object result ({results, matched, first_match, match_count, list_id}) into 'searchResult'. Unlike list-lookup's plain scalar output, this demonstrates the richer typed result a downstream step (e.g. a Branch condition on searchResult.matched) could reference. Run it with code = US, CA, MX, or FR to see a match -- SU is a deliberately Expired seed row, excluded from matching by default (docs/goals/0011-lists-maturation.md).", + Nodes: listSearchNodes, + Attributes: []AttributeDef{ + {Key: "code", Label: "Code", Type: FieldText}, + {Key: "searchResult", Label: "Search result", Type: typedfield.TypeObject}, + }, + Edges: []Edge{ + {ID: "example-list-search-e0", Source: listSearchTriggerID, Target: listSearchCaptureID}, + {ID: "example-list-search-e1", Source: listSearchCaptureID, Target: listSearchStepID}, + }, + BuiltIn: true, + }, + } +} diff --git a/internal/domain/composition/listlookup.go b/internal/domain/composition/listlookup.go index 20551cdd..6b992f14 100644 --- a/internal/domain/composition/listlookup.go +++ b/internal/domain/composition/listlookup.go @@ -1,15 +1,28 @@ package composition -import "fmt" +import ( + "fmt" -// ResolvedList is a List's entries, assembled by whatever owns List + "github.com/alicoding/mill/internal/domain/list" + "github.com/alicoding/mill/internal/domain/typedfield" +) + +// ResolvedList is a List's data, assembled by whatever owns List // storage at request time. Same shape and same reasoning as // ResolvedHTTPRequest (integration.go): composition.go doesn't own List // persistence (ConfigureService does), so this is injected once via // SetListLookup rather than composition depending on ConfigureService // directly. +// +// Entries stays list-lookup's own flat key/value read -- the +// resolver's own list.DeriveEntries output (goal 0011), computed +// once at resolve time so list-lookup's execution logic here needed +// zero changes when List grew typed columns. Columns/Rows are goal +// 0011's typed additions, read by list-search (listsearch.go). type ResolvedList struct { Entries map[string]string + Columns []typedfield.Field + Rows []list.Row } // lookupListFn defaults to erroring so a list-lookup node run before diff --git a/internal/domain/composition/listsearch.go b/internal/domain/composition/listsearch.go new file mode 100644 index 00000000..1d291bf2 --- /dev/null +++ b/internal/domain/composition/listsearch.go @@ -0,0 +1,173 @@ +package composition + +import ( + "encoding/json" + "fmt" + + "github.com/alicoding/mill/internal/adapters/fuzzymatch" + "github.com/alicoding/mill/internal/domain/list" +) + +// listSearchMatchParam is one row-filter criterion, JSON-encoded into +// the list-search node's "matchParams" ConfigField -- the same +// one-string-field-carries-a-JSON-array shape integration-http's +// inputBindings and mcp-tool-call's argumentsJSON already establish +// (Node.Config stays map[string]string, so a repeated/array-shaped +// config value is always JSON inside one field, never a second +// mechanism). Value is a literal or an "attr:" reference, +// resolved the same way every other binding in this package already +// does (resolveBindingValue, attributebinding.go). +// +// Multiple match parameters are AND'd together (docs/goals/0011- +// lists-maturation.md item 4: "multiple match parameters" reads as a +// composite-key filter -- e.g. matching first_name AND last_name -- +// the standard reading for "search using several criteria," not an OR +// union). +type listSearchMatchParam struct { + Column string `json:"column"` + Value string `json:"value"` + MatchType string `json:"matchType"` // "exact" (default) | "fuzzy" + Threshold float64 `json:"threshold,omitempty"` +} + +// defaultFuzzyThreshold applies when a fuzzy match param omits (or +// zeroes) its own Threshold -- 0.7 keeps typo-tolerance real without +// matching two genuinely different values. +const defaultFuzzyThreshold = 0.7 + +func init() { + RegisterNodeType(NodeType{ + ID: "list-search", Kind: KindProcess, + Label: "List: search", + Output: "payload unchanged; matches -> a typed Object attribute " + + "({results, matched, first_match, match_count, list_id})", + Description: "Searches a Configure-authored List's rows against one or more match parameters " + + "(exact or fuzzy, per-column, AND'd together) and writes the result into Attributes. " + + "Supersedes list-lookup for anything beyond a single exact key match -- list-lookup keeps " + + "working unchanged for existing workflows. Expired rows are excluded by default " + + "(docs/goals/0011-lists-maturation.md's own researched default, uniform across exact and " + + "fuzzy matching); \"Include expired rows\" opts in.", + ConfigFields: []ConfigField{ + { + Key: "listId", Label: "List", Type: FieldText, RefKind: "list", + Description: "The Configure-authored List to search.", + }, + { + Key: "matchParams", Label: "Match parameters", Type: FieldText, Multiline: true, + Description: `JSON array of match criteria, ALL must match (AND): ` + + `[{"column":"code","value":"attr:code","matchType":"exact"},` + + `{"column":"name","value":"Untied States","matchType":"fuzzy","threshold":0.7}]. ` + + `value is a literal or "attr:". Authored via the Inspector's match-parameter ` + + `rows; this raw field stays the LLM-authoring vocabulary (docs/adr/0025).`, + }, + { + Key: "includeExpired", Label: "Include expired rows", Type: FieldBoolean, Default: "false", + Description: "Off by default -- Expired rows never match unless explicitly included.", + }, + { + Key: "firstMatchOnly", Label: "Stop at first match", Type: FieldBoolean, Default: "false", + Description: "Stops scanning after the first match. The output shape stays the same " + + "typed Object either way -- results just has at most one entry -- so turning this on " + + "or off never changes what a downstream Decision/binding can reference.", + }, + { + Key: "outputAttribute", Label: "Output attribute", Type: FieldText, + Description: "Which Attributes field receives the typed search-result object.", + }, + }, + }, execListSearch) +} + +func execListSearch(node Node, ctx ExecContext) (ExecContext, error) { + listID := node.Config["listId"] + rl, err := lookupListFn(listID) + if err != nil { + return ctx, fmt.Errorf("list-search: %w", err) + } + + var params []listSearchMatchParam + if raw := node.Config["matchParams"]; raw != "" { + if err := json.Unmarshal([]byte(raw), ¶ms); err != nil { + return ctx, fmt.Errorf("list-search: invalid matchParams: %w", err) + } + } + if len(params) == 0 { + return ctx, fmt.Errorf("list-search: at least one match parameter is required") + } + + outKey := node.Config["outputAttribute"] + if outKey == "" { + return ctx, fmt.Errorf("list-search: outputAttribute is required") + } + + includeExpired := node.Config["includeExpired"] == "true" + firstOnly := node.Config["firstMatchOnly"] == "true" + + var results []map[string]string + for _, row := range rl.Rows { + if !includeExpired && row.Status == list.RowExpired { + continue + } + if listSearchRowMatches(row, params, ctx.Attributes) { + results = append(results, row.Values) + if firstOnly { + break + } + } + } + + var firstMatch any + if len(results) > 0 { + firstMatch = results[0] + } + if ctx.Attributes == nil { + ctx.Attributes = map[string]any{} + } + ctx.Attributes[outKey] = map[string]any{ + "results": results, + "matched": len(results) > 0, + "match_count": len(results), + "first_match": firstMatch, + // list_id: goal 0011 item 5's minimum evidence bar ("record the + // List identity used ... at minimum log it" -- full + // per-execution dataset-version snapshotting is deliberately + // deferred, see the goal file). Recorded inline with the very + // result it produced -- visible wherever the output Attribute + // itself is (e.g. a workflow's Runs tab) -- rather than an + // out-of-band log line: no logging convention exists anywhere + // in internal/domain/composition today, and introducing one + // here for a single node type would be a bigger, separate + // decision than this goal's own "at minimum" bar asks for. + "list_id": listID, + } + return ctx, nil +} + +// listSearchRowMatches reports whether every match parameter matches +// row (AND semantics -- see listSearchMatchParam's own doc comment). +// A column name that doesn't exist on the row (row.Values[p.Column] +// zero-valuing to "") simply fails to match rather than erroring -- +// the same permissive-miss reasoning list-lookup's own onMiss="fail" +// default still surfaces as a run failure, just one level up (no +// results at all), not a hard error from this function. +func listSearchRowMatches(row list.Row, params []listSearchMatchParam, attrs map[string]any) bool { + for _, p := range params { + colVal := row.Values[p.Column] + target := resolveBindingValue(p.Value, attrs) + switch p.MatchType { + case "fuzzy": + threshold := p.Threshold + if threshold <= 0 { + threshold = defaultFuzzyThreshold + } + if len(fuzzymatch.Search(target, []string{colVal}, threshold)) == 0 { + return false + } + default: // "exact" and any unrecognized value -- plain equality, never through the fuzzy lib + if colVal != target { + return false + } + } + } + return true +} diff --git a/internal/domain/composition/listsearch_test.go b/internal/domain/composition/listsearch_test.go new file mode 100644 index 00000000..850bc421 --- /dev/null +++ b/internal/domain/composition/listsearch_test.go @@ -0,0 +1,224 @@ +package composition + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/alicoding/mill/internal/domain/list" +) + +// Same injected-list harness shape TestListLookup_MissBehavior already +// establishes (listlookup_test.go) -- list-search reads the same +// SetListLookup seam, just against Columns/Rows instead of a flat +// Entries map. +func withTestList(t *testing.T, rows []list.Row) { + t.Helper() + SetListLookup(func(string) (ResolvedList, error) { + return ResolvedList{ + Columns: nil, + Rows: rows, + }, nil + }) + t.Cleanup(func() { + SetListLookup(func(id string) (ResolvedList, error) { + return ResolvedList{}, nil + }) + }) +} + +// execListSearchDirect runs list-search's own exec function directly +// (bypassing the full graph engine) so tests can assert on the real +// typed map[string]any result rather than its stringified payload +// form -- ExecContext.Attributes holds `any`, and Payload/ +// capture-attribute only ever round-trips through a string. +func execListSearchDirect(t *testing.T, matchParams []listSearchMatchParam, extraConfig, attrValues map[string]string) map[string]any { + t.Helper() + raw, err := json.Marshal(matchParams) + if err != nil { + t.Fatal(err) + } + config := map[string]string{ + "listId": "x", "matchParams": string(raw), "outputAttribute": "result", + } + for k, v := range extraConfig { + config[k] = v + } + attrs := map[string]any{} + for k, v := range attrValues { + attrs[k] = v + } + ctx, err := execListSearch(Node{ID: "s", NodeTypeID: "list-search", Config: config}, ExecContext{Attributes: attrs}) + if err != nil { + t.Fatalf("execListSearch: %v", err) + } + result, ok := ctx.Attributes["result"].(map[string]any) + if !ok { + t.Fatalf("Attributes[result] = %v (%T), want map[string]any", ctx.Attributes["result"], ctx.Attributes["result"]) + } + return result +} + +var seedRows = []list.Row{ + {ID: "row-us", Values: map[string]string{"code": "US", "name": "United States"}, Status: list.RowActive}, + {ID: "row-fr", Values: map[string]string{"code": "FR", "name": "France"}, Status: list.RowActive}, + {ID: "row-su", Values: map[string]string{"code": "SU", "name": "Soviet Union"}, Status: list.RowExpired}, +} + +func TestListSearch_ExactMatch_Hit(t *testing.T) { + withTestList(t, seedRows) + result := execListSearchDirect(t, + []listSearchMatchParam{{Column: "code", Value: "attr:code", MatchType: "exact"}}, + nil, map[string]string{"code": "US"}) + + if matched, _ := result["matched"].(bool); !matched { + t.Fatalf("result = %+v, want matched=true", result) + } + if count, _ := result["match_count"].(int); count != 1 { + t.Errorf("match_count = %v, want 1", result["match_count"]) + } + first, ok := result["first_match"].(map[string]string) + if !ok || first["name"] != "United States" { + t.Errorf("first_match = %+v, want the US row", result["first_match"]) + } + if result["list_id"] != "x" { + t.Errorf("list_id = %v, want %q (the execution evidence bar, goal 0011 item 5)", result["list_id"], "x") + } +} + +func TestListSearch_ExactMatch_Miss(t *testing.T) { + withTestList(t, seedRows) + result := execListSearchDirect(t, + []listSearchMatchParam{{Column: "code", Value: "attr:code", MatchType: "exact"}}, + nil, map[string]string{"code": "ZZ"}) + + if matched, _ := result["matched"].(bool); matched { + t.Fatalf("result = %+v, want matched=false", result) + } + if count, _ := result["match_count"].(int); count != 0 { + t.Errorf("match_count = %v, want 0", result["match_count"]) + } + if result["first_match"] != nil { + t.Errorf("first_match = %v, want nil on a miss", result["first_match"]) + } +} + +func TestListSearch_FuzzyMatch(t *testing.T) { + withTestList(t, seedRows) + result := execListSearchDirect(t, + []listSearchMatchParam{{Column: "name", Value: "Fracne", MatchType: "fuzzy", Threshold: 0.6}}, + nil, nil) + + if matched, _ := result["matched"].(bool); !matched { + t.Fatalf("fuzzy result = %+v, want a match against 'France' for the typo'd query 'Fracne'", result) + } + first, _ := result["first_match"].(map[string]string) + if first["name"] != "France" { + t.Errorf("first_match = %+v, want France", result["first_match"]) + } +} + +func TestListSearch_FuzzyMatch_BelowThreshold_NoMatch(t *testing.T) { + withTestList(t, seedRows) + result := execListSearchDirect(t, + []listSearchMatchParam{{Column: "name", Value: "Zzzzzzzzz", MatchType: "fuzzy", Threshold: 0.9}}, + nil, nil) + + if matched, _ := result["matched"].(bool); matched { + t.Fatalf("result = %+v, want no match for a dissimilar query at a high threshold", result) + } +} + +func TestListSearch_ExpiredRow_ExcludedByDefault(t *testing.T) { + withTestList(t, seedRows) + result := execListSearchDirect(t, + []listSearchMatchParam{{Column: "code", Value: "SU", MatchType: "exact"}}, + nil, nil) + + if matched, _ := result["matched"].(bool); matched { + t.Fatalf("result = %+v, want the Expired SU row excluded by default", result) + } +} + +func TestListSearch_IncludeExpired_OptIn(t *testing.T) { + withTestList(t, seedRows) + result := execListSearchDirect(t, + []listSearchMatchParam{{Column: "code", Value: "SU", MatchType: "exact"}}, + map[string]string{"includeExpired": "true"}, nil) + + if matched, _ := result["matched"].(bool); !matched { + t.Fatalf("result = %+v, want the Expired SU row matched once includeExpired=true", result) + } +} + +func TestListSearch_MultipleParams_ANDSemantics(t *testing.T) { + withTestList(t, seedRows) + // code=US AND name=France should match nothing -- no single row + // satisfies both. + result := execListSearchDirect(t, []listSearchMatchParam{ + {Column: "code", Value: "US", MatchType: "exact"}, + {Column: "name", Value: "France", MatchType: "exact"}, + }, nil, nil) + if matched, _ := result["matched"].(bool); matched { + t.Fatalf("result = %+v, want AND'd params with no single satisfying row to not match", result) + } + + // code=US AND name=United States (the real pairing) should match. + result = execListSearchDirect(t, []listSearchMatchParam{ + {Column: "code", Value: "US", MatchType: "exact"}, + {Column: "name", Value: "United States", MatchType: "exact"}, + }, nil, nil) + if matched, _ := result["matched"].(bool); !matched { + t.Fatalf("result = %+v, want the real US/United States pairing to match", result) + } +} + +func TestListSearch_FirstMatchOnly_StopsEarly_SameShape(t *testing.T) { + rows := []list.Row{ + {ID: "r1", Values: map[string]string{"code": "US", "name": "United States"}, Status: list.RowActive}, + {ID: "r2", Values: map[string]string{"code": "US", "name": "USA (alias)"}, Status: list.RowActive}, + } + withTestList(t, rows) + + all := execListSearchDirect(t, + []listSearchMatchParam{{Column: "code", Value: "US", MatchType: "exact"}}, nil, nil) + if count, _ := all["match_count"].(int); count != 2 { + t.Fatalf("without firstMatchOnly, match_count = %v, want 2", all["match_count"]) + } + + first := execListSearchDirect(t, + []listSearchMatchParam{{Column: "code", Value: "US", MatchType: "exact"}}, + map[string]string{"firstMatchOnly": "true"}, nil) + if count, _ := first["match_count"].(int); count != 1 { + t.Fatalf("with firstMatchOnly, match_count = %v, want 1", first["match_count"]) + } + // The type never changes -- results/matched/first_match/match_count + // are all still present, just results has fewer entries (goal + // 0011's own "never changes the published type" requirement). + for _, key := range []string{"results", "matched", "first_match", "match_count", "list_id"} { + if _, ok := first[key]; !ok { + t.Errorf("firstMatchOnly result missing key %q -- the output shape must stay identical", key) + } + } +} + +func TestListSearch_NoMatchParams_Errors(t *testing.T) { + withTestList(t, seedRows) + _, err := execListSearch(Node{NodeTypeID: "list-search", Config: map[string]string{ + "listId": "x", "outputAttribute": "result", + }}, ExecContext{Attributes: map[string]any{}}) + if err == nil || !strings.Contains(err.Error(), "at least one match parameter") { + t.Fatalf("err = %v, want an 'at least one match parameter' error", err) + } +} + +func TestListSearch_NoOutputAttribute_Errors(t *testing.T) { + withTestList(t, seedRows) + raw, _ := json.Marshal([]listSearchMatchParam{{Column: "code", Value: "US", MatchType: "exact"}}) + _, err := execListSearch(Node{NodeTypeID: "list-search", Config: map[string]string{ + "listId": "x", "matchParams": string(raw), + }}, ExecContext{Attributes: map[string]any{}}) + if err == nil || !strings.Contains(err.Error(), "outputAttribute is required") { + t.Fatalf("err = %v, want an 'outputAttribute is required' error", err) + } +} diff --git a/internal/domain/composition/seedproof_test.go b/internal/domain/composition/seedproof_test.go index 0ff1c9c3..76899d2b 100644 --- a/internal/domain/composition/seedproof_test.go +++ b/internal/domain/composition/seedproof_test.go @@ -83,6 +83,12 @@ var workflowProofRegistry = map[string]seedProof{ "executionsvc.TestSeededCountryLookupExample_NoMatch_FailsClosed", "e2e: seed-completeness.spec.ts > Country code lookup", ), + "example-list-search-workflow": proven( + "executionsvc.TestSeededListSearchExample_Match_WritesTypedResult", + "executionsvc.TestSeededListSearchExample_NoMatch_WritesUnmatchedResult", + "executionsvc.TestSeededListSearchExample_ExpiredRow_ExcludedByDefault", + "e2e: seed-completeness.spec.ts > Example: Country lookup (search) runs a real exact match through list-search", + ), "example-mcp-echo-workflow": proven( "composition.TestSeededMCPExample_EchoToolCall_RunsEndToEnd", "e2e: seed-completeness.spec.ts > MCP echo call (presence/config only)", diff --git a/internal/domain/list/builtin.go b/internal/domain/list/builtin.go index 3e68fc94..7744f120 100644 --- a/internal/domain/list/builtin.go +++ b/internal/domain/list/builtin.go @@ -1,9 +1,16 @@ package list +import ( + "time" + + "github.com/alicoding/mill/internal/domain/typedfield" +) + // ExampleCountryCodesID is the seeded example List's ID -- exported so -// composition.BuiltInWorkflows' own list-lookup seed (nodetypes.go) -// can reference it without a string literal that could drift, same -// pattern httprequest.ExampleNoneID/decision.ExampleApproveID already +// composition.BuiltInWorkflows' own list-lookup/list-search seeds +// (nodetypes.go, builtinworkflows.go) can reference it without a +// string literal that could drift, same pattern +// httprequest.ExampleNoneID/decision.ExampleApproveID already // establish. const ExampleCountryCodesID = "example-country-codes-list" @@ -11,17 +18,52 @@ const ExampleCountryCodesID = "example-country-codes-list" // persistence (mirrors httprequest.BuiltIn/decision.BuiltIn's shape: // this package stays free of the settings-store concern, per // CLAUDE.md's backend rule -- ConfigureService owns seeding/top-up). -// docs/goals/0010 item 4: Lists had zero seeded example and zero -// seeded workflow exercising list-lookup before this. +// +// Grown from a plain key/value map (docs/goals/0010) to a typed +// "code"/"name" dataset by goal 0011 -- the seed proof for BOTH +// list-lookup (via DeriveEntries' first-two-columns reading, so the +// existing "Example: Country code lookup" workflow keeps working +// completely unchanged) and list-search (typed exact/fuzzy matching +// against real columns). Includes a deliberately Expired row (a +// defunct country code) so the seed itself demonstrates goal 0011's +// "Expired excluded from matching by default" rule live, not just in +// a unit test -- and a real near-miss ("France" vs a typo'd query) so +// fuzzy matching has something genuine to match against. func BuiltIn() []List { + now := time.Now() + activeRow := func(id, code, name string) Row { + return Row{ + ID: id, + Values: map[string]string{"code": code, "name": name}, + CreatedAt: now, + UpdatedAt: now, + Status: RowActive, + } + } + expiredRow := func(id, code, name string) Row { + r := activeRow(id, code, name) + r.Status = RowExpired + return r + } + return []List{ { ID: ExampleCountryCodesID, Label: "Example: Country codes", - Entries: map[string]string{ - "US": "United States", - "CA": "Canada", - "MX": "Mexico", + Description: "A typed lookup dataset (code -> country name) -- goal 0011's seeded proof for " + + "both list-lookup (legacy, via the derived key/value view over its first two columns) and " + + "list-search (typed exact/fuzzy matching). Includes one deliberately Expired row (a defunct " + + "code) demonstrating the exclude-by-default rule live.", + Columns: []typedfield.Field{ + {Key: "code", Label: "Code", Type: typedfield.TypeText, Required: true}, + {Key: "name", Label: "Name", Type: typedfield.TypeText, Required: true}, + }, + Rows: []Row{ + activeRow("row-us", "US", "United States"), + activeRow("row-ca", "CA", "Canada"), + activeRow("row-mx", "MX", "Mexico"), + activeRow("row-fr", "FR", "France"), + expiredRow("row-su", "SU", "Soviet Union"), }, BuiltIn: true, }, diff --git a/internal/domain/list/list.go b/internal/domain/list/list.go index 69ea95b3..c2895fa9 100644 --- a/internal/domain/list/list.go +++ b/internal/domain/list/list.go @@ -1,31 +1,96 @@ // Package list holds the core-domain shape of a List (docs/SPEC.md -// §3.5): a reusable (1:many), Configure-authored named lookup table -- -// the same reuse cardinality as a Connector, but with no external call -// or credential involved, just a static key/value mapping a workflow's -// list-lookup node reads from at run time. Per CLAUDE.md's core-domain -// rule, the shape and its validation stay hand-written -- no library has -// an opinion on Mill's own List model. +// §3.2.2/§3.5, docs/goals/0011-lists-maturation.md): a reusable +// (1:many), Configure-authored typed tabular dataset a workflow's +// list-search (or the legacy list-lookup) node reads from at run +// time. Per CLAUDE.md's core-domain rule, the shape and its +// validation stay hand-written -- no library has an opinion on +// Mill's own List model. +// +// Grown from a flat key/value map (Entries) to typed Columns + Row +// records by goal 0011, against ADR-0029's canonical typedfield.Field +// vocabulary (Columns reuse it directly -- never a fifth schema +// system) and a reference-platform review recorded in SPEC.md §3.2.2: +// typed columns, system-managed audit fields (CreatedAt/UpdatedAt/ +// Status) kept as platform-owned struct fields rather than +// user-declarable TypedField columns (the BuiltIn/Versions +// precedent), and Active/Expired row lifecycle with Expired excluded +// from matching by default (industry research recorded in the goal +// file: the soft-delete convention, OFAC sanctions screening, and +// Informatica MDM all exclude by default with a per-step opt-in -- +// uniform across exact and fuzzy matching, never split by match +// type). +// +// Actor identity (who created/updated a row) is deliberately NOT +// modeled: Mill is single-user forever (SPEC.md §3.7's own researched- +// and-declined multi-tenancy seam) -- a fixed "local" CreatedBy/ +// UpdatedBy constant would be config/schema surface for a decision +// with no real consumer today, the same premature-abstraction trap +// SPEC.md §3.5's Configure recheck already names for other fields. +// This is the goal file's own open call ("note which"), noted here. package list import ( "fmt" "strings" "time" + + "github.com/alicoding/mill/internal/domain/typedfield" +) + +// RowStatus is a Row's platform-owned lifecycle state -- never a +// TypedField/user-declared column (goal 0011's audit-column +// decision: system-managed fields are Go struct fields, not entries +// in List.Columns). +type RowStatus string + +const ( + RowActive RowStatus = "active" + RowExpired RowStatus = "expired" ) -// List is one reusable, named lookup table. Entries maps an input key -// (whatever a workflow's list-lookup node is configured to look up) to -// the value that gets written back into the workflow's Attributes. +// Row is one typed record in a List. Values maps a declared Column's +// Key to its string value -- the same "every value stays a plain +// string on the wire" discipline typedfield.Field itself documents; +// a Column's Type only governs validation/rendering/matching, never +// the wire shape. CreatedAt/UpdatedAt/Status are platform-owned audit +// fields, set by the owning service (internal/services/configuresvc), +// never a user-declared Column. +type Row struct { + ID string + Values map[string]string + CreatedAt time.Time + UpdatedAt time.Time + Status RowStatus +} + +// List is one reusable, named typed dataset. Columns declares its +// schema (typedfield.Field, ADR-0029); Rows carries its data. +// +// Entries is the PRE-0011 flat key/value shape, kept on the struct +// for wire/backward compatibility only -- MigrateLegacyEntries +// (migrate.go) converts any list still carrying it (Columns empty, +// Entries non-empty) into the typed Columns+Rows shape the first time +// it's loaded (internal/services/configuresvc's restore()), so +// list-search/list-lookup execution logic only ever deals with +// Columns+Rows, never a third code path for the legacy shape. A list +// persisted before this goal, loaded once, re-persists in the typed +// shape; Entries is never populated by any code path after that first +// load -- new lists never populate it at all. DeriveEntries below is +// the read-side mirror: list-lookup's own execution keeps reading a +// flat map, computed from any 2+-column typed list's first two +// columns, so it never needed to change at all. type List struct { - ID string - Label string - Entries map[string]string - // BuiltIn marks a seeded example list (BuiltIn() below) -- purely - // informational, same as httprequest.HTTPRequest.BuiltIn/ - // decision.Decision.BuiltIn: drives a "built-in" badge only, never - // gates Edit/Delete. A seeded example is an ordinary, fully- - // editable/deletable list from the moment it exists (docs/SPEC.md - // §2.2's Update note). + ID string + Label string + Description string + Columns []typedfield.Field + Rows []Row + Entries map[string]string + // BuiltIn marks a seeded example list -- purely informational, + // same as httprequest.HTTPRequest.BuiltIn/decision.Decision.BuiltIn: + // drives a "built-in" badge only, never gates Edit/Delete. A seeded + // example is an ordinary, fully-editable list from the moment it + // exists (docs/SPEC.md §2.2's Update note). BuiltIn bool // CreatedAt/UpdatedAt are system-managed audit timestamps (SPEC.md // §3.2.2's reserved-column pattern), stamped server-side at every @@ -36,14 +101,58 @@ type List struct { } // Validate checks a List is well-formed before it's persisted -- same -// "never store an unconfigured/invalid value" discipline -// internal/domain/composition's ResolveNodeDefaults and -// internal/domain/connector's Validate already apply to their own types. -// An empty Entries map is valid (a list starts empty and gets rows added -// later); a missing Label is not. +// "never store an unconfigured/invalid value" discipline every other +// domain package's own Validate already applies. func Validate(l List) error { if strings.TrimSpace(l.Label) == "" { return fmt.Errorf("a list needs a label") } + seen := make(map[string]bool, len(l.Columns)) + for _, c := range l.Columns { + if err := typedfield.Validate(c); err != nil { + return fmt.Errorf("column: %w", err) + } + if seen[c.Key] { + return fmt.Errorf("duplicate column key %q", c.Key) + } + seen[c.Key] = true + } + for _, r := range l.Rows { + if strings.TrimSpace(r.ID) == "" { + return fmt.Errorf("a row needs a non-empty id") + } + } return nil } + +// DeriveEntries computes a legacy flat key/value view over a typed +// List's first two Columns (Columns[0] as key, Columns[1] as value) +// -- how list-lookup (internal/domain/composition/listlookup.go) +// keeps working completely unchanged against a typed list, whether +// it's a migrated legacy list (whose synthesized Columns are +// literally "key"/"value") or a genuinely typed one (e.g. the seeded +// "code"/"name" country-codes list): list-lookup only ever needed a +// flat map, and any 2+-column list has an unambiguous "first two +// columns" reading, so no per-list configuration is needed to keep it +// working. Expired rows are excluded (goal 0011's uniform default); +// list-lookup's own config has no field to opt back in, so this +// derived view is Active-only, full stop. +// +// Returns nil for a list with fewer than 2 columns -- nothing to +// derive a key/value pair from (a real gap for list-lookup against +// e.g. a single-column list, but list-lookup already errored on an +// empty Entries map before this goal too, so nothing regresses). +func DeriveEntries(l List) map[string]string { + if len(l.Columns) < 2 { + return nil + } + keyCol, valCol := l.Columns[0].Key, l.Columns[1].Key + out := make(map[string]string, len(l.Rows)) + for _, r := range l.Rows { + if r.Status == RowExpired { + continue + } + out[r.Values[keyCol]] = r.Values[valCol] + } + return out +} diff --git a/internal/domain/list/list_test.go b/internal/domain/list/list_test.go index 770b99c0..227910e6 100644 --- a/internal/domain/list/list_test.go +++ b/internal/domain/list/list_test.go @@ -1,6 +1,10 @@ package list -import "testing" +import ( + "testing" + + "github.com/alicoding/mill/internal/domain/typedfield" +) func TestValidate_Accepts(t *testing.T) { l := List{ID: "l1", Label: "Region codes", Entries: map[string]string{"US": "United States"}} @@ -22,3 +26,117 @@ func TestValidate_EmptyLabel_Rejected(t *testing.T) { t.Error("Validate with an empty label returned nil error, want an error") } } + +func TestValidate_TypedColumns_Accepted(t *testing.T) { + l := List{ + ID: "l1", Label: "Typed list", + Columns: []typedfield.Field{ + {Key: "code", Label: "Code", Type: typedfield.TypeText}, + {Key: "name", Label: "Name", Type: typedfield.TypeText}, + }, + Rows: []Row{ + {ID: "r1", Values: map[string]string{"code": "US", "name": "United States"}, Status: RowActive}, + }, + } + if err := Validate(l); err != nil { + t.Errorf("Validate(typed list) returned error: %v", err) + } +} + +func TestValidate_DuplicateColumnKey_Rejected(t *testing.T) { + l := List{ + ID: "l1", Label: "Typed list", + Columns: []typedfield.Field{ + {Key: "code", Label: "Code", Type: typedfield.TypeText}, + {Key: "code", Label: "Code again", Type: typedfield.TypeText}, + }, + } + if err := Validate(l); err == nil { + t.Error("Validate with a duplicate column key returned nil error, want an error") + } +} + +func TestValidate_InvalidColumnType_Rejected(t *testing.T) { + l := List{ + ID: "l1", Label: "Typed list", + Columns: []typedfield.Field{{Key: "code", Label: "Code", Type: "not-a-real-type"}}, + } + if err := Validate(l); err == nil { + t.Error("Validate with an invalid column type returned nil error, want an error") + } +} + +func TestValidate_RowWithEmptyID_Rejected(t *testing.T) { + l := List{ + ID: "l1", Label: "Typed list", + Rows: []Row{{ID: " ", Values: map[string]string{}}}, + } + if err := Validate(l); err == nil { + t.Error("Validate with a blank row id returned nil error, want an error") + } +} + +func TestDeriveEntries_TwoColumns_ExcludesExpired(t *testing.T) { + l := List{ + Columns: []typedfield.Field{ + {Key: "code", Label: "Code", Type: typedfield.TypeText}, + {Key: "name", Label: "Name", Type: typedfield.TypeText}, + }, + Rows: []Row{ + {ID: "r1", Values: map[string]string{"code": "US", "name": "United States"}, Status: RowActive}, + {ID: "r2", Values: map[string]string{"code": "SU", "name": "Soviet Union"}, Status: RowExpired}, + }, + } + got := DeriveEntries(l) + if len(got) != 1 || got["US"] != "United States" { + t.Errorf("DeriveEntries = %+v, want only the Active row's US -> United States", got) + } + if _, ok := got["SU"]; ok { + t.Error("DeriveEntries included an Expired row's entry, want it excluded by default") + } +} + +func TestDeriveEntries_FewerThanTwoColumns_ReturnsNil(t *testing.T) { + l := List{Columns: []typedfield.Field{{Key: "code", Label: "Code", Type: typedfield.TypeText}}} + if got := DeriveEntries(l); got != nil { + t.Errorf("DeriveEntries(1 column) = %+v, want nil", got) + } +} + +func TestMigrateLegacyEntries_DeterministicSortedByKey(t *testing.T) { + entries := map[string]string{"US": "United States", "CA": "Canada", "MX": "Mexico"} + i := 0 + ids := []string{"row-a", "row-b", "row-c"} + newID := func() string { id := ids[i]; i++; return id } + + columns, rows := MigrateLegacyEntries(entries, newID) + if len(columns) != 2 || columns[0].Key != "key" || columns[1].Key != "value" { + t.Fatalf("MigrateLegacyEntries columns = %+v, want [key, value]", columns) + } + if len(rows) != 3 { + t.Fatalf("MigrateLegacyEntries rows = %d, want 3", len(rows)) + } + // Sorted by key: CA, MX, US. + wantKeys := []string{"CA", "MX", "US"} + for i, r := range rows { + if r.Values["key"] != wantKeys[i] { + t.Errorf("row %d key = %q, want %q (deterministic sorted order)", i, r.Values["key"], wantKeys[i]) + } + if r.Status != RowActive { + t.Errorf("row %d status = %q, want Active", i, r.Status) + } + if r.ID != ids[i] { + t.Errorf("row %d id = %q, want %q (from the injected newRowID)", i, r.ID, ids[i]) + } + if r.CreatedAt.IsZero() || r.UpdatedAt.IsZero() { + t.Errorf("row %d CreatedAt/UpdatedAt is zero, want stamped at migration time", i) + } + } +} + +func TestMigrateLegacyEntries_Empty_ReturnsNil(t *testing.T) { + columns, rows := MigrateLegacyEntries(nil, func() string { return "x" }) + if columns != nil || rows != nil { + t.Errorf("MigrateLegacyEntries(nil) = %+v, %+v, want nil, nil", columns, rows) + } +} diff --git a/internal/domain/list/migrate.go b/internal/domain/list/migrate.go new file mode 100644 index 00000000..f3ac01c0 --- /dev/null +++ b/internal/domain/list/migrate.go @@ -0,0 +1,58 @@ +package list + +import ( + "sort" + "time" + + "github.com/alicoding/mill/internal/domain/typedfield" +) + +// MigrateLegacyEntries converts a pre-0011 flat key/value List +// (Entries populated, Columns/Rows empty) into the typed Columns+Rows +// shape -- goal 0011's decided backward-compat approach ("a migration +// (single key/value columns)"). Synthesizes two text Columns, "key" +// and "value", and one Row per entry, sorted by key for a +// deterministic, reviewable result (Go map iteration order is +// otherwise random, which would make a migrated list's row order +// change on every restart for no reason). Every migrated row starts +// Active with CreatedAt/UpdatedAt stamped at migration time -- the +// legacy Entries map carries no real creation timestamp to preserve, +// and Mill has no actor identity to attribute rows to (see list.go's +// own doc comment on CreatedBy/UpdatedBy). +// +// newRowID is injected rather than generated here: row-ID minting +// stays a service-layer concern (internal/services/configuresvc, +// which already mints List IDs themselves via seeding.NewSlugID) -- +// this package stays pure per .claude/rules/backend.md's domain- +// purity rule, with no ID-generation policy of its own. +// +// Called once per list, either from ConfigureService's restore() (a +// real machine's already-persisted data) or ImportList (an old +// exported-list JSON document still carrying the legacy shape). +func MigrateLegacyEntries(entries map[string]string, newRowID func() string) ([]typedfield.Field, []Row) { + if len(entries) == 0 { + return nil, nil + } + columns := []typedfield.Field{ + {Key: "key", Label: "Key", Type: typedfield.TypeText}, + {Key: "value", Label: "Value", Type: typedfield.TypeText}, + } + keys := make([]string, 0, len(entries)) + for k := range entries { + keys = append(keys, k) + } + sort.Strings(keys) + + now := time.Now() + rows := make([]Row, 0, len(keys)) + for _, k := range keys { + rows = append(rows, Row{ + ID: newRowID(), + Values: map[string]string{"key": k, "value": entries[k]}, + CreatedAt: now, + UpdatedAt: now, + Status: RowActive, + }) + } + return columns, rows +} diff --git a/internal/services/configuresvc/configureservice.go b/internal/services/configuresvc/configureservice.go index 3eb569b3..95bf031e 100644 --- a/internal/services/configuresvc/configureservice.go +++ b/internal/services/configuresvc/configureservice.go @@ -16,6 +16,7 @@ import ( "github.com/alicoding/mill/internal/domain/httprequest" "github.com/alicoding/mill/internal/domain/list" "github.com/alicoding/mill/internal/domain/mcpserver" + "github.com/alicoding/mill/internal/domain/typedfield" "github.com/alicoding/mill/internal/services/compositionsvc" "github.com/alicoding/mill/internal/services/seeding" ) @@ -94,13 +95,21 @@ func NewConfigureService(store settings.Store, comp *compositionsvc.CompositionS return c } -// resolveList implements composition.go's lookupListFn seam. +// resolveList implements composition.go's lookupListFn seam. Entries +// is list.DeriveEntries's own computed key/value view (goal 0011) -- +// list-lookup keeps reading a flat map with zero changes to its own +// execution logic; Columns/Rows are the typed additions list-search +// reads. func (c *ConfigureService) resolveList(id string) (composition.ResolvedList, error) { c.mu.Lock() defer c.mu.Unlock() for _, l := range c.lists { if l.ID == id { - return composition.ResolvedList{Entries: l.Entries}, nil + return composition.ResolvedList{ + Entries: list.DeriveEntries(l), + Columns: l.Columns, + Rows: l.Rows, + }, nil } } return composition.ResolvedList{}, fmt.Errorf("no list with id %q", id) @@ -116,9 +125,44 @@ func (c *ConfigureService) Lists() []list.List { return out } -func (c *ConfigureService) CreateList(label string, entries map[string]string) (list.List, error) { +// findListLocked returns the index of the list with id in c.lists, or +// -1 -- caller must hold c.mu. +func (c *ConfigureService) findListLocked(id string) int { + for i, l := range c.lists { + if l.ID == id { + return i + } + } + return -1 +} + +// removeListByIDLocked removes the list with id from c.lists, if +// present -- caller must hold c.mu. +func (c *ConfigureService) removeListByIDLocked(id string) { + if idx := c.findListLocked(id); idx != -1 { + c.lists = append(c.lists[:idx], c.lists[idx+1:]...) + } +} + +// revertListLocked restores previous (keyed by its own ID) into +// c.lists -- caller must hold c.mu. Shared undo path for every List/ +// row mutation below's persist-failure revert (docs/goals/0025 item +// 2's memory-vs-store rule, extended here from CreateList/UpdateList +// to the new row-level mutations for the same reason: a phantom +// in-memory row a restart would silently drop is exactly as wrong as +// a phantom list). +func (c *ConfigureService) revertListLocked(previous list.List) { + if idx := c.findListLocked(previous.ID); idx != -1 { + c.lists[idx] = previous + } +} + +func (c *ConfigureService) CreateList(label, description string, columns []typedfield.Field) (list.List, error) { now := time.Now() - l := list.List{ID: seeding.NewSlugID(label, "list"), Label: label, Entries: entries, CreatedAt: now, UpdatedAt: now} + l := list.List{ + ID: seeding.NewSlugID(label, "list"), Label: label, Description: description, + Columns: columns, CreatedAt: now, UpdatedAt: now, + } if err := list.Validate(l); err != nil { return list.List{}, err } @@ -138,54 +182,149 @@ func (c *ConfigureService) CreateList(label string, entries map[string]string) ( return l, nil } -// removeListByIDLocked removes the list with id from c.lists, if -// present -- caller must hold c.mu. -func (c *ConfigureService) removeListByIDLocked(id string) { - for i, l := range c.lists { - if l.ID == id { - c.lists = append(c.lists[:i], c.lists[i+1:]...) - return - } +func (c *ConfigureService) UpdateList(id, label, description string, columns []typedfield.Field) (list.List, error) { + c.mu.Lock() + idx := c.findListLocked(id) + if idx == -1 { + c.mu.Unlock() + return list.List{}, fmt.Errorf("no list with id %q", id) } + previous := c.lists[idx] + l := previous + // CreatedAt is preserved from the stored entity, never trusted from + // the wire; UpdatedAt always advances on a real update. + l.Label, l.Description, l.Columns = label, description, columns + l.UpdatedAt = time.Now() + if err := list.Validate(l); err != nil { + c.mu.Unlock() + return list.List{}, err + } + c.lists[idx] = l + c.mu.Unlock() + + if err := c.persistLists(); err != nil { + c.mu.Lock() + c.revertListLocked(previous) + c.mu.Unlock() + return list.List{}, fmt.Errorf("save list: %w", err) + } + return l, nil } -func (c *ConfigureService) UpdateList(id, label string, entries map[string]string) (list.List, error) { - l := list.List{ID: id, Label: label, Entries: entries} +// AddListRow appends a new, Active row to a List, minting its ID here +// (row-ID generation stays a service-layer concern, same as List IDs +// themselves via seeding.NewSlugID -- internal/domain/list stays pure +// per .claude/rules/backend.md). +func (c *ConfigureService) AddListRow(listID string, values map[string]string) (list.List, error) { + c.mu.Lock() + idx := c.findListLocked(listID) + if idx == -1 { + c.mu.Unlock() + return list.List{}, fmt.Errorf("no list with id %q", listID) + } + previous := c.lists[idx] + now := time.Now() + row := list.Row{ + ID: seeding.NewSlugID("", "row"), Values: values, + CreatedAt: now, UpdatedAt: now, Status: list.RowActive, + } + l := previous + l.Rows = append(append([]list.Row{}, l.Rows...), row) + l.UpdatedAt = now if err := list.Validate(l); err != nil { + c.mu.Unlock() return list.List{}, err } + c.lists[idx] = l + c.mu.Unlock() + if err := c.persistLists(); err != nil { + c.mu.Lock() + c.revertListLocked(previous) + c.mu.Unlock() + return list.List{}, fmt.Errorf("save list: %w", err) + } + return l, nil +} + +// UpdateListRow replaces one row's Values/Status (its ID/CreatedAt +// stay put; UpdatedAt is stamped here, not client-supplied). +func (c *ConfigureService) UpdateListRow(listID, rowID string, values map[string]string, status list.RowStatus) (list.List, error) { c.mu.Lock() - idx := -1 - for i, existing := range c.lists { - if existing.ID == id { - idx = i + idx := c.findListLocked(listID) + if idx == -1 { + c.mu.Unlock() + return list.List{}, fmt.Errorf("no list with id %q", listID) + } + previous := c.lists[idx] + l := previous + rowIdx := -1 + for i, r := range l.Rows { + if r.ID == rowID { + rowIdx = i break } } + if rowIdx == -1 { + c.mu.Unlock() + return list.List{}, fmt.Errorf("no row with id %q in list %q", rowID, listID) + } + if status == "" { + status = list.RowActive + } + now := time.Now() + rows := append([]list.Row{}, l.Rows...) + rows[rowIdx].Values = values + rows[rowIdx].Status = status + rows[rowIdx].UpdatedAt = now + l.Rows = rows + l.UpdatedAt = now + if err := list.Validate(l); err != nil { + c.mu.Unlock() + return list.List{}, err + } + c.lists[idx] = l + c.mu.Unlock() + + if err := c.persistLists(); err != nil { + c.mu.Lock() + c.revertListLocked(previous) + c.mu.Unlock() + return list.List{}, fmt.Errorf("save list: %w", err) + } + return l, nil +} + +func (c *ConfigureService) DeleteListRow(listID, rowID string) (list.List, error) { + c.mu.Lock() + idx := c.findListLocked(listID) if idx == -1 { c.mu.Unlock() - return list.List{}, fmt.Errorf("no list with id %q", id) + return list.List{}, fmt.Errorf("no list with id %q", listID) } - // CreatedAt is preserved from the stored entity, never trusted from - // the wire (the caller-supplied l above never set it); UpdatedAt - // always advances on a real update. BuiltIn's own pre-existing - // reset-on-update behavior is left exactly as it was -- out of this - // change's scope. - l.CreatedAt = c.lists[idx].CreatedAt - l.UpdatedAt = time.Now() previous := c.lists[idx] + l := previous + rows := make([]list.Row, 0, len(l.Rows)) + found := false + for _, r := range l.Rows { + if r.ID == rowID { + found = true + continue + } + rows = append(rows, r) + } + if !found { + c.mu.Unlock() + return list.List{}, fmt.Errorf("no row with id %q in list %q", rowID, listID) + } + l.Rows = rows + l.UpdatedAt = time.Now() c.lists[idx] = l c.mu.Unlock() if err := c.persistLists(); err != nil { c.mu.Lock() - for i, existing := range c.lists { - if existing.ID == id { - c.lists[i] = previous - break - } - } + c.revertListLocked(previous) c.mu.Unlock() return list.List{}, fmt.Errorf("save list: %w", err) } @@ -317,6 +456,40 @@ func (c *ConfigureService) restore() { var lists []list.List if err := json.Unmarshal([]byte(raw), &lists); err == nil { c.lists = lists + c.migrateLegacyLists() + } + } +} + +// migrateLegacyLists converts any pre-0011 flat key/value List +// (Columns/Rows empty, Entries populated) into the typed shape once, +// in place, and re-persists -- goal 0011's decided backward-compat +// approach (list.MigrateLegacyEntries's own doc comment has the full +// reasoning). A list that's already typed, or one that's genuinely +// empty, is left untouched. Runs on every restore() call, but the +// migration is idempotent (a list only ever qualifies once -- after +// migrating, it carries Columns, so the condition never fires again), +// so persisting again on a later restart is a cheap no-op. +func (c *ConfigureService) migrateLegacyLists() { + changed := false + for i, l := range c.lists { + if len(l.Columns) > 0 || len(l.Entries) == 0 { + continue + } + columns, rows := list.MigrateLegacyEntries(l.Entries, func() string { return seeding.NewSlugID("", "row") }) + c.lists[i].Columns = columns + c.lists[i].Rows = rows + changed = true + } + if changed { + // Startup migration, not a user-initiated RPC -- nothing to + // return the error to (this runs from restore()). Logged so a + // failure is diagnosable rather than silently dropped + // (docs/goals/0025 item 1's fire-and-forget bucket); worst + // case the migration simply re-runs identically on the next + // launch, since Entries itself is untouched by this function. + if err := c.persistLists(); err != nil { + slog.Error("failed to persist migrated legacy lists", "error", err) } } } diff --git a/internal/services/configuresvc/configureservice_export.go b/internal/services/configuresvc/configureservice_export.go index 35fb4c4d..f5bea6ee 100644 --- a/internal/services/configuresvc/configureservice_export.go +++ b/internal/services/configuresvc/configureservice_export.go @@ -8,6 +8,8 @@ import ( "github.com/alicoding/mill/internal/domain/httprequest" "github.com/alicoding/mill/internal/domain/list" "github.com/alicoding/mill/internal/domain/mcpserver" + "github.com/alicoding/mill/internal/domain/typedfield" + "github.com/alicoding/mill/internal/services/seeding" ) // This file extends compositionservice_export.go's workflow export/ @@ -96,9 +98,19 @@ func (c *ConfigureService) ImportHTTPRequest(jsonData string) (httprequest.HTTPR // --- List --- +// exportedList carries the typed shape (Columns/Rows, goal 0011) on +// export, always. Entries stays accepted on IMPORT ONLY, for an old +// export document written before this goal existed -- ImportList +// below runs it through the exact same list.MigrateLegacyEntries a +// real machine's persisted data goes through (configureservice.go's +// migrateLegacyLists), so there's still only one migration code path, +// not two. type exportedList struct { - Label string `json:"label"` - Entries map[string]string `json:"entries"` + Label string `json:"label"` + Description string `json:"description,omitempty"` + Columns []typedfield.Field `json:"columns,omitempty"` + Rows []list.Row `json:"rows,omitempty"` + Entries map[string]string `json:"entries,omitempty"` } func (c *ConfigureService) ExportList(id string) (string, error) { @@ -117,7 +129,9 @@ func (c *ConfigureService) ExportList(id string) (string, error) { return "", fmt.Errorf("no list with id %q", id) } - data, err := json.MarshalIndent(exportedList{Label: l.Label, Entries: l.Entries}, "", " ") + data, err := json.MarshalIndent(exportedList{ + Label: l.Label, Description: l.Description, Columns: l.Columns, Rows: l.Rows, + }, "", " ") if err != nil { return "", fmt.Errorf("export list: %w", err) } @@ -129,7 +143,42 @@ func (c *ConfigureService) ImportList(jsonData string) (list.List, error) { if err := json.Unmarshal([]byte(jsonData), &in); err != nil { return list.List{}, fmt.Errorf("import list: invalid JSON: %w", err) } - return c.CreateList(in.Label, in.Entries) + columns, rows := in.Columns, in.Rows + if len(columns) == 0 && len(in.Entries) > 0 { + columns, rows = list.MigrateLegacyEntries(in.Entries, func() string { return seeding.NewSlugID("", "row") }) + } + + created, err := c.CreateList(in.Label, in.Description, columns) + if err != nil { + return list.List{}, err + } + if len(rows) == 0 { + return created, nil + } + + c.mu.Lock() + idx := c.findListLocked(created.ID) + if idx == -1 { + c.mu.Unlock() + return list.List{}, fmt.Errorf("import list: created list %q vanished", created.ID) + } + previous := c.lists[idx] + c.lists[idx].Rows = rows + updated := c.lists[idx] + c.mu.Unlock() + + if err := c.persistLists(); err != nil { + // Don't leave imported rows sitting in memory only + // (docs/goals/0025 item 2's memory-vs-store rule) -- the + // created list itself (empty rows) is already durably + // persisted via CreateList above, so reverting to it here is + // exact, not approximate. + c.mu.Lock() + c.revertListLocked(previous) + c.mu.Unlock() + return list.List{}, fmt.Errorf("import list: save rows: %w", err) + } + return updated, nil } // --- MCPServer --- diff --git a/internal/services/configuresvc/configureservice_export_test.go b/internal/services/configuresvc/configureservice_export_test.go index a5be1530..3eff215e 100644 --- a/internal/services/configuresvc/configureservice_export_test.go +++ b/internal/services/configuresvc/configureservice_export_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/alicoding/mill/internal/domain/httprequest" + "github.com/alicoding/mill/internal/domain/typedfield" ) func TestExportImportHTTPRequest_RoundTrips_NeverCarriesASecret(t *testing.T) { @@ -56,12 +57,23 @@ func TestExportImportHTTPRequest_RoundTrips_NeverCarriesASecret(t *testing.T) { } } +func testListColumns() []typedfield.Field { + return []typedfield.Field{ + {Key: "a", Label: "A", Type: typedfield.TypeText}, + {Key: "b", Label: "B", Type: typedfield.TypeText}, + } +} + func TestExportImportList_RoundTrips(t *testing.T) { cfg, _ := newTestConfigureService(t) - created, err := cfg.CreateList("My list", map[string]string{"a": "1", "b": "2"}) + created, err := cfg.CreateList("My list", "a list", testListColumns()) if err != nil { t.Fatalf("CreateList: %v", err) } + created, err = cfg.AddListRow(created.ID, map[string]string{"a": "1", "b": "2"}) + if err != nil { + t.Fatalf("AddListRow: %v", err) + } exported, err := cfg.ExportList(created.ID) if err != nil { @@ -75,17 +87,44 @@ func TestExportImportList_RoundTrips(t *testing.T) { if imported.ID == created.ID { t.Error("ImportList reused the original ID -- should always mint a new one") } - if imported.Label != created.Label { - t.Errorf("imported.Label = %q, want %q", imported.Label, created.Label) + if imported.Label != created.Label || imported.Description != created.Description { + t.Errorf("imported = %+v, want matching Label/Description from %+v", imported, created) + } + if len(imported.Columns) != 2 { + t.Errorf("imported.Columns = %+v, want 2 columns", imported.Columns) + } + if len(imported.Rows) != 1 || imported.Rows[0].Values["a"] != "1" || imported.Rows[0].Values["b"] != "2" { + t.Errorf("imported.Rows = %+v, want a copy of the one created row", imported.Rows) + } +} + +func TestImportList_LegacyEntriesShape_Migrates(t *testing.T) { + cfg, _ := newTestConfigureService(t) + // An old export document written before goal 0011 -- no + // columns/rows, just the flat key/value shape. + legacy := `{"label":"Old list","entries":{"US":"United States","CA":"Canada"}}` + imported, err := cfg.ImportList(legacy) + if err != nil { + t.Fatalf("ImportList(legacy shape): %v", err) + } + if len(imported.Columns) != 2 || imported.Columns[0].Key != "key" || imported.Columns[1].Key != "value" { + t.Fatalf("imported.Columns = %+v, want synthesized [key, value]", imported.Columns) + } + if len(imported.Rows) != 2 { + t.Fatalf("imported.Rows = %+v, want 2 rows", imported.Rows) + } + entries := map[string]string{} + for _, r := range imported.Rows { + entries[r.Values["key"]] = r.Values["value"] } - if len(imported.Entries) != 2 || imported.Entries["a"] != "1" || imported.Entries["b"] != "2" { - t.Errorf("imported.Entries = %+v, want a copy of %+v", imported.Entries, created.Entries) + if entries["US"] != "United States" || entries["CA"] != "Canada" { + t.Errorf("imported entries = %+v, want the legacy key/value pairs preserved", entries) } } func TestExportList_IsDeterministic(t *testing.T) { cfg, _ := newTestConfigureService(t) - created, err := cfg.CreateList("My list", map[string]string{"a": "1", "b": "2", "c": "3"}) + created, err := cfg.CreateList("My list", "", testListColumns()) if err != nil { t.Fatalf("CreateList: %v", err) } diff --git a/internal/services/configuresvc/configureservice_test.go b/internal/services/configuresvc/configureservice_test.go index d3f0dff1..27cae7cd 100644 --- a/internal/services/configuresvc/configureservice_test.go +++ b/internal/services/configuresvc/configureservice_test.go @@ -7,6 +7,8 @@ import ( "github.com/alicoding/mill/internal/adapters/credential" "github.com/alicoding/mill/internal/domain/composition" "github.com/alicoding/mill/internal/domain/httprequest" + "github.com/alicoding/mill/internal/domain/list" + "github.com/alicoding/mill/internal/domain/typedfield" "github.com/alicoding/mill/internal/services/compositionsvc" "github.com/alicoding/mill/internal/services/servicetest" "github.com/zalando/go-keyring" @@ -85,9 +87,16 @@ func TestRestore_MigratesLegacyConnectorsKey(t *testing.T) { } } +func regionCodeColumns() []typedfield.Field { + return []typedfield.Field{ + {Key: "code", Label: "Code", Type: typedfield.TypeText}, + {Key: "name", Label: "Name", Type: typedfield.TypeText}, + } +} + func TestCreateList_ValidatesAndPersists(t *testing.T) { cfg, _ := newTestConfigureService(t) - l, err := cfg.CreateList("Region codes", map[string]string{"US": "United States"}) + l, err := cfg.CreateList("Region codes", "", regionCodeColumns()) if err != nil { t.Fatalf("CreateList returned error: %v", err) } @@ -99,14 +108,14 @@ func TestCreateList_ValidatesAndPersists(t *testing.T) { func TestUpdateList_UnknownID_Rejected(t *testing.T) { cfg, _ := newTestConfigureService(t) - if _, err := cfg.UpdateList("does-not-exist", "New label", nil); err == nil { + if _, err := cfg.UpdateList("does-not-exist", "New label", "", nil); err == nil { t.Fatal("UpdateList with an unknown id returned nil error, want an error") } } func TestDeleteList_RemovesIt(t *testing.T) { cfg, _ := newTestConfigureService(t) - l, err := cfg.CreateList("Region codes", nil) + l, err := cfg.CreateList("Region codes", "", nil) if err != nil { t.Fatalf("CreateList returned error: %v", err) } @@ -132,7 +141,7 @@ func TestCreateList_PersistFailure_ReturnsErrorAndDoesNotPhantomSave(t *testing. cfg.lists = nil store.SetErr = errFakeConfigurePersist - if _, err := cfg.CreateList("Should not stick", nil); err == nil { + if _, err := cfg.CreateList("Should not stick", "", nil); err == nil { t.Fatal("CreateList() with a failing store: want error, got nil") } @@ -148,7 +157,7 @@ func TestDeleteList_PersistFailure_ReturnsErrorAndRestoresIt(t *testing.T) { cfg := NewConfigureService(store, comp, credential.New()) cfg.lists = nil - l, err := cfg.CreateList("Should survive the failed delete", nil) + l, err := cfg.CreateList("Should survive the failed delete", "", nil) if err != nil { t.Fatalf("CreateList: %v", err) } @@ -167,10 +176,13 @@ func TestDeleteList_PersistFailure_ReturnsErrorAndRestoresIt(t *testing.T) { func TestResolveList_ReturnsEntries(t *testing.T) { cfg, _ := newTestConfigureService(t) - l, err := cfg.CreateList("Region codes", map[string]string{"US": "United States"}) + l, err := cfg.CreateList("Region codes", "", regionCodeColumns()) if err != nil { t.Fatalf("CreateList returned error: %v", err) } + if _, err := cfg.AddListRow(l.ID, map[string]string{"code": "US", "name": "United States"}); err != nil { + t.Fatalf("AddListRow returned error: %v", err) + } rl, err := cfg.resolveList(l.ID) if err != nil { t.Fatalf("resolveList returned error: %v", err) @@ -180,6 +192,39 @@ func TestResolveList_ReturnsEntries(t *testing.T) { } } +func TestAddListRow_UpdateListRow_DeleteListRow(t *testing.T) { + cfg, _ := newTestConfigureService(t) + l, err := cfg.CreateList("Region codes", "", regionCodeColumns()) + if err != nil { + t.Fatalf("CreateList returned error: %v", err) + } + + l, err = cfg.AddListRow(l.ID, map[string]string{"code": "US", "name": "United States"}) + if err != nil { + t.Fatalf("AddListRow returned error: %v", err) + } + if len(l.Rows) != 1 || l.Rows[0].Status != list.RowActive { + t.Fatalf("after AddListRow, Rows = %+v, want one Active row", l.Rows) + } + rowID := l.Rows[0].ID + + l, err = cfg.UpdateListRow(l.ID, rowID, map[string]string{"code": "US", "name": "USA"}, list.RowExpired) + if err != nil { + t.Fatalf("UpdateListRow returned error: %v", err) + } + if l.Rows[0].Values["name"] != "USA" || l.Rows[0].Status != list.RowExpired { + t.Fatalf("after UpdateListRow, row = %+v, want name=USA status=Expired", l.Rows[0]) + } + + l, err = cfg.DeleteListRow(l.ID, rowID) + if err != nil { + t.Fatalf("DeleteListRow returned error: %v", err) + } + if len(l.Rows) != 0 { + t.Errorf("after DeleteListRow, Rows = %+v, want empty", l.Rows) + } +} + func TestResolveList_UnknownID_Rejected(t *testing.T) { cfg, _ := newTestConfigureService(t) if _, err := cfg.resolveList("does-not-exist"); err == nil { diff --git a/internal/services/configuresvc/configureservice_timestamps_test.go b/internal/services/configuresvc/configureservice_timestamps_test.go index dfd5d216..b2c0ca4d 100644 --- a/internal/services/configuresvc/configureservice_timestamps_test.go +++ b/internal/services/configuresvc/configureservice_timestamps_test.go @@ -131,7 +131,7 @@ func TestCreateList_StampsBothTimestamps(t *testing.T) { cfg, _ := newTestConfigureService(t) before := time.Now() - l, err := cfg.CreateList("Region codes", map[string]string{"US": "United States"}) + l, err := cfg.CreateList("Region codes", "", regionCodeColumns()) after := time.Now() if err != nil { t.Fatalf("CreateList: %v", err) @@ -151,7 +151,7 @@ func TestCreateList_StampsBothTimestamps(t *testing.T) { func TestUpdateList_PreservesCreatedAt_AdvancesUpdatedAt(t *testing.T) { cfg, _ := newTestConfigureService(t) - l, err := cfg.CreateList("Region codes", map[string]string{"US": "United States"}) + l, err := cfg.CreateList("Region codes", "", regionCodeColumns()) if err != nil { t.Fatalf("CreateList: %v", err) } @@ -159,7 +159,7 @@ func TestUpdateList_PreservesCreatedAt_AdvancesUpdatedAt(t *testing.T) { time.Sleep(2 * time.Millisecond) - updated, err := cfg.UpdateList(l.ID, "Region codes (edited)", map[string]string{"US": "United States", "CA": "Canada"}) + updated, err := cfg.UpdateList(l.ID, "Region codes (edited)", "edited", regionCodeColumns()) if err != nil { t.Fatalf("UpdateList: %v", err) } diff --git a/internal/services/executionsvc/listlookup_seed_test.go b/internal/services/executionsvc/listlookup_seed_test.go index 22f36921..193e2121 100644 --- a/internal/services/executionsvc/listlookup_seed_test.go +++ b/internal/services/executionsvc/listlookup_seed_test.go @@ -44,7 +44,12 @@ func newListLookupSeedHarness(t *testing.T) (*ExecutionService, string) { composition.SetListLookup(func(id string) (composition.ResolvedList, error) { for _, l := range list.BuiltIn() { if l.ID == id { - return composition.ResolvedList{Entries: l.Entries}, nil + // list.DeriveEntries (goal 0011): the seeded List is + // now typed (code/name columns), same derived + // first-two-columns view ConfigureService.resolveList + // uses in production -- list-lookup's own execution + // logic needed zero changes. + return composition.ResolvedList{Entries: list.DeriveEntries(l), Columns: l.Columns, Rows: l.Rows}, nil } } return composition.ResolvedList{}, fmt.Errorf("no list with id %q", id) diff --git a/internal/services/executionsvc/listsearch_seed_test.go b/internal/services/executionsvc/listsearch_seed_test.go new file mode 100644 index 00000000..a8612902 --- /dev/null +++ b/internal/services/executionsvc/listsearch_seed_test.go @@ -0,0 +1,110 @@ +package executionsvc + +import ( + "fmt" + "path/filepath" + "testing" + "time" + + "github.com/alicoding/mill/internal/domain/composition" + "github.com/alicoding/mill/internal/domain/list" + "github.com/alicoding/mill/internal/services/compositionsvc" + "github.com/alicoding/mill/internal/services/guardrailsvc" + "github.com/alicoding/mill/internal/services/servicetest" +) + +// docs/goals/0011-lists-maturation.md item 4: list-search's own seeded +// proof, run against a real DBOS runtime -- the "Example: Country +// lookup (search)" workflow, exercising an exact match hit and a miss +// against the SAME typed "Example: Country codes" List +// listlookup_seed_test.go already proves list-lookup against. Same +// harness shape as that file (composition.SetListLookup wired to +// list.BuiltIn(), restored via t.Cleanup). + +func newListSearchSeedHarness(t *testing.T) (*ExecutionService, string) { + t.Helper() + store := servicetest.NewFakeStore() + comp := compositionsvc.NewCompositionService(store) + guard := guardrailsvc.NewGuardrailService(store, comp) + dbPath := filepath.Join(t.TempDir(), "exec.db") + exec, err := NewExecutionService("sqlite:"+dbPath, comp, guard) + if err != nil { + t.Fatalf("NewExecutionService: %v", err) + } + t.Cleanup(func() { _ = exec.Shutdown(2 * time.Second) }) + + composition.SetListLookup(func(id string) (composition.ResolvedList, error) { + for _, l := range list.BuiltIn() { + if l.ID == id { + return composition.ResolvedList{Entries: list.DeriveEntries(l), Columns: l.Columns, Rows: l.Rows}, nil + } + } + return composition.ResolvedList{}, fmt.Errorf("no list with id %q", id) + }) + t.Cleanup(func() { + composition.SetListLookup(func(listID string) (composition.ResolvedList, error) { + return composition.ResolvedList{}, fmt.Errorf("no list lookup registered (yet) for id %q", listID) + }) + }) + + wfID := findBuiltInWorkflowID(t, comp, "Example: Country lookup (search)") + return exec, wfID +} + +func TestSeededListSearchExample_Match_WritesTypedResult(t *testing.T) { + exec, wfID := newListSearchSeedHarness(t) + + summary, err := exec.RunWorkflow(wfID, RunKindTest, map[string]string{"code": "US"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + if summary.Status != "SUCCESS" { + t.Fatalf("RunWorkflow(code=US) status = %q, want SUCCESS -- error: %s", summary.Status, summary.Error) + } + // The seed ends AT list-search itself (no terminal apply step -- + // a real Linux-CI clipboard failure caught during goal 0011's own + // PR is why, see builtinworkflows_list.go), so the workflow's + // final Payload/Output is still whatever capture-attribute set it + // to; list-search's own typed result lives in the 'searchResult' + // Attribute, not the string Payload/Output, proven at the + // composition-unit-test layer (listsearch_test.go) rather than + // re-asserted here. + if summary.Output != "US" { + t.Errorf("RunWorkflow(code=US) output = %q, want %q", summary.Output, "US") + } +} + +func TestSeededListSearchExample_NoMatch_WritesUnmatchedResult(t *testing.T) { + exec, wfID := newListSearchSeedHarness(t) + + // Unlike list-lookup's onMiss="fail" default, list-search never + // fails the run on a miss -- it always writes a typed + // {matched:false, results:[], ...} object and lets the workflow + // continue (a Decision downstream would branch on it). + summary, err := exec.RunWorkflow(wfID, RunKindTest, map[string]string{"code": "ZZ"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + if summary.Status != "SUCCESS" { + t.Fatalf("RunWorkflow(code=ZZ, no match) status = %q, want SUCCESS (list-search never fails on a miss) -- error: %s", summary.Status, summary.Error) + } + if summary.Output != "ZZ" { + t.Errorf("RunWorkflow(code=ZZ) output = %q, want %q", summary.Output, "ZZ") + } +} + +func TestSeededListSearchExample_ExpiredRow_ExcludedByDefault(t *testing.T) { + exec, wfID := newListSearchSeedHarness(t) + + // SU (Soviet Union) is seeded as a deliberately Expired row + // (internal/domain/list.BuiltIn) -- an exact match against it + // should behave exactly like a miss under the seed's default + // includeExpired=false. + summary, err := exec.RunWorkflow(wfID, RunKindTest, map[string]string{"code": "SU"}) + if err != nil { + t.Fatalf("RunWorkflow: %v", err) + } + if summary.Status != "SUCCESS" { + t.Fatalf("RunWorkflow(code=SU, expired row) status = %q, want SUCCESS -- error: %s", summary.Status, summary.Error) + } +}