[dev] [Marfuen] mariano/nist-sp800-53-readiness#2922
Conversation
Covers CS-389 (control family column), CS-390 (sort order), CS-391 (collapsed default view), CS-392 (expand all), CS-393 (remove duplicate requirements heading). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s fetch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Moves PAGE_SIZE_OPTIONS, ControlItem, getStatusBadge, buildRequirementMap, and buildControlItems into framework-controls-shared.ts so both the flat view and the upcoming grouped view can reuse them without duplication. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements grouped view of framework controls organized by control family with search, expand/collapse individual families, and expand all/collapse all toggle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ily data Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…editor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace EditableCell with a searchable ComboboxCell dropdown for the controlFamily column. Shows existing families from current data, supports filtering, and allows creating new families inline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Stronger bg-muted/60 on family header rows for clearer group separation - Add search input to family filter dropdown with auto-reset on close - Cap dropdown list at 256px with overflow scroll (search stays pinned) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Framework-editor uses @trycompai/ui, not @trycompai/design-system. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Dialog Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dialog Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…l on family delete Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mpact summary Shows framework summary, collapsible control list (capped at 5 preview), and "N more..." for large families. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Simpler layout: single summary line with count + frameworks, collapsible control list shows just names (no per-control frameworks), contained in a bordered inset panel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai re-review this |
@Marfuen I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 45 files
Confidence score: 3/5
- There is a concrete regression risk in
apps/api/src/frameworks/framework-versioning/framework-rollback.service.ts: usingcreateduring rollback can fail when control-family rows already exist, so rollback may not complete reliably without an idempotent restore approach (upsertorcreateMany+skipDuplicates). - The sorting issue in
docs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.mdis moderate impact:localeComparewithout{ numeric: true }can misorder identifiers likeAC-10vsAC-2, which affects correctness/readability but is less likely to break runtime behavior. - Pay close attention to
apps/api/src/frameworks/framework-versioning/framework-rollback.service.tsanddocs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.md- rollback idempotency and numeric control sorting should be validated before merge.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx">
<violation number="1" location="apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx:197">
P2: Deleting a family sets `controlFamily` to an empty string instead of `null`, which creates inconsistent "no family" states and can send unexpected payloads to the API.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…estore
- localeCompare now uses { numeric: true } so AC-2 sorts before AC-10
- Rollback uses upsert instead of create for deleted family restore
to avoid failure on existing rows
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai re-review this |
@Marfuen I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
3 issues found across 45 files
Confidence score: 3/5
- There is a concrete regression risk in
apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/framework-controls-shared.test.ts: the tests assertfamily: "Other"while implementation usesUNCATEGORIZED_FAMILY("__uncategorized__"), which can mask or introduce incorrect grouping behavior for uncategorized controls. docs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.mddescribes sorting withlocaleComparebut without numeric-aware options, so IDs like AC-10 vs AC-2 can be ordered incorrectly; this is user-visible if copied into implementation.- Overall this looks manageable, but the combination of a medium-high severity grouping mismatch and a numeric sorting pitfall makes this more than a low-risk merge.
- Pay close attention to
apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/components/framework-controls-shared.test.tsanddocs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.md- sentinel-vs-label assumptions and non-numeric sorting guidance could lead to inconsistent behavior.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx">
<violation number="1" location="apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx:197">
P2: Deleting a family sets `controlFamily` to an empty string instead of `null`, which creates inconsistent "no family" states and can send unexpected payloads to the API.</violation>
</file>
<file name="docs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.md">
<violation number="1" location="docs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.md:450">
P2: Control names with numeric suffixes sort incorrectly because `localeCompare` lacks the `{ numeric: true }` option. Without it, "AC-10" sorts before "AC-2" as a plain string comparison.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| ungrouped.push(...groupItems); | ||
| continue; | ||
| } | ||
| const sorted = groupItems.sort((a, b) => a.control.name.localeCompare(b.control.name)); |
There was a problem hiding this comment.
P2: Control names with numeric suffixes sort incorrectly because localeCompare lacks the { numeric: true } option. Without it, "AC-10" sorts before "AC-2" as a plain string comparison.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.md, line 450:
<comment>Control names with numeric suffixes sort incorrectly because `localeCompare` lacks the `{ numeric: true }` option. Without it, "AC-10" sorts before "AC-2" as a plain string comparison.</comment>
<file context>
@@ -0,0 +1,1057 @@
+ ungrouped.push(...groupItems);
+ continue;
+ }
+ const sorted = groupItems.sort((a, b) => a.control.name.localeCompare(b.control.name));
+ families.push({ name, displayName: name, items: sorted });
+ }
</file context>
…numeric sort - Tests now assert against UNCATEGORIZED_FAMILY constant instead of hardcoded 'Other' string - Design spec updated to reflect numeric-aware sorting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@cubic-dev-ai re-review this |
@Marfuen I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 45 files
Confidence score: 3/5
- There is a concrete user-impacting behavior in
apps/framework-editor/app/components/table/ComboboxCell.tsx: re-selecting the already selected option clears the value, which risks accidental data loss. - Given the moderate severity (5/10) and high confidence (8/10), this is more than a cosmetic issue and introduces some merge risk unless idempotent selection behavior is restored.
- Pay close attention to
apps/framework-editor/app/components/table/ComboboxCell.tsx- make selecting the current option a no-op and keep removal limited to the explicit Clear action.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx">
<violation number="1" location="apps/framework-editor/app/(pages)/controls/ControlsClientPage.tsx:197">
P2: Deleting a family sets `controlFamily` to an empty string instead of `null`, which creates inconsistent "no family" states and can send unexpected payloads to the API.</violation>
</file>
<file name="docs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.md">
<violation number="1" location="docs/superpowers/plans/2026-05-21-nist-sp800-53-controls-grouping.md:450">
P2: Control names with numeric suffixes sort incorrectly because `localeCompare` lacks the `{ numeric: true }` option. Without it, "AC-10" sorts before "AC-2" as a plain string comparison.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Clicking the already-selected option now closes the dropdown without clearing the value. Use the explicit Clear action to remove a family. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… selection change Clear now directly calls onUpdate with empty string instead of going through handleSelect (which returns early for the current value). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This is an automated pull request to merge mariano/nist-sp800-53-readiness into dev.
It was created by the [Auto Pull Request] action.
Summary by cubic
Adds control family grouping with per‑framework assignments and a grouped UI for large frameworks like NIST SP800‑53, with URL‑persisted filters and clearer change summaries. Includes numeric sorting, idempotent rollback, and a fix for the family combobox Clear action; covers Linear CS-389–CS-393.
New Features
UNCATEGORIZED_FAMILYbucket; numeric sort for control names.controlFamilycolumn with a searchable combobox (add/clear, idempotent selection; Clear now works reliably), plus a Manage Families dialog to rename/delete with batched updates; tests updated for the sentinel.controlFamilyincluded in manifests, diffs, export/import, sync apply, and rollback; change summaries show family edits; empty strings normalized to null; undo payload tracks family changes and rollback restores them idempotently; sync applies family updates even when controls are otherwise edited.Migration
FrameworkControlFamilytable scopes family per framework instance; data backfilled and migrations squashed; old undo payloads remain supported.Written for commit d0b9517. Summary will update on new commits. Review in cubic