Skip to content

feat(skills): mirror Vellum's slug/name separation in Nova architect brief#352

Merged
jjackson merged 1 commit into
mainfrom
emdash/vellum-slug-name-separation
May 18, 2026
Merged

feat(skills): mirror Vellum's slug/name separation in Nova architect brief#352
jjackson merged 1 commit into
mainfrom
emdash/vellum-slug-name-separation

Conversation

@jjackson
Copy link
Copy Markdown
Owner

Summary

Follow-up to #347 (0.13.274). That PR capped module/deliver-unit name length at 40 chars to dodge Connect's 50-char SlugField columns — treating the symptom. This PR addresses the cause by mirroring what the HQ-side authoring source of truth — dimagi/Vellum:src/commcareConnect.js, the plugin humans use in the form designer when the COMMCARE_CONNECT flag is on — does naturally: keep the Connect slug and the human display name as TWO SEPARATE fields.

From src/commcareConnect.js:117-173:

```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 },
...
} }
}
```

Emitter at `dataChildFilter:89-115`:

```javascript
getExtraDataAttributes: () => ({
"xmlns": CCC_XMLNS,
"id": mug.p.nodeID, // ← slug from nodeID, NOT a slugified name
})
```

Fixture `tests/static/commcareConnect/learn_module.xml`:

```xml
<module xmlns="http://commcareconnect.com/data/v1/learn\" id="module_1">
module 1
...
<time_estimate>2</time_estimate>

```

Two distinct values: `id="module_1"` (short, code-like) and `module 1` (display label, can be long). Vellum-authored apps never hit the slug-length trap because humans naturally pick short identifiers.

Nova diverges. Nova's `compile_app` defaults the Connect `id` to `module__<slugify(name)>` when `connect.learn_module.id` is omitted. The API already supports the explicit-id field; ACE's brief just never told the architect to set it.

Changes

Three brief-template additions to `pdd-to-learn-app/SKILL.md` and `pdd-to-deliver-app/SKILL.md`:

  1. NEW REQUIRED clause: set `connect..id` explicitly. Every `learn_module` / `assessment` / `deliver_unit` / `task` block MUST include an explicit `id` field (8-20 chars, lowercase, snake_case, stable across renames of the display name). This becomes the load-bearing rule; the ≤40-char name fallback below drops to defense-in-depth.

  2. NEW REQUIRED clause (Learn only): `time_estimate` is in HOURS. Vellum's plugin help text says "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". An architect on the LEEP run set 10-20 (intending minutes) which Connect would have stored as 10-20-hour modules.

  3. Reframed ≤40-char name clause as 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 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 exposed in Nova)
  • `ConnectWorkAreaUpdate` mug type (entire feature missing from Nova's vocabulary)

What's NOT in this PR

  • No MCP / test code changes. The structural backstop (`app-release` Step 6's `oversized_slugs` projection gate, shipped in 0.13.274) remains the wall — this PR shifts left of it, not at it.
  • The Vellum-prescribed rules in the new doc table are surfaced for visibility but not implemented (`time_estimate` validator narrowing, `relevantAttr`, `ConnectWorkAreaUpdate`) — they're follow-up candidates, not load-bearing today.

Test plan

  • No code changes; doc + skill template only. `tsc --noEmit` clean (unchanged from main).
  • After merge + `/ace:update`, the next `/ace:run` invocation of `pdd-to-learn-app` / `pdd-to-deliver-app` will emit a brief with the new REQUIRED clauses. Verify the Nova architect actually sets `connect..id` explicitly in the resulting build.
  • Cross-reference: a Nova-built Learn app from a fresh run should produce CCZ form XML matching the shape `<module id="m1_background">Why This Survey Exists…` (id and name distinct) rather than the prior `<module id="module_1_why_this_survey_exists">` shape.

🤖 Generated with Claude Code

@jjackson jjackson enabled auto-merge May 18, 2026 20:51
…eliver}-app brief

Follow-up to 0.13.274. That fix capped module/deliver-unit *name* length
at 40 chars as a workaround for Connect's 50-char SlugField columns —
treating the symptom. The actual cause is that Nova's default
`<learn:module id>` derivation conflates the Connect slug with the
human-readable display name.

The HQ-side authoring source of truth, `dimagi/Vellum:src/commcareConnect.js`
(the Vellum plugin that loads when COMMCARE_CONNECT flag is on), keeps
these as two SEPARATE fields:
  - `nodeID` is the slug — user picks something short like `module_1`
  - `name` element is the display label — any length

The fixture confirms: `<module id="module_1"><name>module 1</name>…</module>`.
Nova's API already supports the split (`connect.learn_module.id` is an
optional field on `update_form`); ACE's brief never told the architect
to set it.

Three brief-template additions to `pdd-to-{learn,deliver}-app/SKILL.md`:

1. New REQUIRED clause: every `connect.*` block MUST include explicit
   `id` (8-20 chars, lowercase, snake_case, stable across renames).
   This becomes the load-bearing rule; ≤40-char name fallback drops
   to defense-in-depth.

2. New REQUIRED clause (Learn only): `time_estimate` is in HOURS, not
   minutes. Vellum's help text + Connect's model docstring both say
   hours. An architect on the LEEP run set 10-20 minutes — Connect
   would have stored 10-20-hour modules.

3. Reframed ≤40-char fallback as FALLBACK only. Removal criterion split:
   drop fallback when commcare-connect#1195 lands + SLUG_LENGTH_LIMIT
   bumps; KEEP explicit-id rule indefinitely (cleanliness invariant
   matching Vellum, not a workaround).

`docs/learnings/2026-05-17-connect-slug-length-50-char-trap.md`
§ Generalization extended with full Vellum source citations + table
of other Vellum rules to mirror (`time_estimate` integer-only validator,
`relevantAttr` for conditional Connect-block display, `ConnectWorkAreaUpdate`
mug type).

No MCP/test code changes — purely brief-template + doc. Structural
backstop (`app-release` Step 6 `oversized_slugs` gate from 0.13.274)
remains the wall.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@jjackson jjackson force-pushed the emdash/vellum-slug-name-separation branch from 7d81cf9 to 5766ac2 Compare May 18, 2026 21:00
@jjackson jjackson merged commit a21727a into main May 18, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant