feat(formation): canonical shared types for formations and checklist templates - #2175
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Essentials Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
PR SummaryLow Risk Overview Adds Adds Barrel exports wire the new modules through Reviewed by Cursor Bugbot for commit 288e87f. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
Introduces canonical shared formation contracts to unblock parallel formation feature branches.
Changes:
- Adds formation interfaces and response shapes.
- Adds section, owner-team, and action enums.
- Adds enum vocabulary and template-shape tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/index.ts |
Exports formation interfaces. |
packages/shared/src/interfaces/formation.interface.ts |
Defines formation domain contracts. |
packages/shared/src/enums/index.ts |
Exports formation enums. |
packages/shared/src/enums/formation.enum.ts |
Defines canonical formation vocabularies. |
packages/shared/src/enums/formation.enum.spec.ts |
Tests enum values and a template fixture. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ], | ||
| }, | ||
| ], | ||
| } satisfies FormationTemplate; |
| entity_type: FormationEntityType; | ||
| template_uid: string; | ||
| template_version: number; | ||
| sub_stage: FormationSubStage; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/shared/src/interfaces/formation.interface.ts:50
- Linked issue #1957 defines Activating as all gating items being
doneandannouncement_datebeing set. This definition omits the date and says the downstream helper is identical, but #2033's helper only inspects items and also treatsskippedas resolved. That would give fixtures and the eventual service different meanings foris_activating; define the canonical semantics here and align the helper when #2033 rebases.
/** Derived: every gating item `done` (and at least one gating item exists). TODO(#1957): backend-derived once real; see `deriveFormationReadinessSummary`, which computes this identically. */
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
packages/shared/src/interfaces/formation.interface.ts:30
FormationSubStageis also the type ofFormation.sub_stage, so restricting it to queue-filter values makesFormationChecklistResponseunable to represent a Confidential formation. Issue #1957 now explicitly requires Confidential projects to receive a checklist like every other Formation stage. Split the full runtime stage union from the narrower queue-filter union (and use the latter for tiles/labels) so the canonical service payload can includeconfidentialwithout adding it as a staff queue pill.
export type FormationSubStage = 'exploratory' | 'engaged' | 'on_hold';
packages/shared/src/enums/formation.enum.spec.ts:67
- This
satisfies FormationTemplatecheck is not enforced by either package gate:check-typesexcludes*.spec.ts, whilevitest runtranspiles TypeScript without type-checking. Consequently, an incompatible change such as adding a required template field would still leave this “accepts” test green; its only runtime assertion verifies an array length. Move the compile-time fixture into a file included bytsc, or add a dedicated type-test configuration that CI executes.
} satisfies FormationTemplate;
| /** ISO date. Null until a gating item sets it. */ | ||
| announcement_date: string | null; | ||
| /** | ||
| * Derived: every gating item `done` (and at least one gating item exists). An `awaiting_acceptance` | ||
| * gating item does not count as `done`, so it keeps this false. TODO(#1957): backend-derived once real. |
…templates (GH-2163) Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…H-2163) Six amendments to the canonical formation types, per the Epic 1 architecture review and two product calls (4 Sep): - FormationItemStatus: drop waiting_on_partner (a UI wording of blocked, not a stored value), add blocked and awaiting_acceptance (an assignee-completed item stays on Pending Actions until the formation team accepts it; only done counts toward readiness). - FormationSubStage: drop activating — it's derived readiness (Formation.is_activating), not a PCC sub-stage, and belongs in a separate "Gates cleared" badge, never the Stage column. - can_complete: keep the field, rewrite the comment — there is no gate_writer relation; gating is a property of the item, guarded by project write permission + is_gating. - FormationEntityType: rename 'subproject' -> 'child_project' (user-facing term is "child project"); documented as a display taxonomy to derive later (TODO #1958). FormationQueueTiles.subprojects renamed to child_projects to match. - is_gating doc: "Gates Active" -> "Required for Active", the agreed 4 Sep vocabulary for the chip/strip/Me-lens marker/email. - FormationLead -> FormationUser: not an Epic 2 "formation lead" record (#1992); it's a plain user reference. Adds a runtime regression guard on FORMATION_SUB_STAGE_LABELS' key set in formation.enum.spec.ts. Every consumer of these types lives on feat/GH-1958-formation-checklist-queue (#2033) — verified read-only, not touched here. feat/issue-1959 (#2035) has zero references to any of the six. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
|
@manishdixitlfx the types landing is doing the job #2163 asked for. Naming and the 4 Sep amendments ( I still cannot approve. The branch conflicts with
Bot reconciliation
Revision tracking Resolved in Still open: the type-test comment (documented, not blocking), the CodeRabbit skipped. Cursor Bugbot flagged low risk and did not raise findings. 🔴 Needs changes before approval |
dealako
left a comment
dealako
left a comment
There was a problem hiding this comment.
Canonical shared types for GH-2163 look sound on naming and the 4 Sep amendments. Requesting changes because this branch conflicts with main on the constants barrel, and the announcement_date / is_activating JSDoc still disagrees with #1957 (Copilot's open thread).
- 🔴 Blocking: 1. Rebase and keep both
id-migration.constantsandformation.constantsexports inpackages/shared/src/constants/index.ts. - 🟡 Minor: 1. Fix the
announcement_datecomment (PCC-owned legal field, not "set by a gating item") and say plainly thatis_activatingis the gates-cleared half of readiness, not the full #1957 predicate.
See the review summary comment for bot reconciliation and revision tracking.
MRashad26
left a comment
MRashad26
left a comment
There was a problem hiding this comment.
Reviewed the full diff (7 files, +376/-0, types/constants only) and cross-checked the claimed conflict-resolution/naming decisions against the actual current main.
What this does (GH-2163): Lands the canonical formation.interface.ts + formation.enum.ts on main ahead of three in-flight PRs (#2019/#2033/#2035) that would otherwise each independently create these files and collide (add/add conflicts, plus a genuine duplicate-export name collision on FORMATION_SUB_STAGE_LABELS). Pure types/constants/tests — no runtime consumers touched.
Verified against current code:
- Confirmed via
git grepagainstorigin/mainthat none ofFormationTemplateSection,FormationOwnerTeam,FormationActionType,FormationSubStage,FORMATION_SUB_STAGE_LABELSexist anywhere inpackages/shared/srctoday — this really is a clean net-new addition, not a rename colliding with something already there. - The
FormationTemplateSectionKeyenum rename (from #2035's originalFormationTemplateSection) is necessary and correctly applied:FormationTemplateSectionis used here as the structural interface name ({ key: FormationTemplateSectionKey; title; items }), so keeping both names distinct avoids the exact collision the PR describes. FORMATION_SUB_STAGE_LABELS'sas const satisfies Record<FormationSubStage, string>pattern matches an existing precedent in this package (survey.constants.ts'sas const satisfies Record<string, CombinedSurveyStatus>), and the repo's TypeScript version (5.8.3) supportssatisfies.formation.enum.spec.ts's exhaustiveness assertions (Object.values(...).sort()) directly pin each enum's member set, and theFormationTemplate"satisfies" object literal round-trips a real nested section→item→sub_item shape — a reasonable, honestly-scoped substitute for the type-level "sub-items nest one level only" invariant, which the spec's own comment explains can't be gate-enforced given this package'svitest run(no type-check) +check-types(excludes*.spec.ts) setup. That's a good call rather than a silently-decorative@ts-expect-error.- The doc comments consistently distinguish enum-scoped fields (template-side, curated vocabulary) from plain-string runtime fields (
FormationItem.owner_team: string | null,Formation.entity_type) with explicitTODO(#1957)/TODO(#1958)narrowing markers — this is a deliberate, well-reasoned split (template vocab is curated/stable; runtime fields await the real backend service) rather than an inconsistency. FormationItemStatus'sawaiting_acceptanceaddition and the readiness-counting comment ("onlydonecounts toward readiness") are consistent with theFormation.is_activatingdoc comment's own wording ("Anawaiting_acceptancegating item does not count asdone").- Barrel-file wiring (
constants/index.ts,enums/index.ts,interfaces/index.ts) is correctly added; no existing export at those insertion points is disturbed.
Minor, non-blocking observations:
- This PR proactively documents (in its own description) that it blocks #2019/#2033/#2035 and that #2033's rebase must hand-resolve
formation.constants.ts(a same-path add/add it can't auto-merge) — that coordination is called out clearly enough that it's on the follow-up PRs, not a gap in this one. - Nothing to flag on test coverage: this is a types-only change, and the new spec file's exhaustiveness + shape-round-trip tests are an appropriate level of verification for what's being added (no runtime logic to exercise).
No issues found. Approving.
d0eb54b to
288e87f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Suppressed comments (3)
packages/shared/src/interfaces/formation.interface.ts:30
FormationSubStageis also used byFormation.sub_stage, but it cannot representconfidential. Issues #1957 and #1958 require Confidential projects to receive a checklist and appear as ordinary access-filtered queue rows. Define the full service-level sub-stage domain (includingconfidential) separately from the narrower queue-filter keys; otherwise the canonical response drops a required record or incorrectly adds a Confidential filter.
export type FormationSubStage = 'exploratory' | 'engaged' | 'on_hold';
packages/shared/src/interfaces/formation.interface.ts:58
- This documents the wrong owner for the field:
announcement_datecomes from the project record, which the formation service reads but never writes; a gating item does not set it. Correcting this is important because this file is intended to define the future service contract.
/** ISO date. Null until a gating item sets it. */
announcement_date: string | null;
packages/shared/src/enums/formation.enum.spec.ts:67
satisfies FormationTemplateis not checked by either gate: this spec is excluded fromtsc, and Vitest/esbuild erases the type expression. An incompatible change toFormationTemplatetherefore still passes while this test continues to claim the shape round-trips. Move the fixture into a TypeScript input included bycheck-types, or add a dedicated type-test configuration.
} satisfies FormationTemplate;
| parent_project_name: string; | ||
| /** Present only for a `child_project` — the foundation/project this formation nests under, for the queue's indented display. */ | ||
| parent_formation_name?: string; | ||
| entity_type: FormationEntityType; |
| /** First not-done gating item's title, precomputed for the queue's "Blocking" column. */ | ||
| blocking_item_title: string | null; |
| /** Per-`sub_stage` counts for the queue's filter pills. */ | ||
| export type FormationQueueTiles = Record<FormationSubStage, number> & { | ||
| total: number; | ||
| foundations: number; | ||
| child_projects: number; | ||
| }; | ||
|
|
||
| /** Response body for `GET /api/formations`. */ | ||
| export interface FormationsQueueResponse { | ||
| tiles: FormationQueueTiles; | ||
| rows: Formation[]; |
| /** Required and logged when a gating item is skipped. */ | ||
| skip_reason: string | null; |
|
@manishdixitlfx thanks for the rebase. That was the blocking item. 👏 Nice work. You rebased onto
New Copilot threads (after the rebase)
No new CodeRabbit or Bugbot findings this round. ✅ Approved with minor comments |
dealako
left a comment
dealako
left a comment
There was a problem hiding this comment.
Rebase onto main cleared the blocking barrel conflict. Approving.
One leftover minor: announcement_date still says a gating item sets it. PCC owns that field. Copilot's thread already covers it: #2175 (comment)
…ary (GH-1958) Migrate onto the canonical shared Formation types from GH-2163/PR #2175 and close three product gaps: every checklist row is now actionable via a status-control menu and overflow menu (assign/due date/skip), the new awaiting_acceptance status is handled end-to-end (Accept flow, correct counts, blue segment color), and gate-related copy is unified to "Required for Active" across the row, drawer, readiness strip, and queue table. Refs #1958 Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…ary (GH-1958) Migrate onto the canonical shared Formation types from GH-2163/PR #2175 and close three product gaps: every checklist row is now actionable via a status-control menu and overflow menu (assign/due date/skip), the new awaiting_acceptance status is handled end-to-end (Accept flow, correct counts, blue segment color), and gate-related copy is unified to "Required for Active" across the row, drawer, readiness strip, and queue table. Refs #1958 Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
…ary (GH-1958) Migrate onto the canonical shared Formation types from GH-2163/PR #2175 and close three product gaps: every checklist row is now actionable via a status-control menu and overflow menu (assign/due date/skip), the new awaiting_acceptance status is handled end-to-end (Accept flow, correct counts, blue segment color), and gate-related copy is unified to "Required for Active" across the row, drawer, readiness strip, and queue table. Refs #1958 Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org>
Summary
Lands the canonical
packages/shared/src/interfaces/formation.interface.tsand a newpackages/shared/src/enums/formation.enum.tsonmain, resolving the add/add conflict where#2033 and #2035 each independently create
formation.interface.tsfrom scratch.uid, snake_case, nestedsections[].items[].FormationTemplateItem.sub_items?is new (feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033 had no equivalent), and
FormationOwnerTeam/FormationActionType/FormationTemplateSectionKey(renamed from feat(formation): seed the project formation checklist template #2035'sFormationTemplateSection, which collidedwith feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033's structural interface of the same name) are TS enums per feat(formation): seed the project formation checklist template #2035's convention.
FormationActionTypegainsSTATUS_ONLY, which only feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033 needed.owner_teamis split: enum on the template item/sub-item,string | null(TODO(New service: lfx-v2-formation-service (templates, checklists, items, invites, activity) #1957) tonarrow) on the runtime
FormationItem— feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033's fixtures use display labels (incl.'PMO',deliberately absent from the enum) there today.
Formation.state/FormationStatedropped — banned application-flow values, and dead in all11 of feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033's fixture sites.
FORMATION_SUB_STAGE_LABELSmap(
packages/shared/src/constants/formation.constants.ts), keyed byFormationSubStage. feat(formation): badge, card and staff deep links on the project page #2019 andfeat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033 each independently export a constant with this same name — a duplicate-export build break
invisible until both land, same class of defect as the original add/add. The two turned out to
key genuinely different enums (feat(formation): badge, card and staff deep links on the project page #2019's is
ProjectStage-keyed, covers 5 stages includingDisengaged/Confidential, and backs
isFormationStage/getFormationSubStageLabel), so this isn'tone map declared twice — it's a name collision between two different maps. feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033's
FormationSubStage-keyed map (the queue's heavier-consumer shape) is canonical here, verbatimincluding its
'Formation · 'prefix; feat(formation): badge, card and staff deep links on the project page #2019 renames its own map on its rebase.product decisions, before any of feat(formation): badge, card and staff deep links on the project page #2019/feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033/feat(formation): seed the project formation checklist template #2035 consumes them:
FormationItemStatus: dropwaiting_on_partner(a UI wording ofblocked, not a storedvalue), add
blockedandawaiting_acceptance(an assignee-completed item stays on PendingActions until the formation team accepts it —
in_progressunderstates it,doneoverstatesit and would count it toward readiness). Only
donecounts toward readiness;awaiting_acceptanceisTODO(#1957)-flagged as a provisional name.FormationSubStagedropsactivating— derived readiness (Formation.is_activating), not aPCC sub-stage; rendered as a separate "Gates cleared" badge (Formation checklist on the project page and the Formations queue #1958), never the Stage column.
can_completefield stays; its comment no longer describes agate_writerrelation that willnever exist — gating is a property of the item (
is_gating), guarded by project writepermission,
TODO(#1957)for the real service-side check.FormationEntityType:'subproject'→'child_project'(user-facing term), documented as adisplay taxonomy to derive later (
TODO(#1958)) rather than store — the canonicalFormationhas neither
is_foundationnor a parent-is-root signal yet, so deriving now would mean addingfields, not removing one.
FormationQueueTiles.subprojectsrenamed tochild_projectsto match.is_gatingdoc: "Gates Active" → "Required for Active", the agreed 4 Sep vocabulary forthe chip, readiness strip, Me-lens marker and item-assigned email.
FormationLead→FormationUser— it backs a plain user reference (FormationItem.owner,FormationActivity.actor), not the Epic 2 "formation lead" record (Formation Checklist — Epic 2: project applications, provisioning, requests, join page, template editor #1992).FORMATION_SUB_STAGE_LABELS' key set information.enum.spec.ts(noactivating).feat/GH-1958-formation-checklist-queue) —inventoried read-only, not touched here; feat(formation): seed the project formation checklist template #2035 has zero references to any of them. See the
consumer inventory in the Land canonical formation shared types on main before the feature branches merge #2163 issue thread for the exact list feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033's rebase must absorb,
including the three duplicated
status !== 'done' && status !== 'skipped'predicates thatalready exclude
awaiting_acceptancefor free, and the three exhaustiveRecord<FormationItemStatus, …>constant maps that do not.Verified round-trip against #2035's real 17-item seeded template (2 sections, 4 gating items, 4
chat-workspace sub-items) — every field maps with no information loss; #2035 gains two new
required fields on rebase (
FormationTemplate.name,FormationTemplateSection.title), which itsflat shape never carried.
Blocks: #2019, #2033, #2035 (and #1993, #1956 later). Full derivation and decisions:
~/lfx/formation-integration-conflict-report.md(2026-09-03 section) and the GH-2163 issue.Closes #2163
Test plan
./check-headers.sh && yarn lint && yarn format && yarn test && yarn build— all passformation.enum.spec.ts: enum exhaustiveness for all three vocabularies (real,runtime-enforced) + a minimal template literal round-trips the canonical shape
gate-enforced (this package's
testis plainvitest run, no type-checking;check-typesexcludes
*.spec.ts) — documented in the spec file rather than left as an inert@ts-expect-errorgit merge-tree --write-tree) against scratch copies of feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033 and feat(formation): seed the project formation checklist template #2035with their local
formation.interface.ts/formation.enum.tsdeleted (simulating thepost-merge rebase): clean merge, no
add/addconflict, in both casesFORMATION_SUB_STAGE_LABELS: confirmedformation.constants.tsis a genuinely new path onmainbut an existing one on feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033 — so, unlike the interface/enum files, this one cannotresolve to a clean automatic merge; feat(formation): checklist section and Formations queue, Epic 1 scope (#1958) #2033's rebase must hand-resolve the add/add by keeping
this canonical label block and re-adding its own remaining unique constants (severity maps,
queue tiles, row-action configs) into the same file. Simulated that resolution by hand in a
scratch worktree and confirmed the merged file references only types already present in the
canonical
formation.interface.ts/formation-checklist.interface.ts/components.interface.ts— not full-build-verified there (the disposable worktree has no installed
node_modules)./check-headers.sh && yarn lint && yarn format && yarn test && yarn build— all pass (
@lfx-one/shared60/60 files,lfx-one-ui92/92 + 83/83 files);grepconfirmswaiting_on_partner,'activating','subproject', andFormationLeadare gone frompackages/shared🤖 Generated with Claude Code
https://claude.ai/code/session_01CdGb7zS2HyLZtgFAxokw71