fix(formation): stop checklist sub_stage guessing engaged

[manishdixitlfx]

Summary

deriveFormationSubStage (formation-mapper.helper.ts) fell through its default: case to 'engaged' for every project stage outside the three queue sub-stages — Formation - Disengaged, Formation - Confidential, Active, Archived, Prospect, and an empty stage all silently became 'engaged'. In production, 3 of 126 projects (Agent Router, Durable Agents, KapunSDK) hit this. Same defect class #2370 already fixed on the queue path (getFormationsQueueLive).

Fixes GH-2328 scope item 4 only — the other four items (propagating lifecycle, 404-vs-read-only, gating the eight item mutations, tightening the route guard) stay on #2328.

Correction to the issue's 13 Sep comment

The 13 Sep comment on #2328 states "their checklist pages label them Formation · Engaged right now." This is not reproducible on main — verified by repo-wide grep of sub_stage/SubStage:

• deriveFormationSubStage had exactly one caller (mapUpstreamFormationChecklist), writing Formation.sub_stage. Its own doc comment claimed a second caller in getProjectFormation's ROOT-collapse path — that was stale; ROOT-collapse never called it.
• Nothing renders Formation.sub_stage. The checklist page has no stage display at all; the checklist section only reads gating_items_open/gating_items_total.
• The "Formation · Engaged" chip visible on checklist-adjacent pages comes from a separate, already-correct path: project-dashboard.component.html / formation-card.component.html → ProjectContextService.activeProjectFormationSubStage → getFormationSubStageLabel(project.stage), which uses the 5-value ProjectStage taxonomy and already returns null for Active.

So the wrong 'engaged' reached the wire in GET /api/projects/:slug/formation's response body, but no pixel rendered it. Still worth fixing — a confidently-wrong value on a shipped API contract, before any consumer trusts it — but it's a contract fix, not a rendering fix. No template changes are in this PR.

Changes

