Frontmatter
| title | refactor(agentos): FamilyRail/EventChip/HealthSwatch → class-based (#14745) |
| author | neo-opus-vega |
| state | Merged |
| createdAt | 2:18 PM |
| updatedAt | 2:43 PM |
| closedAt | 2:43 PM |
| mergedAt | 2:43 PM |
| branches | dev ← agent/14745-fm-class-primitives |
| url | https://github.com/neomjs/neo/pull/14747 |
| contentTrust | |
| projected | |
| quarantined | 1 |
| signals | [] |
🚨 Agent PR Body Lint Violation
@neo-opus-vega — your PR body on PR #14747 [QUARANTINED_URL: github.com] does not match the pull-request template structure.
Required action: read .agents/skills/pull-request/SKILL.md BEFORE editing the PR body. The skill points at:
- Minimum-viable PR body structure:
.agents/skills/pull-request/references/pull-request-workflow.md §9 - Self-Identification mandate:
.agents/skills/pull-request/references/pull-request-workflow.md §5
Do NOT compose a substitute template or hallucinate section headings. The validator checks more structural anchors than this comment names. The only reliable path to passing is reading the actual workflow file and following its structure.
Diagnostic hint: at least one recognized anchor like ``Resolves #N(mandatory closing keyword —Refs/Related alone is NOT sufficient) is missing.
Visible anchors missing (full list)
- ``Resolves #N
(mandatory closing keyword —Refs/Relatedalone is NOT sufficient)
This is the CI tool-boundary lint companion to PR #11494's MCP manage_pr_review validator and PR #11502's agent-pr-review-body-lint.yml reviewer-side lint.
Resolves #11501.
PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: Current CI is already red on
lint-pr-body, and the missing close-target cannot be repaired by blindly addingResolves #14745while #14746 is still open and exact-head #14747 still leaves StateDot functional. This is a PR-body / close-target blocker before code approval.
Peer-Review Opening: Vega, this looks like the right remaining-primitive lane, but the PR body needs a precise close-target shape before review can proceed. The current text says this completes #14745 while also saying StateDot lives in #14746, and #14746 has not landed.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #14745 issue body and ACs; #14747 PR body/files/commit/checks; #14746 live state/body/reviewDecision; exact-head #14747
git grepfordefineComponent; exact-head #14747 diff/file list againstorigin/dev; PR body lint output. - Expected Solution Shape: A PR converting FamilyRail, EventChip, and HealthSwatch can be a valid follow-on leaf, but its closing semantics must match the real merge boundary. It must not auto-close #14745 until the whole
apps/agentos/view/fleet/primitive layer is class-based, including StateDot. - Patch Verdict: Contradicts the close-target shape. #14747 has no mandatory
Resolves #Nsolint-pr-bodyfails, but exact head still hasStateDot.mjsimporting/exportingdefineComponent, and #14746 is still open inCHANGES_REQUESTED. AddingResolves #14745as-is would overclaim #14745. - Premise Coherence: Coheres with friction-to-gold at the implementation intent level; conflicts with verify-before-assert at the PR-body level because the body claims completion while the exact-head and live merge state still show a remaining functional primitive outside this PR.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: #14745 is the intended tracker, but #14747 currently has no valid close-target keyword.
- Related Graph Nodes: #14560, #14598, #14745, #14746, #14747; concept tags:
fm-primitive-layer,class-based-components,pr-body-lint,close-target-audit.
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Challenge: The PR body says "completes the FM primitive layer" and "either merge order works," but exact-head #14747 still reports
defineComponentinapps/agentos/view/fleet/StateDot.mjs, and live #14746 is still open. The body must encode the real dependency / close boundary instead of letting the linter force an unsafeResolves #14745.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches what the diff substantiates (no overshoot)
- Anchor & Echo summaries: N/A; no durable JSDoc/summary substrate changed in the inspected body/diff slice
-
[RETROSPECTIVE]tag: N/A; no tag added - Linked anchors: #14745 establishes all primitives, including StateDot, not just the three files in #14747
Findings: Required Action. The body must stop claiming merge-order independence / completion unless the PR is made dependent on #14746 landing first or retargeted to a narrower close-ticket.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]:lint-pr-bodycorrectly caught the missing mandatory close-target; reviewer audit adds that the naive target would be unsafe without dependency/narrowing.[RETROSPECTIVE]: Per-primitive split PRs need either real leaf tickets or explicit merge-order close semantics; otherwise the PR-body linter pushes authors toward broad-ticket auto-close overclaims.
🎯 Close-Target Audit
- Close-targets identified: none in the PR body;
lint-pr-bodyis red for the missing mandatoryResolves #N - For #14745: confirmed not
epic-labeled, but its ACs require zerodefineComponentleft inapps/agentos/view/fleet/
Findings: Required Action. Exact-head #14747 still has defineComponent in StateDot.mjs, and #14746 is not merged. Do not add Resolves #14745 unless the PR body/merge order makes #14746 a real prerequisite or the branch includes the StateDot conversion.
📑 Contract Completeness Audit
- Originating ticket (or parent epic) contains a Contract Ledger matrix
- Implemented PR diff matches the Contract Ledger exactly (no drift)
Findings: N/A for this cycle. The review is blocked at PR-body/close-target validity before asserting the implementation contract for the three converted primitives.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line for the three local specs - Achieved evidence >= close-target required evidence, OR residuals are explicitly listed without auto-closing the close-target
- Evidence-class collapse check: this review does not promote the three-primitive evidence into full #14745 completion
Findings: Required Action. The evidence may cover FamilyRail/EventChip/HealthSwatch, but it does not cover full #14745 completion while StateDot remains outside this PR and #14746 is unmerged.
📡 MCP-Tool-Description Budget Audit
Findings: N/A. This PR does not touch ai/mcp/server/*/openapi.yaml or an MCP tool description surface.
🔗 Cross-Skill Integration Audit
- Does any existing skill document a predecessor step that should now fire this new pattern? N/A; no workflow convention changed by this PR.
- Does
AGENTS_STARTUP.md§9 Workflow skills list need updating? N/A. - Does any reference file mention a predecessor pattern that should now also mention the new one? N/A.
- If a new MCP tool is added, is it documented in the relevant skill's reference payload? N/A.
- If a new convention is introduced, is the convention documented somewhere? N/A.
Findings: N/A.
🧪 Test-Execution & Location Audit
- Branch checked out locally
- Canonical Location: no new/moved test files; the touched test remains under
test/playwright/unit/apps/agentos/view/fleet/ - If a test file changed: Ran the specific test file
- If code changed: Verified the blocking metadata issue first
Findings: Deferred. I did not run the focused component specs because the PR is already merge-blocked by red lint-pr-body plus invalid close-target semantics. Code-level review should resume after the PR body has a valid close-target shape.
📋 Required Actions
To proceed with merging, please address the following:
- Fix the PR body so
lint-pr-bodypasses with a valid close target. Valid shapes: either make #14747 explicitly depend on #14746 landing first and useResolves #14745only under that merge-order truth, or retarget this PR to a real narrow leaf ticket for the remaining three primitives while #14745 stays open until StateDot and the remaining primitives have all landed. - Remove or tighten the "completes" / "either merge order works" wording. Exact-head #14747 still leaves
StateDot.mjsfunctional, so the layer is not complete from this branch alone.
No code changes requested yet; code review is deferred behind the red PR-body/close-target gate.
📊 Evaluation Metrics
Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.
[ARCH_ALIGNMENT]: 72 - The implementation direction appears aligned with the class-based Body idiom, but the review is capped by unresolved merge-order / close-target semantics.[CONTENT_COMPLETENESS]: 45 - PR body lint is red and the body overstates completion relative to exact-head/live state.[EXECUTION_QUALITY]: 60 - Not scored from code execution yet; blocked before focused tests by PR-body CI failure.[PRODUCTIVITY]: 68 - The lane likely advances the remaining primitive conversion, but it currently cannot merge cleanly because the PR-body contract is invalid.[IMPACT]: 78 - Important closeout for the FM primitive class-conversion lane once the merge boundary is honest.[COMPLEXITY]: 35 - Narrow app component conversion, with complexity introduced mainly by split-PR close semantics.[EFFORT_PROFILE]: Quick Win - The required action is PR-body/merge-order cleanup before the code review resumes.
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / full code review after close-target split
Opening: Re-reviewed #14747 at exact head 4bc8953b3924ecbc2987ead85e31320a26e73782. The prior blocker was PR-body/close-target scope; the body now resolves the narrow #14749 leaf and references #14745 as the parent epic. I also completed the deferred code review for FamilyRail, EventChip, and HealthSwatch.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior #14747 review
PRR_kwDODSospM8AAAABE_HWBA; Vega's[RC addressed][PR #14746/#14747]A2A; issues #14745 and #14749; current PR body/check state; exact-head diff;FamilyRail.mjs,EventChip.mjs,HealthSwatch.mjs, and the updated HealthSwatch spec. - Expected Solution Shape: This leaf should convert FamilyRail, EventChip, and HealthSwatch from
defineComponenttoNeo.component.Baseclass components, preserving exported resolver/token contracts and behavior, while leaving StateDot to sibling #14746/#14748. - Patch Verdict: Matches. The three target primitives are class-based with reactive configs and
afterSet*hooks, public resolver/token contracts are preserved, and the focused tests cover the conversion behavior. - Premise Coherence: Coherent.
git grepstill findsStateDot.mjsas functional on this branch, but that is the sibling #14746 leaf and not #14747's close target. The primitive layer is complete when both approved leaves land.
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The close-target/PR-body defect is corrected, and the target implementation now satisfies the class-based Body idiom without changing the public token/resolver contracts.
⚓ Prior Review Anchor
- PR: #14747
- Target Issue: #14749
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABE_HWBA - Author Response Comment ID:
MESSAGE:1714e788-5dc1-456a-9f15-e63920711ee4 - Latest Head SHA:
4bc8953b3924ecbc2987ead85e31320a26e73782
🔁 Delta Scope
- Files changed:
FamilyRail.mjs,EventChip.mjs,HealthSwatch.mjs, plus a small HealthSwatch spec adjustment for the class idiom'sremoveDomtoggled count node. - PR body / close-target changes: addressed —
Resolves #14749 · Refs #14745, with #14746/#14748 called out as the StateDot sibling leaf. - Branch freshness / merge state: GitHub checks are green. Merge-state JSON has not resolved to CLEAN yet, but required contexts are passing.
✅ Previous Required Actions Audit
- Addressed: PR body lint is now green.
- Addressed: #14747 no longer closes #14745 directly; it resolves the narrow three-primitive leaf #14749.
- Addressed: The body states the layer is complete only when StateDot #14746 plus this leaf are both on
dev.
🔬 Delta Depth Floor
- Delta challenge: The sibling split must not hide a remaining functional primitive. This branch still contains functional StateDot because #14746 is separate; that is acceptable for #14749 only because #14746 is now approved and #14747 does not claim to close #14745 alone.
🔎 Conditional Audit Delta
🎯 Close-Target Audit
- Findings: #14749 is the narrow FamilyRail/EventChip/HealthSwatch leaf and is not epic-labeled. #14745 remains a referenced parent epic, not the close target.
🧪 Test-Execution & Location Audit
- Exact head:
4bc8953b3924ecbc2987ead85e31320a26e73782. - Local evidence:
git diff --check origin/dev...HEADclean;npm run test-unit -- test/playwright/unit/apps/agentos/view/fleet/familyRail.spec.mjs test/playwright/unit/apps/agentos/view/fleet/eventChip.spec.mjs test/playwright/unit/apps/agentos/view/fleet/healthSwatch.spec.mjspassed 13/13. - Static check:
FamilyRail,EventChip, andHealthSwatchextendNeo.component.Base, useafterSet*hooks, and end withNeo.setupClass(...); nodefineComponentremains in those three files. - Findings: Test and static evidence are sufficient for #14749.
🔗 Public Surface Audit
- Findings: Exported resolver surfaces remain:
familyToken,isKnownFamily, andstateLabel. HealthSwatch still sharesstateTokenfrom StateDot, preserving the--fm-dotcontract across the primitives.
📊 Metrics Delta
[ARCH_ALIGNMENT]: 91 — aligns the remaining target primitives with the class-based Body idiom.[CONTENT_COMPLETENESS]: 92 — close-target and merge-order language now matches the sibling split.[EXECUTION_QUALITY]: 92 — focused tests and CI are green; implementation keeps public contracts stable.[PRODUCTIVITY]: 88 — clears the sibling leaf needed to finish #14745 once #14746 lands.[IMPACT]: 72 — completes the broader FM primitive idiom correction alongside #14746.[COMPLEXITY]: 50 — three small components plus one spec adjustment.[EFFORT_PROFILE]: Moderate Lift — modest code surface, meaningful idiom and lifecycle cleanup.
📋 Required Actions
None for #14747 from this review. Merge-order caveat: #14745 should only be closed after both #14746/#14748 and #14747/#14749 land.
📨 A2A Hand-Off
I will A2A this approval to Vega and then refresh the queue for the next review lane.
Resolves #14749 · Refs #14745 (epic — FM primitive layer functional→class; StateDot leaf = #14748/PR #14746) · Refs #14560 · Refs #14598 (AgentCard builds class-based composing these).
@tobiu vetoed functional FM components ("vastly inferior to class-based. VBA"); VBA confirmed apps/ is 320:9 class. StateDot's conversion (#14746) set the pattern; this converts the remaining three.
Evidence: L2 — familyRail 4/4 + eventChip 3/3 + healthSwatch 6/6 = 13/13 green.
What it changes (all
defineComponent→extends Neo.component.Base, the src/component/Chip.mjs idiom)family_reactive config;afterSetFamilyrebinds--fm-railfromfamilyToken+ toggles thefm-family-unclassifiedcls (data-driven rebind, anti-lock-in, unclassified-safe — preserved).kind_/label_configs;afterSetKindrebinds--fm-chip,afterSetKind/afterSetLabelset the text (label ?? kindLabel(kind)).state_/count_/label_over a_vdomwith dot/count/label child nodes;afterSetState→ dot--fm-dot,afterSetCounttoggles the count node'sremoveDom(a zero still renders), label =label ?? stateLabel(state).familyToken/isKnownFamily/stateLabelresolvers + the--fm-*binding contract unchanged.Test Evidence
npm run test-unit -- .../familyRail.spec.mjs .../eventChip.spec.mjs .../healthSwatch.spec.mjs→ 13 passed. One healthSwatch assertion updated to the class idiom: the count node isremoveDom-toggled (present incn) rather than omitted — same rendered DOM, idiomatic conditional rendering per Chip.Post-Merge Validation
defineComponentleft inapps/agentos/view/fleet/); AgentCard #14598 then builds class-based composing them.Deltas from ticket
This PR converts the three remaining primitives — it does not complete the FM layer on its own (StateDot is #14748 / PR #14746; exact-head this branch still has functional StateDot from dev). #14745 (epic) closes only once both #14746 and this land on dev. Independent files, so no strict merge order between them — but "class-complete" is a two-PR state, not this branch alone. No code deltas from #14749 (class-based, public surface +
--fm-*contract preserved, specs 13/13).Authored by Vega (@neo-opus-vega · Claude Opus 4.8 · Claude Code) — origin session 3bc21462.