Skip to content

[VW-354] Issue/Note model changes; VEX sorting agent#150

Open
0xcad wants to merge 13 commits into
mainfrom
VW-354
Open

[VW-354] Issue/Note model changes; VEX sorting agent#150
0xcad wants to merge 13 commits into
mainfrom
VW-354

Conversation

@0xcad

@0xcad 0xcad commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator
  • Modified Issue model per CDST IV&V RFC -- now include VEX status fields, asset-level issues override device-group-matching-level issues
  • Created new Note model and EntityFilter models per CDST IV&V RFC -- notes are more structured ways of storing information about an asset/vulns/remediation, etc
  • New vex agent sorts vulnerabilities into UNDER_INVESTIGATION, AFFECTED, NOT_AFFECTED

Summary by CodeRabbit

  • New Features

    • Added issue notes and filtering support, including richer status options and clearer device-group vs. asset scoping.
    • Inbox processing now automatically sorts vulnerability notifications and applies the results.
    • Notification seeding includes an additional real-world scenario for better coverage.
  • Bug Fixes

    • Issues can now exist without a direct asset, with views and exports updated to handle that case gracefully.
    • Dashboard, advisory, and asset counts now use the updated issue status labels consistently.

@vercel

vercel Bot commented Jul 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
viper Ready Ready Preview, Comment Jul 6, 2026 11:24pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
viper-demo Ignored Ignored Jul 6, 2026 11:24pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@0xcad, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c89e86eb-f5f9-426f-9aff-cde2a3dadea1

📥 Commits

Reviewing files that changed from the base of the PR and between 40c404b and 7663869.

📒 Files selected for processing (2)
  • src/features/inbox/agent/vex/context.ts
  • src/features/notes/server/get-relevant-notes.ts
📝 Walkthrough

Walkthrough

This PR restructures issue status semantics (replacing ACTIVE/FALSE_POSITIVE/REMEDIATED with AFFECTED/NOT_AFFECTED/FIXED/UNDER_INVESTIGATION), makes Issue asset scoping optional in favor of device-group scoping, adds Note/EntityFilter models, and introduces a new VEX sorting agent that determines issue statuses from inbox notifications and writes deterministic updates/overrides.

Changes

Schema, migration, and status consumers

