diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 261ce24..6ac5a7f 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -6,13 +6,13 @@ "url": "https://github.com/jjackson" }, "metadata": { - "version": "0.13.276" + "version": "0.13.277" }, "plugins": [ { "name": "ace", "source": "./", - "version": "0.13.276", + "version": "0.13.277", "description": "AI Connect Engine — orchestrates the CRISPR-Connect lifecycle from idea through app building, Connect setup, LLO management, and closeout" } ] diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index b6869bd..d58b3c6 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "ace", - "version": "0.13.276", + "version": "0.13.277", "description": "AI Connect Engine — orchestrates the CRISPR-Connect lifecycle from idea through app building, Connect setup, LLO management, and closeout", "author": { "name": "Jonathan Jackson", diff --git a/CHANGELOG.md b/CHANGELOG.md index f5053d9..49c8b83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,28 @@ All notable changes to the ACE plugin will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the plugin follows [semantic versioning](https://semver.org/spec/v2.0.0.html). +## 0.13.277 — 2026-05-18 + +**Mirror Vellum's slug/name separation in the Nova architect brief (follow-up to 0.13.274).** + +The 0.13.274 fix capped module/deliver-unit *name* length at 40 chars as a workaround for Connect's 50-char `LearnModule.slug` / `DeliverUnit.slug` columns. That treats the symptom — the actual cause is that Nova's default `` derivation conflates the Connect slug (which feeds those columns) with the human-readable display name (which can be long and rich). + +The HQ-side authoring source of truth — `dimagi/Vellum:src/commcareConnect.js`, the Vellum plugin that humans use in the form designer when the `COMMCARE_CONNECT` flag is on — keeps these as two SEPARATE fields. The form's `nodeID` is the slug (user types something short and code-like like `module_1` or `m6_sample_prep`); the `name` element is the display label (any length, any character set). The fixture at `tests/static/commcareConnect/learn_module.xml` confirms: `module 1…`. Vellum-authored apps don't hit the slug-length trap because humans naturally pick short identifiers. + +Nova's API already supports this split — `connect.learn_module.id` is an optional field on `update_form` — but ACE's brief template never told the architect to set it, so Nova defaulted to `module__` and inherited the column-overflow risk. + +Three brief-template additions to `pdd-to-{learn,deliver}-app/SKILL.md`: + +1. **New REQUIRED clause** — every `connect.learn_module` / `connect.assessment` / `connect.deliver_unit` / `connect.task` block MUST include an explicit `id` field (8-20 chars, lowercase, snake_case, stable across renames of the display name). Examples: `m1_background`, `m6_sample_prep`, `wohl_shipment`. This is now the load-bearing rule; the ≤40-char name fallback below becomes defense-in-depth. + +2. **New REQUIRED clause (Learn only)** — `connect.learn_module.time_estimate` is in **HOURS**, not minutes. Vellum's plugin help text and Connect's model docstring both say hours; an architect on the LEEP run set 10-20 (intending minutes) which Connect would have stored as 10-20-hour modules. For typical modules this is 1 or 2. + +3. **Reframed the ≤40-char name clause** — explicitly labelled FALLBACK, kicks in only when the explicit-id rule above is missed. Removal criterion split: drop the fallback when commcare-connect#1195 lands + `SLUG_LENGTH_LIMIT` bumps in lock-step, but KEEP the explicit-id rule indefinitely — it's a cleanliness invariant matching Vellum's separation, not a workaround. + +`docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md` § Generalization extended with the full Vellum source citations + a table of other Vellum-prescribed rules Nova should mirror (`time_estimate` integer-only validator → currently Nova allows decimals via `"type": "number"`; `relevantAttr` for conditional Connect-block display → not currently exposed; `ConnectWorkAreaUpdate` mug type → entire feature missing from Nova's vocabulary). + +No test or MCP code changes — purely brief-template updates + doc additions. The structural backstop (`app-release` Step 6 `oversized_slugs` projection gate, shipped in 0.13.274) remains the wall. + ## 0.13.276 — 2026-05-18 **Sweep-labs docs follow up connect-labs PR #197 (cascade-based delete gate).** diff --git a/VERSION b/VERSION index f4a6531..35500ec 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.13.276 +0.13.277 diff --git a/docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md b/docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md index 53bdb9a..2ce7bf0 100644 --- a/docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md +++ b/docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md @@ -80,7 +80,69 @@ Plus migration. The 50-char default has no operational justification (slug strin Removal criterion for the ACE-side preventers: drop the brief-template REQUIRED clauses and bump `SLUG_LENGTH_LIMIT` (and any test fixtures relying on it) in lock-step with the Connect column widening. -## Generalization +## Generalization (Vellum-as-source-of-truth) + +**The HQ-side authoring source of truth for Connect blocks is the Vellum plugin at `dimagi/Vellum:src/commcareConnect.js`.** That file is the spec Nova's `compile_app` should mirror — and it makes a key separation Nova currently elides: the Connect slug (``) comes from the form's `nodeID` field (an explicit user-supplied identifier), NOT from a slugified display name. The `name` element is a SEPARATE field that holds the human-readable label. + +From `src/commcareConnect.js:117-173` (Learn Module spec): + +```javascript +ConnectLearnModule: { + rootName: "module", + childNodes: [ + {id: "name", writeToData: true}, + {id: "description", writeToData: true}, + {id: "time_estimate", writeToData: true}, + ], + mugOptions: { spec: { + nodeID: { lstring: 'Module ID' }, // ← slug; user-supplied directly + name: { lstring: 'Name', required: true }, + description: { ... required ... }, + time_estimate: { + ... required ..., + validationFunc: mug => /^\d+$/.test(mug.p.time_estimate) ? 'pass' : 'Must be an integer', + help: 'Estimated time to complete the module in hours.', + }, + } } +} +``` + +And the emitter at `dataChildFilter` (lines 89-115): + +```javascript +getExtraDataAttributes: () => ({ + "xmlns": CCC_XMLNS, + "id": mug.p.nodeID, // ← the slug comes from nodeID, NOT a slugified name +}) +``` + +The Vellum test fixture `tests/static/commcareConnect/learn_module.xml` confirms: + +```xml + + module 1 + ... + 2 + +``` + +Two distinct values: `id="module_1"` (short, code-like, picked as the form's nodeID) and `module 1` (display label, can be long). Vellum-authored apps don't have the slug-length problem because the user picks a short stable identifier for the nodeID and writes their descriptive title in `name`. + +**Nova's behavior diverges.** Nova's `compile_app` derives the Connect `id` as `module__` when the architect doesn't set `connect.learn_module.id` explicitly — conflating two fields Vellum keeps separate. The architect-brief REQUIRED clauses (this learning doc → `pdd-to-{learn,deliver}-app/SKILL.md`) instruct the architect to ALWAYS set `id` explicitly, mirroring what a human Vellum author naturally does. + +**Other Vellum-prescribed rules Nova should mirror:** + +| Vellum rule | Source | Currently in Nova/ACE? | +|---|---|---| +| `time_estimate` is in **hours**, not minutes | `src/commcareConnect.js:158` ("Estimated time to complete the module in hours") + Connect's `LearnModule.time_estimate` docstring | NOW in `pdd-to-learn-app/SKILL.md` brief template (added 0.13.275) — previously not specified; an architect on the LEEP run set 10-20 (intending minutes) which Connect would have stored as 10-20 hours | +| `time_estimate` MUST match `^\d+$` (positive integer only) | `src/commcareConnect.js:155` (`validationFunc`) | Nova's API schema says `"type": "number"` (allows decimals) — narrower validator should be on the Nova side | +| `connect.deliver_unit.entity_id` / `entity_name` are OPTIONAL XPath fields | `src/commcareConnect.js:240, 249` (`presence: 'optional'`) | Nova has them but the `update_form` deliver_unit runtime auto-fills broken defaults (`voidcraft-labs/nova-plugin#1`) — known upstream bug | +| `relevantAttr` (XPath) for conditional display of the Connect block itself | `src/commcareConnect.js:27-40` (`logicSection`), applied to every Connect mug type | Not exposed in Nova — no current PDD needs it, low priority | +| New mug type `ConnectWorkAreaUpdate` — FLW-driven work-area issue reporting (status, reason, additional_details, photo_evidence) | `src/commcareConnect.js:321-402` | Not in Nova's vocabulary at all — possible new Connect feature we haven't surfaced. File a spec doc when relevant. | + +**Why this matters operationally.** Once the architect sets `id` explicitly, the brief's ≤40-char-name fallback becomes a defense-in-depth safety net rather than the primary rule. After commcare-connect#1195 widens the columns to 255 (or whatever), drop the ≤40-char fallback — but KEEP the explicit-id rule, because it's not a workaround. It's a cleanliness invariant matching Vellum's slug/name separation and stays correct regardless of upstream column width. + +## Two failure shapes, one bug class The 2026-05-12 `short_description` 50-char trap and this 2026-05-17 slug-length trap are **the same shape of bug at different boundaries**: diff --git a/package.json b/package.json index 0db296c..d4b9986 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ace", - "version": "0.13.276", + "version": "0.13.277", "description": "AI Connect Engine - orchestrator for building Connect Opps using AI", "type": "module", "scripts": { diff --git a/skills/pdd-to-deliver-app/SKILL.md b/skills/pdd-to-deliver-app/SKILL.md index fcbee4b..dcc9673 100644 --- a/skills/pdd-to-deliver-app/SKILL.md +++ b/skills/pdd-to-deliver-app/SKILL.md @@ -104,33 +104,61 @@ plugin (`voidcraft-labs/nova-marketplace`, slash command operator gets clean diagnostic + auto-recovery instead of "Cannot make new version" + a CCHQ UI peek. See `docs/learnings/2026-04-29-nova-connect-marker-bugs.md` § Bug 4. - - **REQUIRED — Keep deliver_unit names short enough that the derived - slug fits Connect's 50-char column.** Insert this paragraph - **verbatim** into the brief, in its own paragraph, prefixed - `REQUIRED:`: - - > REQUIRED: Every `connect.deliver_unit.name` you set MUST be ≤ 40 - > characters. Nova's `compile_app` derives the `` - > slug from the module slug (which itself comes from the module's - > display name slugified), and Connect's `DeliverUnit.slug` column - > is `SlugField()` with the Django default `max_length=50`. A - > longer module/deliver-unit name slugifies past 50 and triggers - > Postgres `DataError: value too long for type character varying(50)` - > at Connect's sync, which surfaces as an opaque HTTP 500 from - > `connect_create_opportunity` with no diagnostic. Prefer short - > active titles like "Stage 2: Sample Prep + Shipment" over the - > full descriptive form — there's no description field on - > DeliverUnit but the name is what shows up in the deliver-unit - > picker on Connect, so terseness helps readability too. + - **REQUIRED — Set `connect.deliver_unit.id` AND `connect.task.id` + explicitly to short stable identifiers, separately from the human- + readable `name`.** This is the load-bearing constraint; the ≤40-char + name fallback below is just a safety net. Insert this paragraph + **verbatim** into the brief, in its own paragraph, prefixed `REQUIRED:`: + + > REQUIRED: Every `connect.deliver_unit` and `connect.task` block + > MUST include an explicit `id` field. The id is the Connect slug — + > it MUST be short (8-20 chars), lowercase, snake_case, code-like, + > and stable across renames of the human-readable name. Examples: + > `shop_registration`, `sample_prep_initial`, `wohl_shipment`. Do + > NOT rely on Nova's default derivation (which slugifies the module + > name) — that conflates the Connect slug with the display name and + > trips Connect's 50-char `DeliverUnit.slug` column on any name that + > slugifies past ~40 chars. The `name` field is a separate, human- + > readable string that can be any length and is what shows up in + > the deliver-unit picker on Connect — terseness is preferred for + > picker readability but not required for correctness once the id + > is set explicitly. Vellum-authored apps (the human-driven + > authoring path in HQ's form designer) separate these into two UI + > fields ("Delivery Unit ID" / "Task ID" and "Name") and humans + > naturally pick short identifiers; Nova's API exposes the same two + > fields but the architect has to set both explicitly because there's + > no UI to nudge the separation. See + > `docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md` + > § Generalization (Vellum-as-source-of-truth) for the full mechanism + > + source citations. + + - **REQUIRED — Keep deliver_unit/task names short enough that the + derived slug fits Connect's 50-char column (FALLBACK).** This is + the defense-in-depth fallback for cases where the explicit-id rule + above is missed. Insert this paragraph **verbatim** into the brief, + in its own paragraph, prefixed `REQUIRED:`: + + > REQUIRED: If you have not set `connect.deliver_unit.id` / + > `connect.task.id` explicitly per the rule above, the `name` field + > MUST be ≤ 40 characters as a fallback — Nova's default slug + > derivation overflows Connect's 50-char `DeliverUnit.slug` / + > `TaskType.slug` column on longer names and triggers an opaque + > HTTP 500 from `connect_create_opportunity`. Prefer the explicit-id + > rule above (cleaner; lets `name` be any length); this clause + > exists only because architects sometimes skip the id field. Reproducer + class-level preventer: see - `pdd-to-learn-app/SKILL.md` § REQUIRED — Keep module names short. - The structural backstop is `app-release` Step 6's - `projected_connect_state.oversized_slugs.deliver_units` gate. - Removal criterion: drop this constraint when the upstream - commcare-connect PR widens `DeliverUnit.slug` to `max_length=255` + `pdd-to-learn-app/SKILL.md` § REQUIRED — Set id explicitly. The + structural backstop is `app-release` Step 6's + `projected_connect_state.oversized_slugs.deliver_units` / + `oversized_slugs.task_units` gate. Removal criteria: (a) drop the + ≤40-char fallback when the upstream commcare-connect PR widens + `DeliverUnit.slug` to `max_length=255` (already `=100` since a prior + fix) AND `TaskType.slug` to `max_length=255` (dimagi/commcare-connect#1195) and `SLUG_LENGTH_LIMIT` in `mcp/connect/backends/commcare.ts` is - bumped in lock-step. + bumped in lock-step. (b) KEEP the explicit-id rule even after the + column widens — it's a cleanliness invariant matching Vellum's + slug-vs-name separation, not just a workaround for the column width. - **REQUIRED — Architect must verify-then-retry every `add_fields` call.** Nova's `add_fields` has a partial-persistence quirk: a single call with N items often persists only the first few. The diff --git a/skills/pdd-to-learn-app/SKILL.md b/skills/pdd-to-learn-app/SKILL.md index 22f1345..3cb8b7b 100644 --- a/skills/pdd-to-learn-app/SKILL.md +++ b/skills/pdd-to-learn-app/SKILL.md @@ -96,32 +96,74 @@ Generate the Learn (training) app from the PDD using the Nova plugin name + line/col) if the architect violates this constraint anyway, so the operator gets a clear diagnostic instead of "Cannot make new version" and a CCHQ UI peek. - - **REQUIRED — Keep module names short enough that the derived slug - fits Connect's 50-char column.** Insert this paragraph **verbatim** - into the brief, in its own paragraph, prefixed `REQUIRED:`: - - > REQUIRED: Every `connect.learn_module.name` you set MUST be ≤ 40 - > characters. Nova's `compile_app` derives the `` - > slug as `module__`, and Connect's - > `LearnModule.slug` column is `SlugField()` with the Django default - > `max_length=50`. A longer name slugifies past 50 and triggers - > Postgres `DataError: value too long for type character varying(50)` - > at Connect's sync, which surfaces as an opaque HTTP 500 from - > `connect_create_opportunity` with no diagnostic. Prefer short - > active titles like "Stage 2: Sample Prep + Shipment" over the - > full descriptive form "Stage 2: Sample Preparation, Drying, - > Bagging, Shipment" — the description field is the right place - > for the long version. + - **REQUIRED — Set `connect.learn_module.id` AND `connect.assessment.id` + explicitly to short stable identifiers, separately from the human- + readable `name`.** This is the load-bearing constraint; the ≤40-char + name fallback below is just a safety net. Insert this paragraph + **verbatim** into the brief, in its own paragraph, prefixed `REQUIRED:`: + + > REQUIRED: Every `connect.learn_module` and `connect.assessment` + > block MUST include an explicit `id` field. The id is the Connect + > slug — it MUST be short (8-20 chars), lowercase, snake_case, code- + > like, and stable across renames of the human-readable name. Examples: + > `m1_background`, `m6_sample_prep`, `m1_quiz`. Do NOT rely on Nova's + > default derivation (`module__`) — that + > conflates the Connect slug with the display name and trips Connect's + > 50-char `LearnModule.slug` column on any name that slugifies past + > ~40 chars. The `name` field is a separate, human-readable string + > that can be any length and any character set — that's where the + > descriptive title belongs. Vellum-authored apps (the human-driven + > authoring path in HQ's form designer) separate these into two UI + > fields ("Module ID" and "Name") and humans naturally pick short + > identifiers; Nova's API exposes the same two fields but the + > architect has to set both explicitly because there's no UI to + > nudge the separation. See `docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md` + > § Generalization (Vellum-as-source-of-truth) for the full mechanism + > + source citations. + + - **REQUIRED — `connect.learn_module.time_estimate` is in HOURS, not + minutes.** Insert this paragraph **verbatim** into the brief, in its + own paragraph, prefixed `REQUIRED:`: + + > REQUIRED: The `connect.learn_module.time_estimate` field is the + > estimated time to complete the module in **HOURS**, not minutes. + > Vellum's plugin help text says verbatim "Estimated time to complete + > the module in hours" (`src/commcareConnect.js:158`) and Connect's + > `LearnModule.time_estimate` model field docstring says "Estimated + > hours to complete the module". For typical Learn modules this is + > 1 (one hour) or 2; never a two-digit minute count. If a module + > genuinely takes less than an hour, round up to 1 — Connect displays + > the value in dashboards as hours-to-complete and FLW-onboarding + > timing calculations downstream assume the unit. + + - **REQUIRED — Keep module/assessment names short enough that the + derived slug fits Connect's 50-char column (FALLBACK).** This is the + defense-in-depth fallback for cases where the explicit-id rule above + is missed. Insert this paragraph **verbatim** into the brief, in its + own paragraph, prefixed `REQUIRED:`: + + > REQUIRED: If you have not set `connect.learn_module.id` / + > `connect.assessment.id` explicitly per the rule above, the `name` + > field MUST be ≤ 40 characters as a fallback — Nova's default slug + > derivation `module__` overflows Connect's + > 50-char `LearnModule.slug` column on longer names and triggers an + > opaque HTTP 500 from `connect_create_opportunity`. Prefer the + > explicit-id rule above (cleaner; lets `name` be any length); this + > clause exists only because architects sometimes skip the id field. Reproducer: `leep-paint-collection` run 20260517-1515 Phase 4 hit - this on M6 (52-char slug). The structural backstop is `app-release` + this on M6 (52-char slug derived from "Stage 2: Sample Preparation, + Drying, Bagging, Shipment"). The structural backstop is `app-release` Step 6's `projected_connect_state.oversized_slugs` gate — even if - the architect ships an over-length name, the release-time projection - halts before Phase 4 ever calls Connect. Removal criterion: drop - this constraint when the upstream commcare-connect PR widens - `LearnModule.slug` / `DeliverUnit.slug` to `max_length=255` and - `SLUG_LENGTH_LIMIT` in `mcp/connect/backends/commcare.ts` is - bumped in lock-step. + the architect ships an over-length slug, the release-time projection + halts before Phase 4 ever calls Connect. Removal criteria: (a) drop + the ≤40-char fallback when the upstream commcare-connect PR widens + `LearnModule.slug` / `DeliverUnit.slug` to `max_length=255` (PR + dimagi/commcare-connect#1195) and `SLUG_LENGTH_LIMIT` in + `mcp/connect/backends/commcare.ts` is bumped in lock-step. (b) KEEP + the explicit-id rule even after the column widens — it's a + cleanliness invariant matching Vellum's slug-vs-name separation, + not just a workaround for the column width. - **REQUIRED — Architect must verify-then-retry every `add_fields` call.** Nova's `add_fields` has a partial-persistence quirk: a single call with N items often persists only the first few.