• Deleted deriveFormationSubStage outright — its three cases duplicated UPSTREAM_SUB_STAGE_TO_FORMATION_SUB_STAGE's three keys; the only difference was the wrong default:.
• mapUpstreamFormationChecklist now normalizes sub_stage via the shared normalizeFormationSubStage (added by fix(formation): normalize sub_stage at BFF boundary #2370 for the queue), sourced from ctx.project.stage — the checklist read (UpstreamFormationChecklist) carries no stage field of its own.
• Formation.sub_stage widened to FormationSubStage | null; added Formation.sub_stage_raw: string carrying the verbatim upstream stage, mirroring FormationQueueRow. null means the project's ProjectStage has no queue-taxonomy equivalent (Formation Disengaged/Confidential, or a non-Formation stage) — a future consumer renders sub_stage_raw through getFormationQueueStageDisplay rather than guessing. FormationSubStage itself is not widened.
• Updated all Formation-typed fixtures (e2e mocks, component specs, enum spec) for the new required field.
• docs/architecture/backend/server-helpers.md updated to reflect the checklist mapper now also calling normalizeFormationSubStage.

Explicitly out of scope

• FormationSubStage union stays 3 values — no widening.
• No filtering/404-ing of affected projects off the checklist page (Formation checklist ignores upstream lifecycle: completed and frozen formations render as actionable #2328 scope item 2, a product decision).
• getFormationsQueueLive and anything else fix(formation): normalize sub_stage at BFF boundary #2370 changed — untouched beyond consuming its helpers.
• my-formations-card.component.html:24 (unguarded label/severity index off MyFormationSummary.sub_stage) — reported, not fixed (below); already flagged latent on fix(formation): normalize sub_stage at BFF boundary #2370.

Tests

• New apps/lfx-one/src/server/helpers/formation-mapper.helper.spec.ts (helper had no spec before) — covers sub_stage/sub_stage_raw with real upstream stage strings, not short keys (short-key fixtures are how both Formations queue: sub_stage contract mismatch zeroes every tile, blanks stage chips and breaks the filter tabs #2366 and this bug reached production undetected): all 5 Formation-taxonomy stages, Active, Archived, Prospect, unrecognized string, empty string, and a 'constructor' prototype-pollution guard case.
• formation.service.spec.ts: added sub_stage/sub_stage_raw assertions to the existing Active-stage fixture, plus an it.each across 8 real upstream stage strings through the full getProjectFormation service path.

All green: yarn check-types, yarn test (2757/2757), yarn lint, yarn format:check, ./check-headers.sh, yarn build.

Report — other instances of the same fallback-to-valid-value pattern (not fixed here)

A default:/??/unguarded-index that turns an unrecognized upstream value into a specific valid value instead of "unknown":

1. apps/lfx-one/src/server/services/org-lens-projects.service.ts:569-580 — mapInfluence() defaults unrecognized Snowflake TECHNICAL_INFLUENCE/ECOSYSTEM_INFLUENCE values to 'silent'/'non-lf' — real, presentable bands, not "unknown." Reachable (live org-projects dashboard data).
2. apps/lfx-one/src/server/services/org-lens-projects.service.ts:582-584 — mapTrendDirection() collapses any unrecognized trend value to 'flat', masking a real signal or anomaly as "no change." Reachable, same data path.
3. apps/lfx-one/src/server/services/org-lens-project-detail.service.ts:1092-1103 — mapDetailLevel() defaults unrecognized/null/mis-cased influence-level values to 'Silent' (the most negative category). Reachable — feeds the org leaderboard detail drawer.
4. apps/lfx-one/src/server/services/ai.service.ts:535-554 — getMeetingTypeDescription() defaults any meetingType outside the 7 known enum members to 'project team' — the same label as the legitimate OTHER case, so a malformed value is indistinguishable from a real "Other" meeting. Reachable if the request body isn't runtime-validated before this call.
5. packages/shared/src/utils/date-time.utils.ts:539-554 — mapRecurrenceToFormValue() defaults any unrecognized recurrence.type to 'none' ("does not repeat"). Latent today (closed 3-value enum), but would silently hide a genuinely recurring meeting the moment a new recurrence type is added upstream.

Three-plus instances of the same shape (this PR's fix, plus #1–5) — worth considering a lint rule (e.g. flag default: in a mapper that returns a literal from a known-value union) over one-off fixes.

Validation gap — flagging explicitly

Per the plan, prod read-only validation (GET /api/projects/<slug>/formation for Agent Router, Durable Agents, KapunSDK, confirming sub_stage: null / sub_stage_raw verbatim, before/after JSON) requires the integration worktree (~/lfx/ss-integration) wired to prod via /lfx-serve in its own session — that's a separate, heavier bring-up step outside this per-ticket worktree's scope, and no integration server was running. Not done in this PR — recommend running it in the integration pass alongside the other in-flight tickets, per the standard workflow.

Refs #2328 (scope item 4 only).

🤖 Generated with Claude Code

https://claude.ai/code/session_01FCkcxE9GbFf7gg8z4baA2e

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Essentials

Run ID: a0d3ae83-1807-484f-bc9e-0f053827050a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Fixes checklist API stage normalization so unmapped stages return null instead of incorrectly defaulting to engaged.

Changes:

• Reuses shared formation-stage normalization and preserves the raw stage.
• Widens the checklist contract and updates fixtures.
• Adds mapper and service regression coverage.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/shared/src/interfaces/formation.interface.ts	Updates the checklist stage contract.
packages/shared/src/enums/formation.enum.spec.ts	Updates typed contract fixtures.
docs/architecture/backend/server-helpers.md	Documents shared normalization.
apps/lfx-one/src/server/services/formation.service.spec.ts	Tests the full service path.
apps/lfx-one/src/server/helpers/formation-mapper.helper.ts	Replaces fallback derivation with normalization.
apps/lfx-one/src/server/helpers/formation-mapper.helper.spec.ts	Adds mapper regression tests.
apps/lfx-one/src/app/modules/dashboards/components/formation-entry-card/formation-entry-card.component.spec.ts	Updates the component fixture.
apps/lfx-one/e2e/helpers/formation-checklist.helper.ts	Updates checklist API mocks.
apps/lfx-one/e2e/fixtures/mock-data/formation.mock.ts	Updates shared E2E formation data.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[manishdixitlfx]

Prod validation done — the gap flagged in the PR body is closed

Run against the prod-backed integration tree (ss-integration-2328-2372, this branch merged with #2374), read-only, 13 Sep.

GET /api/projects/<slug>/formation for the three affected projects:

Project	sub_stage	sub_stage_raw	items
Agent Router	null	"Active"	17
Durable Agents	null	"Formation - Disengaged"	17
KapunSDK	null	"Active"	17

All three previously resolved to 'engaged' through the deleted default:. The raw stage now comes back verbatim and nothing downstream has to guess.

No regression on the queue path

The same session checked #2370's surface, since this branch and #2374 both touch formation.interface.ts and the two merged cleanly without conflict:

tiles: { exploratory: 106, engaged: 7, on_hold: 10, unmapped: 3,
         total: 126, foundations: 0, projects: 126 }
mapped sum = 123    total - mapped = 3    == unmapped  ✓

The PR's correction to the issue stands

This PR's body corrects the 13 Sep comment on #2328, which claimed the three projects' checklist pages label them "Formation · Engaged" today. That correction is right: nothing renders Formation.sub_stage, so the wrong value reached the API response body and no pixel. I wrote that comment from the derivation code without checking the render path, and the PR's grep evidence is the better account. It is still worth fixing — a confidently wrong value on a shipped contract, before a consumer trusts it — but as a contract fix, which is what this PR is.

Also worth knowing, from the same session

Every one of the 2142 items across all 126 production formations is not_started — nothing has been touched since the reconcile sweep created them. So no consumer is currently reading sub_stage from this endpoint in anger, which is the most comfortable moment to change its type.

yarn e2e was not part of this pass.

[cursor]

PR Summary

Medium Risk
Changes the GET /api/projects/:slug/formation response contract (sub_stage nullable, new sub_stage_raw); any consumer assuming sub_stage is always one of three values could break, though current UI does not render it.

Overview
Fixes GH-2328 (checklist API contract): formation checklist reads no longer default unknown project stages to engaged.

deriveFormationSubStage is removed from the BFF mapper. mapUpstreamFormationChecklist now sets sub_stage via shared normalizeFormationSubStage (same helper as the formations queue), using the NATS project record’s stage because the upstream checklist payload has no stage field.

Formation gains sub_stage_raw (verbatim upstream stage) and sub_stage is FormationSubStage | null when the stage has no queue taxonomy (e.g. Active, Disengaged/Confidential). Fixtures, enum/component/e2e mocks, and server-helpers.md are updated accordingly.

New formation-mapper.helper.spec.ts and expanded formation.service.spec.ts cover real upstream stage strings, empty stage, and a constructor prototype guard so non-Formation stages never fall through to engaged.

^{Reviewed by Cursor Bugbot for commit 8aee0c0. Bugbot is set up for automated code reviews on this repo. Configure here.}

[manishdixitlfx]

Re-confirmed on the merged integration tree (this branch + #2374 + #2376, served against prod, 13 Sep) — the three changes compose without regression:

• Stage chips resolve: Formation · Engaged, Formation · On Hold.
• Queue tiles at root scope: 106 / 7 / 10, unmapped: 3, total 126 — fix(formation): normalize sub_stage at BFF boundary #2370's normalization intact.
• The three affected projects still return sub_stage: null with sub_stage_raw verbatim.

One merge conflict came up folding #2376 in, in formation.mock.ts — this branch's sub_stage_raw against #2376's date-only announcement_date, both on the same fixture object. Resolved by keeping both, which is right: they are independent fields and each PR needs its one.

An unrelated defect surfaced in the same session — #2378, the queue landing on 3 rows instead of 126 — which comes from #2369 and is already on main. Not caused by this PR, and not a reason to hold it.

[MRashad26]

Verified independently against origin/main and the pre-existing normalizeFormationSubStage helper (added by #2370, already tested/used by the queue path). The deleted deriveFormationSubStage did default every non-Formation/Disengaged/Confidential stage to 'engaged' exactly as described — confirmed by reading its old switch/default in the diff. The checklist mapper now sources from the same shared helper the queue uses, so the two screens can't disagree. sub_stage correctly widened to FormationSubStage | null without widening the union itself, and sub_stage_raw gives a future consumer the honest raw string to render instead of guessing. Tests use real upstream stage strings (not short keys) plus a constructor/prototype-collision guard — good given that short-key fixtures are exactly how this bug class reached production twice before.

Non-blocking nit: branch name fix/GH-2328-substage-derivation uses GH-XXX rather than this repo's documented issue-<number> convention (.claude/rules/commit-workflow.md) — same nit flagged on a couple of other PRs in this batch, just flagging for next time, not worth renaming.