Layer / File(s) Summary
Prisma schema updates
prisma/schema.prisma
New IssueStatus, NotAffectedJustification, NoteStatus, ScopeTargetModel enums; Issue.assetId/deviceGroupMatchingId made optional; Note, EntityFilter, EntityFilterMatch models added with relations to User, DeviceGroupMatching, Issue.
SQL migration
prisma/migrations/20260701212209_issue_and_notes/migration.sql
Creates new enums, remaps advisory.status/issue.status values via CASE expressions, adds note/entity_filter/entity_filter_match tables, indexes, and foreign keys.
Status consumers across UI/routers/tests
src/components/status-form.tsx, src/features/advisories/*, src/features/assets/*, src/features/issues/*, src/features/chat/*, src/features/vulnerabilities/*, src/features/tracking/types.ts, src/app/api/v1/__tests__/vulnerabilities.test.ts
Status mappings and count/filter logic updated to new enum values; components/routers handle nullable issue.asset with "Device Group" fallbacks.
Baseline issue creation and seed updates
src/lib/prisma-client-extensions.ts, prisma/seed.ts, scripts/seed-icsma-24-319-01.ts
Vulnerability creation now generates one issue per device-group matching instead of per-asset; seedIssues() removed from seed.ts; advisory seed status updated to AFFECTED.

Estimated code review effort: 4 (Complex) | ~75 minutes

VEX Sorting Agent for Inbox Notifications

Layer / File(s) Summary
Tool schema and result types
src/features/inbox/agent/vex/tools.ts
Zod schemas define VEX determination output shape (status, justification, asset overrides) plus TypeScript "loose" types.
Context gathering and notes
src/features/inbox/agent/vex/context.ts, src/features/notes/server/get-relevant-notes.ts
gatherVexContext builds notification/vulnerability/device-group data into a markdown prompt; getRelevantNotes fetches persistent and scoped notes.
LLM invocation and write planning
src/features/inbox/agent/vex/index.ts, src/features/inbox/agent/vex/process_output.ts, src/features/inbox/agent/vex/__tests__/vex.test.ts
Invokes Claude with a bound tool, parses output, plans deterministic issue/asset writes, and applies them via a Prisma transaction; unit tests cover planVexWrites.
Pipeline wiring and seed scenario
src/inngest/functions/process-inbox-email.ts, scripts/seed-notifications.ts, src/lib/string-utils.ts, src/test/server-only-stub.ts, vitest.config.mts
Inbox pipeline adds a VEX sorting step; a new seed scenario exercises the flow; markdown helpers and test infra added.

Estimated code review effort: 4 (Complex) | ~70 minutes

Sequence Diagram(s)

sequenceDiagram
    participant Inngest as processInboxEmail
    participant Context as gatherVexContext
    participant Agent as sortVulnerabilities (Claude)
    participant Planner as planVexWrites
    participant DB as Prisma / Database

    Inngest->>Context: gatherVexContext(notificationId)
    Context->>DB: fetch notification, vulnerabilities, matchings, notes
    DB-->>Context: notification data
    Context-->>Inngest: VexContext (markdown, issues)
    Inngest->>Agent: sortVulnerabilities(context)
    Agent->>Agent: invoke Claude with tool binding
    Agent-->>Inngest: VexResult
    Inngest->>Planner: planVexWrites(context, result)
    Planner-->>Inngest: issueUpdates, assetOverrides
    Inngest->>DB: applyVexDeterminations ($transaction)
    DB-->>Inngest: VexApplySummary
    Inngest-->>Inngest: return { vexSummary }
Loading

Possibly related PRs

  • PATCH-UPGRADE/viper#28: Both modify the Issue/IssueStatus domain model and related UI/routers, with this PR expanding the earlier status set and adding scoping/notes.
  • PATCH-UPGRADE/viper#49: This PR removes seedIssues() while PR #49 previously added/rewired it, directly overlapping seed logic.
  • PATCH-UPGRADE/viper#79: Both touch src/features/assets/components/dashboard-columns.tsx for issue-status-based counting logic.

Suggested reviewers: perrydev17

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main schema and VEX-agent changes without unnecessary noise.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch VW-354

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/features/advisories/server/routers.ts (1)

78-84: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

progressPercent now counts UNDER_INVESTIGATION as resolved.

status !== IssueStatus.AFFECTED treats UNDER_INVESTIGATION (a non-terminal, unresolved state) as progress, inflating the percentage. The previous logic excluded only the "active" state and counted genuinely resolved issues. Prefer an explicit allowlist of terminal states.

🐛 Proposed fix
-      const nonActiveCount = allIssues.filter(
-        (i) => i.status !== IssueStatus.AFFECTED,
-      ).length;
+      const RESOLVED_STATUSES = [
+        IssueStatus.FIXED,
+        IssueStatus.NOT_AFFECTED,
+      ] as const;
+      const nonActiveCount = allIssues.filter((i) =>
+        RESOLVED_STATUSES.includes(i.status),
+      ).length;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/advisories/server/routers.ts` around lines 78 - 84,
`progressPercent` is currently inflated because the `nonActiveCount` filter in
the advisories router treats `UNDER_INVESTIGATION` as resolved; update the logic
to use an explicit allowlist of terminal/resolved statuses instead of checking
`status !== IssueStatus.AFFECTED`. Locate the calculation in the advisories
server router and adjust the `allIssues.filter(...)` predicate so only genuinely
completed states contribute to `progressPercent`.
src/features/issues/components/issue.tsx (1)

106-106: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Avoid deriving the asset link from issues[0]. In src/features/issues/components/issue.tsx:106, 209, 225, type="assets" lists can mix asset-scoped and device-group-scoped issues; if the first item has no assetId, the overflow and “Non-Active Issues” sections disappear even when later items have one. Pick a non-null asset-scoped id from the list, or derive these links from the current asset context.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/issues/components/issue.tsx` at line 106, The asset link is
currently derived from issues[0], which can be null for mixed asset/device-group
issue lists and breaks the overflow and “Non-Active Issues” sections. Update the
issue component logic around the assetId derivation and the related link
builders in issue.tsx to use a non-null asset-scoped id from the list, or fall
back to the current asset context instead of assuming the first item has an
assetId.
src/features/advisories/components/advisories.tsx (1)

195-209: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Rename the gray bucket or split out UNDER_INVESTIGATION src/features/advisories/components/advisories.tsx:195-209

active is the catch-all remainder, so it includes both AFFECTED and UNDER_INVESTIGATION while being labeled "Active". If this chart is meant to show status breakdown, give UNDER_INVESTIGATION its own bucket; otherwise rename the label to something like "Open" or "Remaining".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/advisories/components/advisories.tsx` around lines 195 - 209,
The gray “Active” bucket in advisories.tsx is a catch-all remainder, so it mixes
AFFECTED and UNDER_INVESTIGATION under one label. Update the status breakdown
logic around the remediated/falsePos/active calculations to either split
UNDER_INVESTIGATION into its own count and legend item, or rename the existing
active bucket and label to something like Open/Remaining so the chart matches
the actual statuses represented.
src/features/assets/components/asset.tsx (1)

208-235: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fragile positional coupling between enum order and results array.

The tab count/order correctness depends entirely on the literal array [naResult, aResult, rResult, uiResult] matching Object.values(IssueStatus) runtime order. The comment documents this, but it's a silent-breakage trap: if IssueStatus in schema.prisma is ever reordered (e.g., alphabetized, or a new status inserted in the middle), tab labels, per-tab items, and pagination keys will silently mismatch with no compile-time or runtime error — only a UI bug caught by a human noticing wrong data under a tab.

Consider keying results by status value in a Record<IssueStatus, ...> instead of relying on array position, which removes the ordering dependency entirely:

♻️ Suggested refactor using a status-keyed map
-  const naResult = useSuspenseIssuesByAssetId({
-    assetId,
-    issueStatus: IssueStatus.NOT_AFFECTED,
-  });
-  const aResult = useSuspenseIssuesByAssetId({
-    assetId,
-    issueStatus: IssueStatus.AFFECTED,
-  });
-  const rResult = useSuspenseIssuesByAssetId({
-    assetId,
-    issueStatus: IssueStatus.FIXED,
-  });
-  const uiResult = useSuspenseIssuesByAssetId({
-    assetId,
-    issueStatus: IssueStatus.UNDER_INVESTIGATION,
-  });
-
-  // Order must match Object.values(IssueStatus) (NOT_AFFECTED, AFFECTED,
-  // FIXED, UNDER_INVESTIGATION) since results are indexed positionally below.
-  const results: PaginatedResponse<{ vulnerability: Vulnerability } & Issue>[] =
-    [];
-  let showTabs = false;
-  for (const res of [naResult, aResult, rResult, uiResult]) {
-    results.push(res.data);
-    if (res.data.totalCount > 0) {
-      showTabs = true;
-    }
-  }
+  const resultsByStatus = Object.fromEntries(
+    Object.values(IssueStatus).map((status) => [
+      status,
+      useSuspenseIssuesByAssetId({ assetId, issueStatus: status }),
+    ]),
+  ) as Record<IssueStatus, ReturnType<typeof useSuspenseIssuesByAssetId>>;
+
+  const showTabs = Object.values(resultsByStatus).some(
+    (res) => res.data.totalCount > 0,
+  );

Note: calling hooks inside .map violates the rules of hooks, so this specific transform isn't directly valid — the safer fix is to keep the four explicit hook calls but assemble the lookup structure afterward, e.g. const resultsByStatus = { [IssueStatus.NOT_AFFECTED]: naResult, ... }, then reference resultsByStatus[status] in the render loops below instead of results[i].

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/assets/components/asset.tsx` around lines 208 - 235, The asset
tabs logic in asset.tsx relies on fragile positional matching between the
explicit issue queries (naResult, aResult, rResult, uiResult) and the runtime
order of IssueStatus. Keep the four useSuspenseIssuesByAssetId calls, but
replace the positional results array with a status-keyed lookup such as a
Record<IssueStatus, PaginatedResponse<{ vulnerability: Vulnerability } & Issue>>
built from those results. Update the tab/rendering code that currently indexes
into results to read from that lookup by IssueStatus so ordering changes in
IssueStatus cannot silently break tab data.
🧹 Nitpick comments (9)
src/features/inbox/agent/vex/process_output.ts (1)

104-141: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Interactive transaction with sequential round trips may hit the default timeout.

Each issueUpdates/assetOverrides entry is a separate awaited round trip inside a single interactive $transaction. With many baseline issues per notification this can exceed Prisma's default interactive-transaction timeout (~5s), aborting the whole batch. Consider raising timeout (and maxWait) on the transaction, or batching writes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/inbox/agent/vex/process_output.ts` around lines 104 - 141, The
interactive Prisma transaction in process_output.ts can time out because
issueUpdates and assetOverrides are applied one-by-one with awaited round trips
inside prisma.$transaction. Update the transaction in the VEX output handling
path to either set a higher timeout/maxWait on prisma.$transaction or batch the
writes so the sequential updates in the issueUpdates and assetOverrides loops do
not exceed the default interactive-transaction limit.
src/features/inbox/agent/vex/index.ts (1)

42-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider logging silent failure paths.

Both if (!call) return {} and the safeParse failure branch silently return an empty result, so a missing/malformed tool call produces zero writes with no signal. Since this agent drives DB determinations, a console.warn (including parsed.error) would aid debugging without changing behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/inbox/agent/vex/index.ts` around lines 42 - 46, The Vex tool
result parsing in the agent flow is silently swallowing two failure paths:
missing tool call and schema parse failure. Update the logic around the
TOOL_NAME lookup and schema.safeParse in the Vex index handler to emit a
console.warn when no call is found and when parsing fails, including
parsed.error for the malformed payload case, while still returning an empty
result to preserve behavior.
prisma/schema.prisma (2)

689-711: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoff

EntityFilter "exactly one of noteId/issueId" invariant is unenforced.

Line 699 documents the invariant, but nothing prevents both (or neither) being set. Since Prisma can't express XOR constraints, add a raw CHECK in the migration (e.g. num_nonnulls("noteId","issueId") = 1) or enforce it in the write path so filters can't be silently orphaned or double-owned.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@prisma/schema.prisma` around lines 689 - 711, The EntityFilter model
documents an XOR ownership rule for noteId and issueId, but Prisma schema alone
does not enforce it. Add enforcement in the EntityFilter write path or the
database migration so only one of noteId/issueId can be set at a time and
neither/both are rejected; use the EntityFilter model and its create/update
flows as the places to apply the validation, and add a raw CHECK constraint in
the migration if you want database-level protection.

623-653: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoff

Issue allows both assetId and deviceGroupMatchingId to be null.

With both scoping identifiers optional and Postgres treating NULLs as distinct in unique constraints, a row can be created with neither set (orphan issue that neither unique constraint binds). The baseline extension always sets deviceGroupMatchingId and the VEX override sets assetId, so this is only reachable via a bad write, but the schema does not encode the "exactly one scope" invariant. Consider a DB CHECK ((assetId IS NOT NULL) OR (deviceGroupMatchingId IS NOT NULL)) (via raw SQL in the migration) or documenting/validating it at the app layer.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@prisma/schema.prisma` around lines 623 - 653, The Issue model currently
allows both assetId and deviceGroupMatchingId to be null, so the schema does not
enforce the intended scope invariant. Update the Prisma schema and migration
path around Issue to prevent orphan rows by enforcing that at least one scope is
present, ideally via a database CHECK constraint added in the migration for the
Issue table. If you prefer app-level enforcement, also add validation where
Issue records are created or updated, using the Issue model fields assetId and
deviceGroupMatchingId to reject writes that leave both unset.
prisma/migrations/20260701212209_issue_and_notes/migration.sql (1)

127-151: 🧹 Nitpick | 🔵 Trivial

Consider lock-safe DDL for production rollout.

On a large/live DB this migration takes blocking locks: the enum type rewrites (Lines 25-32, 46-53) hold ACCESS EXCLUSIVE, the CREATE UNIQUE INDEX/CREATE INDEX block writes, and the added FKs take SHARE ROW EXCLUSIVE with a full validating scan. For zero/low-downtime deploys, prefer CREATE INDEX CONCURRENTLY and add FKs as NOT VALID followed by a separate VALIDATE CONSTRAINT. If these tables are small or a maintenance window is used, this is fine as-is.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@prisma/migrations/20260701212209_issue_and_notes/migration.sql` around lines
127 - 151, The migration block creating indexes and foreign keys on issue, note,
entity_filter, and entity_filter_match can take blocking locks during rollout.
Update the CREATE INDEX / CREATE UNIQUE INDEX statements in this migration to
use lock-safe concurrent creation where supported, and add the foreign keys with
NOT VALID then validate them in a separate step. Use the existing constraint and
index names such as issue_deviceGroupMatchingId_vulnerabilityId_key,
issue_deviceGroupMatchingId_fkey, note_userId_fkey, entity_filter_noteId_fkey,
entity_filter_issueId_fkey, and entity_filter_match_entityFilterId_fkey to keep
the changes localized.

Source: Linters/SAST tools

src/features/vulnerabilities/components/prioritized-columns.tsx (1)

119-120: 📐 Maintainability & Code Quality | 🔵 Trivial

TODO left in code regarding backend display behavior for device-group issues.

Would you like me to help scope this backend change (one issue-per-asset display for device-group issues) or open a tracking issue for it?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/vulnerabilities/components/prioritized-columns.tsx` around lines
119 - 120, Remove the lingering TODO in prioritized-columns.tsx and either
implement or explicitly defer the device-group issue display behavior in the
relevant component logic, likely around the prioritized columns rendering and
issue mapping. If this behavior is not being changed now, replace the comment
with a short actionable note that points to the backend/display follow-up, using
the existing feature/component context in prioritized-columns.tsx to keep the
intent clear.
src/lib/string-utils.ts (1)

83-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Minor inconsistent empty-value fallback text in deviceIdentityInline.

When fields.cpe is an array, an empty array renders as "(none)"; when fields.cpe is a nullish scalar, it renders as "?". Both represent "no CPE known" but use different placeholder text, which could look inconsistent across rendered device identity lines.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/string-utils.ts` around lines 83 - 93, The `deviceIdentityInline`
helper uses inconsistent placeholders for missing `cpe` values, showing “(none)”
for an empty array but “?” for nullish scalar input. Update the
`deviceIdentityInline` logic to use a single fallback string for all empty or
missing `cpe` cases so the rendered identity text is consistent across array and
scalar inputs.
src/features/chat/viper-agent/tools/get-recommendations-context.ts (1)

33-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Raw string literal "AFFECTED" instead of IssueStatus.AFFECTED enum.

Elsewhere in this cohort (asset.tsx, dashboard-columns.tsx, assets/server/routers.ts) status comparisons use the IssueStatus enum constant for type safety and to avoid typo risk. Here the comparison uses a raw string literal. If AssetForContext.issues[].status is typed as IssueStatus, importing and using IssueStatus.AFFECTED would be more consistent and typo-safe.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/chat/viper-agent/tools/get-recommendations-context.ts` around
lines 33 - 35, The status check in get-recommendations-context should use the
IssueStatus enum instead of a raw string literal. Update the filter inside the
active count logic to compare against IssueStatus.AFFECTED, and import
IssueStatus in this module so the comparison stays type-safe and consistent with
asset.tsx, dashboard-columns.tsx, and assets/server/routers.ts.
src/features/assets/components/asset.tsx (1)

266-266: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Stale fallback text "No Active Issues".

With four statuses now shown (including NOT_AFFECTED and UNDER_INVESTIGATION), the empty-state message "No Active Issues" is misleading since it implies only the AFFECTED/active category was checked.

💬 Suggested copy fix
-        <p className="flex justify-center pt-24">No Active Issues</p>
+        <p className="flex justify-center pt-24">No Issues found</p>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/assets/components/asset.tsx` at line 266, Update the empty-state
copy in the asset view so it no longer says “No Active Issues,” since the status
list now includes more than just active/affected items. Adjust the fallback
message in the asset component (the JSX around the status/empty-state render in
asset.tsx) to a neutral, broader message that matches the four-status view and
does not imply only one category was checked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/features/notes/server/get-relevant-notes.ts`:
- Around line 90-105: The asset note lookup path is doing an N+1 query by
calling getNotesForAsset once per scope.assetIds entry inside getRelevantNotes.
Replace the Promise.all(map(getNotesForAsset)) pattern with a single batched
lookup using getNotesForInstance("ASSET", assetIds) for all asset IDs at once,
then flatten/group results as needed. Keep the existing behavior for persistent,
vulnerabilities, remediations, and matchings unchanged, and only touch the asset
branch in getRelevantNotes/getNotesForAsset.

---

Outside diff comments:
In `@src/features/advisories/components/advisories.tsx`:
- Around line 195-209: The gray “Active” bucket in advisories.tsx is a catch-all
remainder, so it mixes AFFECTED and UNDER_INVESTIGATION under one label. Update
the status breakdown logic around the remediated/falsePos/active calculations to
either split UNDER_INVESTIGATION into its own count and legend item, or rename
the existing active bucket and label to something like Open/Remaining so the
chart matches the actual statuses represented.

In `@src/features/advisories/server/routers.ts`:
- Around line 78-84: `progressPercent` is currently inflated because the
`nonActiveCount` filter in the advisories router treats `UNDER_INVESTIGATION` as
resolved; update the logic to use an explicit allowlist of terminal/resolved
statuses instead of checking `status !== IssueStatus.AFFECTED`. Locate the
calculation in the advisories server router and adjust the
`allIssues.filter(...)` predicate so only genuinely completed states contribute
to `progressPercent`.

In `@src/features/assets/components/asset.tsx`:
- Around line 208-235: The asset tabs logic in asset.tsx relies on fragile
positional matching between the explicit issue queries (naResult, aResult,
rResult, uiResult) and the runtime order of IssueStatus. Keep the four
useSuspenseIssuesByAssetId calls, but replace the positional results array with
a status-keyed lookup such as a Record<IssueStatus, PaginatedResponse<{
vulnerability: Vulnerability } & Issue>> built from those results. Update the
tab/rendering code that currently indexes into results to read from that lookup
by IssueStatus so ordering changes in IssueStatus cannot silently break tab
data.

In `@src/features/issues/components/issue.tsx`:
- Line 106: The asset link is currently derived from issues[0], which can be
null for mixed asset/device-group issue lists and breaks the overflow and
“Non-Active Issues” sections. Update the issue component logic around the
assetId derivation and the related link builders in issue.tsx to use a non-null
asset-scoped id from the list, or fall back to the current asset context instead
of assuming the first item has an assetId.

---

Nitpick comments:
In `@prisma/migrations/20260701212209_issue_and_notes/migration.sql`:
- Around line 127-151: The migration block creating indexes and foreign keys on
issue, note, entity_filter, and entity_filter_match can take blocking locks
during rollout. Update the CREATE INDEX / CREATE UNIQUE INDEX statements in this
migration to use lock-safe concurrent creation where supported, and add the
foreign keys with NOT VALID then validate them in a separate step. Use the
existing constraint and index names such as
issue_deviceGroupMatchingId_vulnerabilityId_key,
issue_deviceGroupMatchingId_fkey, note_userId_fkey, entity_filter_noteId_fkey,
entity_filter_issueId_fkey, and entity_filter_match_entityFilterId_fkey to keep
the changes localized.

In `@prisma/schema.prisma`:
- Around line 689-711: The EntityFilter model documents an XOR ownership rule
for noteId and issueId, but Prisma schema alone does not enforce it. Add
enforcement in the EntityFilter write path or the database migration so only one
of noteId/issueId can be set at a time and neither/both are rejected; use the
EntityFilter model and its create/update flows as the places to apply the
validation, and add a raw CHECK constraint in the migration if you want
database-level protection.
- Around line 623-653: The Issue model currently allows both assetId and
deviceGroupMatchingId to be null, so the schema does not enforce the intended
scope invariant. Update the Prisma schema and migration path around Issue to
prevent orphan rows by enforcing that at least one scope is present, ideally via
a database CHECK constraint added in the migration for the Issue table. If you
prefer app-level enforcement, also add validation where Issue records are
created or updated, using the Issue model fields assetId and
deviceGroupMatchingId to reject writes that leave both unset.

In `@src/features/assets/components/asset.tsx`:
- Line 266: Update the empty-state copy in the asset view so it no longer says
“No Active Issues,” since the status list now includes more than just
active/affected items. Adjust the fallback message in the asset component (the
JSX around the status/empty-state render in asset.tsx) to a neutral, broader
message that matches the four-status view and does not imply only one category
was checked.

In `@src/features/chat/viper-agent/tools/get-recommendations-context.ts`:
- Around line 33-35: The status check in get-recommendations-context should use
the IssueStatus enum instead of a raw string literal. Update the filter inside
the active count logic to compare against IssueStatus.AFFECTED, and import
IssueStatus in this module so the comparison stays type-safe and consistent with
asset.tsx, dashboard-columns.tsx, and assets/server/routers.ts.

In `@src/features/inbox/agent/vex/index.ts`:
- Around line 42-46: The Vex tool result parsing in the agent flow is silently
swallowing two failure paths: missing tool call and schema parse failure. Update
the logic around the TOOL_NAME lookup and schema.safeParse in the Vex index
handler to emit a console.warn when no call is found and when parsing fails,
including parsed.error for the malformed payload case, while still returning an
empty result to preserve behavior.

In `@src/features/inbox/agent/vex/process_output.ts`:
- Around line 104-141: The interactive Prisma transaction in process_output.ts
can time out because issueUpdates and assetOverrides are applied one-by-one with
awaited round trips inside prisma.$transaction. Update the transaction in the
VEX output handling path to either set a higher timeout/maxWait on
prisma.$transaction or batch the writes so the sequential updates in the
issueUpdates and assetOverrides loops do not exceed the default
interactive-transaction limit.

In `@src/features/vulnerabilities/components/prioritized-columns.tsx`:
- Around line 119-120: Remove the lingering TODO in prioritized-columns.tsx and
either implement or explicitly defer the device-group issue display behavior in
the relevant component logic, likely around the prioritized columns rendering
and issue mapping. If this behavior is not being changed now, replace the
comment with a short actionable note that points to the backend/display
follow-up, using the existing feature/component context in
prioritized-columns.tsx to keep the intent clear.

In `@src/lib/string-utils.ts`:
- Around line 83-93: The `deviceIdentityInline` helper uses inconsistent
placeholders for missing `cpe` values, showing “(none)” for an empty array but
“?” for nullish scalar input. Update the `deviceIdentityInline` logic to use a
single fallback string for all empty or missing `cpe` cases so the rendered
identity text is consistent across array and scalar inputs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df3f8d55-9f83-41e2-b451-3236219dcdbb

📥 Commits

Reviewing files that changed from the base of the PR and between 62e1ede and 40c404b.

📒 Files selected for processing (31)
  • prisma/migrations/20260701212209_issue_and_notes/migration.sql
  • prisma/schema.prisma
  • prisma/seed.ts
  • scripts/seed-icsma-24-319-01.ts
  • scripts/seed-notifications.ts
  • src/app/api/v1/__tests__/vulnerabilities.test.ts
  • src/components/status-form.tsx
  • src/features/advisories/components/advisories.tsx
  • src/features/advisories/server/routers.ts
  • src/features/advisories/types.ts
  • src/features/assets/components/asset.tsx
  • src/features/assets/components/dashboard-columns.tsx
  • src/features/assets/params.ts
  • src/features/assets/server/routers.ts
  • src/features/chat/utils.ts
  • src/features/chat/viper-agent/tools/get-recommendations-context.ts
  • src/features/inbox/agent/vex/__tests__/vex.test.ts
  • src/features/inbox/agent/vex/context.ts
  • src/features/inbox/agent/vex/index.ts
  • src/features/inbox/agent/vex/process_output.ts
  • src/features/inbox/agent/vex/tools.ts
  • src/features/issues/components/issue.tsx
  • src/features/issues/hooks/use-issues.ts
  • src/features/notes/server/get-relevant-notes.ts
  • src/features/tracking/types.ts
  • src/features/vulnerabilities/components/prioritized-columns.tsx
  • src/inngest/functions/process-inbox-email.ts
  • src/lib/prisma-client-extensions.ts
  • src/lib/string-utils.ts
  • src/test/server-only-stub.ts
  • vitest.config.mts

Comment thread src/features/notes/server/get-relevant-notes.ts
Comment thread prisma/schema.prisma

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested on this scenario: https://www.siemens-healthineers.com/support-documentation/cybersecurity/ssa-016040

I added notes to suggest that one issue should be created to be NOT_AFFECTED, and confirmed that this was the case in an end-to-end test

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests the deterministic writes that should run based on the output of the VEX agent

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agent gets:

  • Markdown for all linked vulnerabilities
  • Markdown for all linked remediations
  • Markdown for all linked device groups
  • The number of assets that are in each linked device group
  • All notes that are linked to any of the above (created TODO helper fn for this)
  • Persistent notes that always affect the hospital

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd look at tools.ts first to see what the agent output is

We process that output to create/modify issues as necessary

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

helper fn to match notes to VIPER db objects. some functionality in CDST RFC unimplemented for now (see VW-358)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On vulnerability create, create one issue per DeviceGroupMatching

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.

2 participants