Conversation
📝 WalkthroughWalkthroughSystematic migration of nullable values from null to undefined across the codebase, Nuxt/Vue ecosystem upgrades and Tailwind v4 via Vite, authentication state split (useAuth / useAuthState), import path updates for Nuxt 4, API plugin augmentation ($hubApi), test/mocks and TypeScript path/type adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Component (uses composable)
participant Composable as useDatastoreRequirement
participant State as Nuxt useState
participant API as Hub API
Component->>Composable: call useDatastoreRequirement()
Composable->>State: read datastoreRequired, nodeType
alt datastoreRequired undefined
Composable->>API: GET /node/settings
API-->>Composable: returns dataRequired
Composable->>State: set datastoreRequired
end
alt nodeType undefined
Composable->>API: GET /node-type
API-->>Composable: returns type
Composable->>State: set nodeType & datastoreRequired
end
Composable-->>Component: return { datastoreState, setDatastoreRequired }
Component->>Composable: setDatastoreRequired(value)
Composable->>API: POST /node/settings
API-->>Composable: updated response
Composable->>State: sync datastoreRequired
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
app/components/table/ApproveRejectButtons.vue (1)
6-9:⚠️ Potential issue | 🟡 Minor
objectIdis optional but used in URL paths without a guard.
objectId: Stringwithoutrequired: truemeans it can beundefined. Passing it directly into the URL produces paths like/project-nodes/undefinedsilently. Consider addingrequired: trueor guarding before the API call.🛡️ Proposed fix: mark props required or add early-exit guard
const props = defineProps({ - objectId: String, - objectClass: String, + objectId: { type: String, required: true }, + objectClass: { type: String, required: true }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/table/ApproveRejectButtons.vue` around lines 6 - 9, The prop objectId defined via defineProps({ objectId: String, objectClass: String }) can be undefined but is used directly in URL construction; either mark objectId as required in defineProps (e.g., make objectId: { type: String, required: true }) or add an early-exit guard in the component methods that build API paths (the approve/reject handlers that construct paths like `/project-nodes/${objectId}`) to return/throw if objectId is missing. Update the defineProps or add the guard near the functions that reference objectId so the component never constructs a URL with "undefined".test/utils/row-test-data.ts (1)
18-42:⚠️ Potential issue | 🔴 CriticalType inconsistencies found requiring fixes:
ModifiedAnalysisNodestill declaresrun_statusasPodStatus | nullrather thanundefined, and the fixture includesconfiguration_statuswhich does not exist in theAnalysisinterface definition.The main
Api.tstypes (AnalysisNodeandAnalysis) have been correctly migrated to usestring | undefinedand union withundefinedfor:comment,artifact_tag,artifact_digest,master_image_id,registry,master_image, andrun_status.However, two critical issues remain:
ModifiedAnalysisNode(app/services/modifiedApiInterfaces.ts) overridesrun_statuswithPodStatus | null—still usingnullinstead ofundefined. This creates a type inconsistency whenModifiedAnalysisNodeis substituted forAnalysisNode.configuration_statusin the fixture does not exist in theAnalysistype definition (rg search returned no matches). Removing this field from the fixture will eliminate the type mismatch.Additionally,
projectis defined asproject?: Project(implicitlyProject | undefined), so setting it toundefinedin the fixture is acceptable but could be clarified by adding explicit| undefinedto the type if needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/utils/row-test-data.ts` around lines 18 - 42, The ModifiedAnalysisNode type still declares run_status as PodStatus | null which conflicts with the migrated AnalysisNode/Analysis signatures that use string | undefined (or union with undefined); update ModifiedAnalysisNode (in app/services/modifiedApiInterfaces.ts) to use the same run_status type (string | undefined or the exact PodStatus | undefined union used by AnalysisNode) and remove null usage, and also delete the non-existent configuration_status field from the test fixture object in row-test-data.ts so the fixture matches the Analysis interface (leave project as undefined since project?: Project allows that).app/components/data-stores/DetailedDataStoreTable.vue (1)
55-60:⚠️ Potential issue | 🟡 MinorRemove
!assertion and use nullish coalescing for type safety.Line 60: The
projectNameMapis typed asMap<string, string | undefined>, makingget()returnstring | undefined. The!suppresses the undefined type unsafely. Use nullish coalescing instead:Proposed fix
project: props.projectNameMap?.has(projectUuid) - ? props.projectNameMap.get(projectUuid)! + ? (props.projectNameMap.get(projectUuid) ?? "N/A") : "N/A",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/data-stores/DetailedDataStoreTable.vue` around lines 55 - 60, The code uses a non-null assertion on props.projectNameMap.get(projectUuid)! which suppresses the possible undefined type; remove the `!` and replace with a nullish coalescing fallback to preserve type safety (e.g. use props.projectNameMap.get(projectUuid) ?? projectUuid or another sensible default) when building the newRow in DetailedDataStoreTable.vue where projectUuid and props.projectNameMap are used.app/components/analysis/logs/ContainerLogs.vue (1)
32-43:⚠️ Potential issue | 🟠 MajorRemove the non-null assertion on
nginxLogsto prevent runtime crashes.The
LogResponseinterface defines bothanalysisandnginxas optional fields. WhileanalysisLogsis safely guarded by theif (analysisLogs)check,nginxLogs!uses a non-null assertion without any guard. If the API response omits thenginxfield, this will throw a runtime error.Use optional chaining to safely access the nested property:
Suggested fix
const newLogEntry: logEntry = { podId: analysisPodId, analysis: analysisLogs[analysisPodId][0] ?? undefined, - nginx: nginxLogs![analysisPodId][0] ?? undefined, + nginx: nginxLogs?.[analysisPodId]?.[0], };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/logs/ContainerLogs.vue` around lines 32 - 43, In parseLogs, avoid the non-null assertion on nginxLogs; instead guard access with optional chaining and a fallback so missing nginx in LogResponse won't crash: inside the analysisPodIds.forEach where newLogEntry is built (symbols: parseLogs, analysisLogs, nginxLogs, analysisPodIds, newLogEntry, logEntry), replace nginxLogs![analysisPodId][0] ?? undefined with something like nginxLogs?.[analysisPodId]?.[0] ?? undefined so the code safely handles absent nginx entries.app/services/hub_adapter_swagger.json (1)
187-195:⚠️ Potential issue | 🟠 Major
type: "undefined"is invalid in OpenAPI/JSON Schema.OpenAPI 3.1.0 schemas are based on JSON Schema Draft 2020-12, which defines valid types as:
null,boolean,object,array,number,string, andinteger. The value"undefined"is not a valid type and will cause schema validation failures. This appears 209 times throughout the file.Optional fields should use
"type": "null"in ananyOfconstruct (as shown below) or be omitted from the schema'srequiredlist.Fix pattern
"anyOf": [ { "type": "string", "format": "uuid" }, - { "type": "undefined" } + { "type": "null" } ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/hub_adapter_swagger.json` around lines 187 - 195, The schema uses invalid "type": "undefined" entries in many anyOf arrays (e.g., the anyOf around UUID fields) which breaks OpenAPI/JSON Schema validation; replace each occurrence of { "type": "undefined" } with { "type": "null" } (or alternatively remove the property from the schema's required list so it becomes optional) across the file (there are ~209 instances), ensuring fields intended to be optional use null in anyOf or are not required; update the anyOf constructs like the one containing { "type": "string", "format": "uuid" } to use { "type": "null" } instead of "undefined".app/components/analysis/AnalysesTable.vue (1)
290-314:⚠️ Potential issue | 🔴 CriticalNull pointer dereference:
nextSetResultscan beundefinedwhen the API call fails.Line 303
.catch(() => undefined)returnsundefinedon failure, but it is castas AnalysisNode[]. Line 304 then accessesnextSetResults.length, which will throwTypeError: Cannot read properties of undefinedat runtime if the request fails.🐛 Proposed fix: add a guard for undefined
.catch(() => undefined)) as AnalysisNode[]; - if (nextSetResults.length > 0) { + if (nextSetResults && nextSetResults.length > 0) {Or more precisely, fix the type cast:
- .catch(() => undefined)) as AnalysisNode[]; - if (nextSetResults.length > 0) { + .catch(() => undefined)) as AnalysisNode[] | undefined; + if (nextSetResults && nextSetResults.length > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 290 - 314, The API call in getNextPage uses .catch(() => undefined) and casts the result to AnalysisNode[], so nextSetResults can be undefined and reading nextSetResults.length will throw; change the error handling so $hubApi either returns an empty array on failure or add a guard immediately after the call (check if nextSetResults is falsy and treat as [] or set allResultsRetrieved = true and return), and remove the unsafe cast; update the code around useNuxtApp().$hubApi("/analysis-nodes", ...) and subsequent logic (the nextSetResults length checks, currentOffset increment, and compileAnalysisTable("success", nextSetResults)) to operate on a properly typed array or bail early on undefined.app/components/header/AvatarButton.vue (1)
27-30:⚠️ Potential issue | 🟡 Minor
userActionLabelanduserActionIconare not reactive — menu items won't update if auth state changes.Lines 27-30 evaluate
isAuthenticated.valueeagerly at setup time, producing static strings. The same applies to thedisabledproperty at line 56. If the session expires or the user's auth state changes while this component is mounted, the menu label, icon, and disabled state will be stale.Consider making these
computedor moving them inside themenuItemsdefinition as getters.🔧 Proposed fix using computed menuItems
-const userActionLabel = isAuthenticated.value ? "Logout" : "Login"; -const userActionIcon = isAuthenticated.value - ? "pi pi-sign-in" - : "pi pi-sign-out"; - -const menuItems = ref([ +const menuItems = computed(() => [ { label: "Options", items: [ { - label: userActionLabel, - icon: userActionIcon, + label: isAuthenticated.value ? "Logout" : "Login", + icon: isAuthenticated.value ? "pi pi-sign-in" : "pi pi-sign-out", command: () => { // eslint-disable-next-line `@typescript-eslint/no-unused-expressions` isAuthenticated.value ? signOut() : signIn(`${idpProvider}`); }, }, ... { label: "Clean Up Resources", icon: "pi pi-eject", command: () => { showCleanupDialog.value = true; }, - disabled: !isAuthenticated.value, + disabled: !isAuthenticated.value, }, ... ], }, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/header/AvatarButton.vue` around lines 27 - 30, userActionLabel and userActionIcon are evaluated once at setup and are not reactive; change them to computed properties and ensure menuItems references those computeds (or make menuItems itself a computed that returns items using isAuthenticated) so the label, icon and the disabled state (the prop referenced around the current "disabled" usage) update when isAuthenticated changes; update AvatarButton.vue to replace the static const userActionLabel/userActionIcon with computed(...) and/or convert menuItems to a computed that reads isAuthenticated.value for the label, icon and disabled logic.app/services/Api.ts (1)
1-11:⚠️ Potential issue | 🟡 MinorThis is an auto-generated file — do not manually edit it.
The
null→undefinedchanges appear to be from regeneration with swagger-typescript-api 13.2.18, which usesundefinedsemantics by default. There is no generator configuration option to control this behavior. Note: thebuild-apiscript in package.json outputs to./services/, but this file is located at./app/services/Api.ts—verify the output path configuration is correct to prevent these changes from being lost on the next regeneration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/Api.ts` around lines 1 - 11, The Api.ts file is auto-generated and shows null→undefined differences due to swagger-typescript-api v13.2.18 semantics and an output path mismatch; do not edit the generated Api.ts directly—instead update the generation pipeline: adjust the build-api script in package.json to output to ./app/services (or change the generator's outputDir to match where Api.ts currently lives) so regenerations overwrite the correct file, and if you need consistent null vs undefined semantics either pin the swagger-typescript-api version or add a post-generation transform step to convert nulls to undefined before committing; reference the generated file Api.ts and the build-api script / swagger-typescript-api version when making these changes.
🧹 Nitpick comments (23)
app/components/data-stores/DataStoreList.vue (1)
83-92:| undefinedin the Map value type is dead code — consider tightening toMap<string, string>.
mapDataFromHubalways stores astringvalue (entry.name ?? "N/A"), so the| undefinedbranch on the return type is never exercised. UsingMap<string, string>would be more precise and make the type self-documenting.♻️ Proposed refactor
function mapDataFromHub(hubData: Project[] | DetailedAnalysis[]) { - let mappedNames = new Map<string, string | undefined>(); + let mappedNames = new Map<string, string>(); if (hubData && hubData.length > 0) { hubData.forEach((entry: Project | DetailedAnalysis) => { if (entry.id) { mappedNames.set(entry.id, entry.name ? entry.name : "N/A"); } }); } return mappedNames; }And at the
refdeclaration:-const projectNameMap = ref<Map<string, string | undefined>>( - new Map<string, string | undefined>(), +const projectNameMap = ref<Map<string, string>>( + new Map<string, string>(), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/data-stores/DataStoreList.vue` around lines 83 - 92, The Map value type is overly permissive: change the mappedNames declaration from Map<string, string | undefined> to Map<string, string> in mapDataFromHub, and ensure you always insert a string (e.g., use entry.name ?? "N/A" when calling mappedNames.set(entry.id, ...)). Also update any related ref type annotations that reference this map (the ref that holds mappedNames) to use Map<string, string> (or Ref<Map<string,string>>) so the types remain consistent.test/components/events/EventViewer.spec.ts (2)
29-37: Optional: add explicit type annotations onerrorandstatusrefs for type correctness.
ref(undefined)is inferred asRef<undefined>andref("success")asRef<string>. BecauseRef<T>is invariant in TypeScript, these are not strictly assignable toRef<FetchError | undefined>andRef<AsyncDataRequestStatus>respectively. The mock likely still works at runtime, but the type gap can silently mask future signature mismatches.🛠️ Proposed fix (applies equally to the second mock at Lines 63–74)
- error: ref(undefined), - status: ref("success"), + error: ref<FetchError | undefined>(undefined), + status: ref<AsyncDataRequestStatus>("success"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/components/events/EventViewer.spec.ts` around lines 29 - 37, The mocked return for getEvents uses ref(undefined) and ref("success") which infer Ref<undefined> and Ref<string> and can’t be assigned to the expected Ref<FetchError | undefined> and Ref<AsyncDataRequestStatus>; update the mock to use explicit generic type annotations on those refs (e.g. ref<FetchError | undefined>(undefined) for the error field and ref<AsyncDataRequestStatus>('success') for the status field) and apply the same change to the second mock block (the other mocked resolved value around Lines 63–74) so the types match the expected signatures for getEvents and fakeEventResponse.
48-49: Consider guarding array lengths before non-null assertions for clearer test failure messages.If the rendered table has fewer columns/rows than expected (e.g., due to a component regression),
headerRow[0]!will throw aTypeErrorinstead of a Vitest assertion failure, making CI diagnosis harder.🛠️ Proposed guard additions
const headerRow = wrapper.findAll("thead tr th"); + expect(headerRow.length).toBeGreaterThanOrEqual(2); expect(headerRow[0]!.text()).toBe("DateTime"); expect(headerRow[1]!.text()).toBe("Event"); const rows = wrapper.findAll("tbody tr"); expect(rows.length).toBe(1); + expect(rows[0]!.findAll("td").length).toBeGreaterThanOrEqual(2); const rowCells = rows[0]!.findAll("td");Also applies to: 55-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/components/events/EventViewer.spec.ts` around lines 48 - 49, Guard array lengths before using non-null assertions: in EventViewer.spec.ts add assertions on headerRow.length (e.g., expect(headerRow.length).toBeGreaterThanOrEqual(2) or toHaveLength(2)) before the existing expect(headerRow[0]!.text()).toBe("DateTime") and headerRow[1]!.text() checks; apply the same pattern to the subsequent checks around lines 55-58 (guard any arrays like bodyRows/rows used with [index]!) so failures produce clear Vitest assertion messages instead of TypeError from a missing element.app/components/table/ApproveRejectButtons.vue (1)
16-46:loadingis never reset if an unexpected exception escapes thecatchhandlers.Both
.catch((e) => console.error(e))calls suppress errors and returnundefined, so no throw currently escapes. However, any future refactor that lets an error propagate (e.g., replacing the inline.catchwith atry/catch) would leaveloading.valuestuck attrue. Wrapping intry/finallyis cheap insurance.♻️ Proposed refactor: move `loading.value = false` to `finally`
async function onSubmitApproval(isApproved: boolean) { loading.value = true; - const formData = new FormData(); - formData.append("approval_status", isApproved ? "approved" : "rejected"); - let approvalResp: AnalysisNode | ProjectNode | undefined; - - if (props.objectClass == "project") { - approvalResp = (await useNuxtApp() - .$hubApi(`/project-nodes/${props.objectId}`, { - method: "POST", - body: formData, - }) - .catch((e) => console.error(e))) as ProjectNode; - } else if (props.objectClass == "analysis") { - approvalResp = (await useNuxtApp() - .$hubApi(`/analysis-nodes/${props.objectId}`, { - method: "POST", - body: formData, - }) - .catch((e) => console.error(e))) as AnalysisNode; - } - - if (approvalResp && "approval_status" in approvalResp) { - showSuccessfulSubmission(isApproved); - emit("updatedRow", approvalResp); - } else { - showFailedSubmission(); - } - loading.value = false; + try { + const formData = new FormData(); + formData.append("approval_status", isApproved ? "approved" : "rejected"); + let approvalResp: AnalysisNode | ProjectNode | undefined; + + if (props.objectClass === "project") { + approvalResp = (await useNuxtApp() + .$hubApi(`/project-nodes/${props.objectId}`, { + method: "POST", + body: formData, + }) + .catch((e) => console.error(e))) as ProjectNode; + } else if (props.objectClass === "analysis") { + approvalResp = (await useNuxtApp() + .$hubApi(`/analysis-nodes/${props.objectId}`, { + method: "POST", + body: formData, + }) + .catch((e) => console.error(e))) as AnalysisNode; + } + + if (approvalResp && "approval_status" in approvalResp) { + showSuccessfulSubmission(isApproved); + emit("updatedRow", approvalResp); + } else { + showFailedSubmission(); + } + } finally { + loading.value = false; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/table/ApproveRejectButtons.vue` around lines 16 - 46, The onSubmitApproval function currently sets loading.value = true but leaves the reset at the end, which can be skipped if an exception escapes; wrap the API calls and post-response logic inside a try block and move loading.value = false into a finally block so it always executes; keep existing error handling for the useNuxtApp() calls (or convert them to throw and let try/catch handle logging) but ensure showSuccessfulSubmission, emit("updatedRow", ...) and showFailedSubmission remain in the try so their flow is preserved, and remove the standalone loading.value = false at the end of onSubmitApproval to avoid duplicate resets.app/services/modifiedApiInterfaces.ts (2)
24-30: Redundant"updated_at"key inOmit<Consumer, ...>.The
Consumerinterface (fromapp/services/Api.ts) does not declare anupdated_atfield at all — it only hascreated_at,custom_id,id,tags, andusername. Omitting a key that doesn't exist is a silent no-op in TypeScript and can mislead maintainers into thinkingConsumeroriginally had anupdated_atthat needed to be overridden.The
updated_at?: modifiedTimestampon line 29 is actually a net-new field being added to the interface, not a replacement.🧹 Proposed cleanup
-export interface ModifiedConsumer extends Omit< - Consumer, - "created_at" | "updated_at" -> { +export interface ModifiedConsumer extends Omit< + Consumer, + "created_at" +> { created_at: modifiedTimestamp; updated_at?: modifiedTimestamp; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/modifiedApiInterfaces.ts` around lines 24 - 30, The Omit in ModifiedConsumer incorrectly lists "updated_at" which doesn't exist on Consumer and misleads readers; update the definition of ModifiedConsumer so Omit only removes "created_at" from Consumer, keep created_at typed as modifiedTimestamp, and keep updated_at?: modifiedTimestamp as the new optional field being added; locate the ModifiedConsumer interface and the modifiedTimestamp type to make this small change.
40-48: Changerun_statusto useundefinedinstead ofnullfor type consistency.
run_status: PodStatus | nullis inconsistent with the rest of the interface, which usesundefined(seeproject_name: string | undefined). The originalAnalysisNode.run_statustype also usesundefined, notnull. Aligning this property with both the original type and the rest of the modified interface makes the codebase more uniform.Suggested fix
- run_status: PodStatus | null; + run_status: PodStatus | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/modifiedApiInterfaces.ts` around lines 40 - 48, Change the ModifiedAnalysisNode.run_status type from allowing null to using undefined for consistency: update the run_status declaration in the ModifiedAnalysisNode interface to use PodStatus | undefined (matching project_name and the original AnalysisNode.run_status) so the interface aligns with the rest of the codebase and original type definitions.app/components/data-stores/DetailedDataStoreTable.vue (3)
148-165:null→undefinedmigration in filters looks correct; line 162 is redundant.The
defaultFiltersvalues already carryvalue: undefined, so the...spreadon line 160 already setsvalue: undefined. The explicit assignment on line 162 is redundant but harmless.♻️ Simplified resetFilters
function resetFilters() { const clearedFilters = {}; for (const filterKey in defaultFilters) { clearedFilters[filterKey] = { ...defaultFilters[filterKey], }; - clearedFilters[filterKey].value = undefined; } filters.value = clearedFilters; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/data-stores/DetailedDataStoreTable.vue` around lines 148 - 165, The resetFilters function is redundantly reassigning value to undefined after spreading defaultFilters; simplify by building clearedFilters from defaultFilters without the extra assignment. Update resetFilters (which mutates filters.value) to clone each entry from defaultFilters (e.g., via { ...defaultFilters[filterKey] }) and assign that to clearedFilters[filterKey] without separately setting .value, ensuring filters.value = clearedFilters remains the same; this removes the redundant line that explicitly sets .value = undefined.
12-12: ExplicituseNuxtAppimport is unnecessary in Nuxt 4 SFCs.Nuxt 4 auto-imports composables including
useNuxtAppin SFC<script setup>context. The explicit import from"nuxt/app"works correctly but is redundant — it can be safely removed.♻️ Suggested cleanup
-import { useNuxtApp } from "nuxt/app";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/data-stores/DetailedDataStoreTable.vue` at line 12, Remove the redundant explicit import of useNuxtApp from "nuxt/app" in DetailedDataStoreTable.vue: delete the `import { useNuxtApp } from "nuxt/app";` line and rely on Nuxt 4's auto-imported composable in the <script setup> context; verify any references to useNuxtApp still work without the import and that the file is indeed using <script setup> so no additional imports are required.
15-23:| undefinedis redundant alongside the?optional modifier.The optional property marker
?already addsundefinedto the type union.name?: string | undefinedis identical toname?: string.♻️ Suggested cleanup
interface DetailedDataStoreTableRow { - name?: string | undefined; - type?: string | undefined; - project?: string | undefined; - path?: string | undefined; - host?: string | undefined; - port?: number | undefined; - protocol?: string | undefined; - created_at?: string | undefined; - updated_at?: string | undefined; + name?: string; + type?: string; + project?: string; + path?: string; + host?: string; + port?: number; + protocol?: string; + created_at?: string; + updated_at?: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/data-stores/DetailedDataStoreTable.vue` around lines 15 - 23, The property type declarations in DetailedDataStoreTable.vue use both the optional marker and explicit `| undefined` (e.g., `name?: string | undefined`, `port?: number | undefined`); remove the redundant `| undefined` on each property so they read `name?: string`, `type?: string`, `project?: string`, `path?: string`, `host?: string`, `port?: number`, `protocol?: string`, `created_at?: string`, `updated_at?: string` (locate the type/interface block containing these property names and update each declaration accordingly).tsconfig.json (1)
6-6: Consider scoping@nuxt/test-utilstypes to a test-specific tsconfig.Adding
@nuxt/test-utilsto the roottypesarray makes test-utility types (e.g.,mountSuspended,mockNuxtImport) visible in all source files, not just tests. A dedicatedtsconfig.test.json(ortsconfig.vitest.json) that extends this root config and adds the test types would keep the type surface clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` at line 6, Remove "@nuxt/test-utils" from the root tsconfig's "types" array to avoid leaking test-only types into production code, and add a new test-specific tsconfig (e.g., tsconfig.test.json or tsconfig.vitest.json) that extends the root tsconfig and adds "types": ["@nuxt/test-utils"] so only test compilation sees those types; update your test runner config to point at the new test tsconfig. Ensure you edit the existing "types" entry in tsconfig.json (the line containing "types": ["@nuxt/test-utils"]) and create the extending test tsconfig that references the root config and injects the test-only types.app/components/data-stores/create/ResourceManagerTabs.vue (1)
6-10: Redundant duplicate| undefinedin thenameunion type
string | undefined | undefinedis a copy-paste artifact; TypeScript deduplicates union members so there is no runtime impact, but it should be cleaned up.🧹 Proposed fix
export interface availableProject { - name: string | undefined | undefined; + name: string | undefined; id: string; dropdown: string; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/data-stores/create/ResourceManagerTabs.vue` around lines 6 - 10, The availableProject interface has a redundant union member on the name property ("name: string | undefined | undefined"); update the name declaration in the availableProject interface to remove the duplicate undefined so it reads as a single union (e.g., "name: string | undefined") to clean up the type definition in ResourceManagerTabs.vue.test/components/header/MenuHeader.spec.ts (1)
31-34: Minor inconsistency:data: ref(null)vs default mock'sdata: ref({}).The default mock in
test/mockapi/nuxt-auth-mock.tsusesdata: ref({}), but this test overrides withdata: ref(null). SinceMenuHeader.vueonly readsstatus, this doesn't affect correctness, but aligning with the default mock value would be slightly more consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/components/header/MenuHeader.spec.ts` around lines 31 - 34, Update the test mock to match the default mock shape: in MenuHeader.spec.ts replace the overridden useAuthState mock payload so its data uses ref({}) instead of ref(null); locate the vi.mocked(useAuthState) call and change the data value to ref({}) to be consistent with test/mockapi/nuxt-auth-mock.ts while leaving status as-is.app/plugins/api.ts (1)
68-71:?? undefinedis a no-op when the optional chain already yieldsundefined.
response._data.detail?.service ?? undefined— the?? undefinedonly converts an explicitnullvalue ofservicetoundefined. If this is intentional for the null→undefined migration, consider a brief comment for clarity, since the expression looks like a no-op at first glance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plugins/api.ts` around lines 68 - 71, In onResponseError, the expression response._data.detail?.service ?? undefined is redundant because optional chaining already yields undefined; either remove the ?? undefined and use const errSvc = response._data.detail?.service or, if you intentionally want null→undefined normalization, keep it and add a short clarifying comment above errSvc explaining that behavior — update the variable definition for errSvc accordingly in the onResponseError handler.app/composables/useDatastoreRequirement.ts (1)
4-7:datastoreRequiredtype still includesnull— inconsistent with undefined migration.The
DatastoreStateinterface declaresdatastoreRequired: boolean | undefined | nullwhilenodeTypeisstring | undefined(nonull). If the codebase is migrating away fromnull, consider droppingnullfrom this type as well — unless there's a specific reason to retain it (e.g.,useStateserialization).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/composables/useDatastoreRequirement.ts` around lines 4 - 7, DatastoreState currently types datastoreRequired as boolean | undefined | null while nodeType uses string | undefined; remove null from datastoreRequired to match the undefined-only migration by changing its type to boolean | undefined in the DatastoreState interface (symbol: DatastoreState, property: datastoreRequired) and then update any code that assigns or checks for null (e.g., in useDatastoreRequirement composable or callers) to use undefined checks instead so no runtime expectations of null remain.app/components/analysis/AnalysisControlButtons.vue (1)
170-175:checkPodStatus()is called withoutawait— result is silently discarded.On line 173,
checkPodStatus()returns aPromise<boolean>but is called withoutawaitin the.catch()handler. The boolean return (indicating whether the pod was already running) is never consumed. If the intent is to update button states as a side-effect, this works but the unhandled promise could swallow errors. Considerawaiting or adding.catch()to prevent unhandled rejection.This appears to be pre-existing behavior, so not blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysisControlButtons.vue` around lines 170 - 175, The catch handler calls checkPodStatus() which returns a Promise<boolean> but is not awaited or handled; update the handler in AnalysisControlButtons.vue to either await checkPodStatus() (e.g., await checkPodStatus()) or append a .catch(...) to it so its result and any errors are handled — ensure checkPodStatus() is invoked in a way that prevents unhandled promise rejections while preserving its side effects for updating button state.app/components/analysis/AnalysesTable.vue (2)
234-236:parseProjects()andgetKongRoutes()are independent — consider parallelizing.Both calls are awaited sequentially but have no data dependency on each other. Running them concurrently would improve table load time.
♻️ Suggested parallel execution
tableLoading.value = true; - await parseProjects(); - await getKongRoutes(); + await Promise.all([parseProjects(), getKongRoutes()]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 234 - 236, The code currently awaits parseProjects() and then getKongRoutes() sequentially which wastes time because they are independent; change the control flow in the component (where tableLoading.value is set and parseProjects/getKongRoutes are called) to run them in parallel by starting both promises and awaiting Promise.all([parseProjectsPromise, getKongRoutesPromise]) (or equivalent) so both execute concurrently, while preserving setting tableLoading.value = true before and setting it false after the combined await; reference the parseProjects and getKongRoutes calls and the tableLoading.value flag when making the change.
88-98: Same pattern:getProjectsandgetKongRoutescast catch-undefined results to concrete types.Both
getProjects()(line 97) andgetKongRoutes()(line 105) use.catch(() => undefined)but cast the result withas Project[]/as ListRoutes, maskingundefinedfrom the type system. The callers do handleundefinedat runtime (line 124 for projects, line 106 for kong routes), so this is not a crash risk here, but the casts are misleading.Consider typing the return value honestly:
♻️ Suggested type fix for getProjects
async function getProjects() { return (await useNuxtApp() .$hubApi("/projects", { ... }) - .catch(() => undefined)) as Project[]; + .catch(() => undefined)) as Project[] | undefined; }Also applies to: 100-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 88 - 98, getProjects and getKongRoutes currently mask a possible undefined result by casting the caught value to concrete types; update their return types to reflect that they may return undefined (e.g., Promise<Project[] | undefined> and Promise<ListRoutes | undefined>) and remove the misleading `as Project[]` / `as ListRoutes` casts so the type system matches runtime behavior; adjust the function signatures for getProjects and getKongRoutes accordingly and keep the existing caller-side checks (which already handle undefined).package.json (1)
29-30: Both@tailwindcss/postcssand@tailwindcss/viteare listed but only the Vite plugin is used.
nuxt.config.tsonly configures the Vite plugin. If PostCSS integration isn't needed,@tailwindcss/postcsscan be removed to reduce dependency footprint.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 29 - 30, The package.json includes both `@tailwindcss/postcss` and `@tailwindcss/vite` while nuxt.config.ts only configures the Vite plugin; remove the unused `@tailwindcss/postcss` dependency to reduce footprint by deleting "@tailwindcss/postcss": "^4.2.0" from package.json and running the package manager uninstall (or update lockfile) to ensure it's removed; if you actually intended to use PostCSS instead of the Vite plugin, instead add/configure `@tailwindcss/postcss` in nuxt.config.ts and remove the Vite plugin entry accordingly.nuxt.config.ts (1)
69-73: Inconsistent CSS path style: relative./app/vs alias~/.Line 70 uses
./app/assets/css/main.csswhile lines 72-73 use~/assets/css/.... In Nuxt 4,~resolves to theapp/directory, so both should work — but mixing styles reduces readability. Consider using one convention consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nuxt.config.ts` around lines 69 - 73, The css array in nuxt.config.ts mixes path styles ("./app/assets/css/main.css" vs "~/assets/css/elements.css" and "~/assets/css/table.css"), hurting readability; pick a single convention and make the entry for main.css match the others (e.g., change "./app/assets/css/main.css" to "~/assets/css/main.css") so all imports in the css array use the same aliasing style.app/composables/useAPIFetch.ts (1)
79-89: Verify that embeddinginclude=analysis,nodein the URL path while also providing aqueryobject doesn't cause issues.Line 81 places
include=analysis,nodedirectly in the URL string, while thequeryobject at lines 85-87 addssort. Nuxt'suseFetchwill appendqueryparams to the URL, resulting in/analysis-nodes?include=analysis,node&sort=-updated_at. This works but is inconsistent with how other functions in this file handle query params (e.g.,getProjectNodesat line 49 putsincludein thequeryobject). Consider movingincludeinto thequeryobject for consistency.♻️ Suggested refactor
export function getAnalysisNodes(opts?) { - return useAPIFetch<AnalysisNode[] | undefined>( - "/analysis-nodes?include=analysis,node", - { - ...opts, - method: "GET", - query: { - sort: "-updated_at", - }, + return useAPIFetch<AnalysisNode[] | undefined>( + "/analysis-nodes", + { + ...opts, + method: "GET", + query: { + include: "analysis,node", + sort: "-updated_at", + }, }, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/composables/useAPIFetch.ts` around lines 79 - 89, getAnalysisNodes currently embeds "include=analysis,node" directly in the URL which is inconsistent with other helpers like getProjectNodes and can be moved to the query object; update getAnalysisNodes (the call to useAPIFetch) to remove the include from the URL path and add include: "analysis,node" to the query object alongside sort so the final request is constructed consistently via useAPIFetch/useFetch.app/services/Api.ts (1)
1879-1883: Redundant check:typeof body === "undefined" || body === undefined.Both conditions test the same thing for a declared variable. Since this is generated code, this is non-blocking, but if you maintain a custom template, simplifying to
body === undefined(orbody == nullto also catchnull) would be cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/services/Api.ts` around lines 1879 - 1883, The body assignment contains a redundant check `typeof body === "undefined" || body === undefined`; update the conditional where the request body is set (the block using requestParams.signal and payloadFormatter(body)) to use a single, clearer check such as `body === undefined` (or `body == null` if you also want to treat null as empty) and leave the payloadFormatter(body) branch unchanged so only defined bodies are formatted and assigned.test/components/header/AvatarButton.spec.ts (1)
27-27:vi.restoreAllMocks()inbeforeEachresets all mock return values — confirm this doesn't interfere with setup.ts mocks.
restoreAllMocks()clears anymockReturnValue/mockImplementationoverrides on all mocks, including those set in the global test setup (e.g.,useToast,useRuntimeConfig). Thevi.mock(...)factory functions insetup.tswill still apply for new calls, so this should be safe, but it's worth keeping in mind if tests start failing due to missing mock returns.Consider using
vi.clearAllMocks()instead if you only need to reset call history without restoring original implementations.restoreAllMocksis more aggressive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/components/header/AvatarButton.spec.ts` at line 27, The beforeEach in AvatarButton.spec.ts currently calls vi.restoreAllMocks(), which will wipe mock implementations established in global setup (e.g., useToast, useRuntimeConfig); change this to vi.clearAllMocks() so only call history is reset and factory mocks from setup.ts remain intact, and verify that functions referenced in tests (like the AvatarButton test utilities and any mocked hooks) still return expected values after the change.test/mockapi/api.ts (1)
30-30: Nit:?? undefinedis a no-op when the goal is just "not null".If
response._data?.detail?.serviceisnull, the??converts it toundefined; if it's alreadyundefined, it staysundefined. But optional chaining (?.) already producesundefined(notnull) when a link in the chain is missing. The only case?? undefinedadds value is when the API actually returns an explicitnullforservice. If that's intentional, a brief comment would help future readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mockapi/api.ts` at line 30, The assignment to errSvc uses a redundant "?? undefined"; remove the "?? undefined" so errSvc is set via optional chaining only (const errSvc = response._data?.detail?.service), or if the API intentionally returns explicit null and you want to normalize null→undefined, keep the "?? undefined" but add a short comment near errSvc explaining that explicit nulls from the API should be treated as undefined; locate the errSvc declaration to apply the change.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/plugins/api.ts (1)
43-51:⚠️ Potential issue | 🟠 Major
getSession()on every request causes unnecessary network round-trips — use the cacheddataref fromuseAuth()instead.
getSession()fetches the session from the server on each invocation. Calling it unconditionally inonRequestmeans every API call throughhubApitriggers a prior HTTP request to the auth endpoint, effectively doubling network traffic.The composable already uses this pattern elsewhere:
useAuthRefresh.tsreads fromconst { status, data, refresh } = useAuth()and accessesdata.valuesynchronously. Use the same approach here:⚡ Proposed fix
- const { signIn, getSession } = useAuth(); + const { signIn, data: sessionRef } = useAuth(); ... async onRequest({ options }) { - const sessionData = await getSession(); + const sessionData = sessionRef.value;Secondary issue — test mock lacks
getSession.
IfgetSession()is retained, the mock attest/mockapi/nuxt-auth-mock.tsmust provide it (currently it only exports{ signIn, signOut }). Any test that loads the plugin will fail with "getSession is not a function":export const useAuth = vi.fn(() => ({ signIn: vi.fn(), signOut: vi.fn(), + getSession: vi.fn().mockResolvedValue({ accessToken: "test-token" }), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plugins/api.ts` around lines 43 - 51, Replace the unconditional server call to getSession() inside the plugin's async onRequest handler by reading the cached session ref from useAuth(): call const { data } = useAuth() and use data.value (instead of await getSession()) when deciding token refresh and access, keeping the existing shouldRefreshToken(), refreshToken(), signIn(idpProvider) flow; additionally, if you choose to keep getSession() calls, update the test mock (nuxt-auth-mock.ts) to export getSession (e.g., a vi.fn().mockResolvedValue({ accessToken: "test-token" })) so tests won’t fail with "getSession is not a function".app/components/analysis/AnalysesTable.vue (1)
298-323:⚠️ Potential issue | 🔴 Critical
nextSetResults.lengthcrashes with aTypeErrorwhen the API call fails.When
$hubApirejects,.catch(() => undefined)returnsundefined. Theas AnalysisNode[]assertion silences TypeScript, but line 312 dereferences.lengthonundefined, throwing aTypeErrorat runtime. The same unsafe cast pattern appears ingetProjects(line 97, safe at runtime — caller guards withif (projResp)) andgetKongRoutes(line 105, also guarded), butgetNextPagehas no such guard.The simplest fix is to return an empty array on failure, which naturally falls into the existing
elsebranch and setsallResultsRetrieved = true:🐛 Proposed fix
const nextSetResults = (await useNuxtApp() .$hubApi("/analysis-nodes", { method: "GET", query: { page: { offset: currentOffset, limit: queryLimit, }, include: "analysis,node", sort: "-updated_at", }, }) - .catch(() => undefined)) as AnalysisNode[]; + .catch(() => [])) as AnalysisNode[];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 298 - 323, getNextPage currently assumes the $hubApi call returns an array but .catch(() => undefined) can make nextSetResults undefined, causing nextSetResults.length to throw; change the error handler to return an empty array (e.g., .catch(() => []) or otherwise default nextSetResults = [] before checking .length) so that the rest of getNextPage (including the allResultsRetrieved flag, currentOffset increment and compileAnalysisTable("success", nextSetResults)) behaves safely at runtime.
🧹 Nitpick comments (4)
test/mockapi/setup.ts (1)
87-89:useStatemock should return a realRef, not a plain object.
useState()returnsRef<T>in Nuxt. The current mock returns{ value: "default" }, a plain POJO that only coincidentally satisfies.valuereads. Any component using the return value reactively (e.g.,watch(useState(...), ...),isRef()guard, two-wayv-model) will silently misbehave in tests. Sincerefis already imported on line 2, the fix is one token:♻️ Proposed fix
- useState: vi.fn(() => ({ - value: "default", - })), + useState: vi.fn(() => ref("default")),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/mockapi/setup.ts` around lines 87 - 89, The mock for useState currently returns a plain object { value: "default" } but must return a real Vue Ref; update the useState mock implementation to return a ref created with the imported ref helper (e.g., ref("default")) so components using reactivity helpers (watch, isRef, v-model) behave correctly; locate the useState mock in test/mockapi/setup.ts and replace the plain object return with a call to ref(...) preserving the same initial value.app/plugins/api.ts (1)
71-71:?? undefinedis a no-op — remove it.The optional-chain
response._data.detail?.servicealready producesundefinedwhendetailis nullish. Coalescing toundefinedvia??is redundant and adds noise.🧹 Proposed cleanup
- const errSvc = response._data.detail?.service ?? undefined; + const errSvc = response._data.detail?.service;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plugins/api.ts` at line 71, The assignment to errSvc in app/plugins/api.ts is using a redundant nullish coalescing to undefined; remove the "?? undefined" part so errSvc is simply assigned from response._data.detail?.service (i.e., delete the "?? undefined" token in the const errSvc = ... line) to clean up the code.app/components/analysis/AnalysesTable.vue (2)
268-271: Floating promise inonMounted— unhandled rejections are silently swallowed.
compileAnalysisTableisasyncbut the returned Promise is neither awaited nor.catch()-ed inonMounted. Vue 3 ignores the returned Promise from lifecycle hooks. While current internal helpers swallow errors via.catch(() => undefined), future changes could throw and leave the table silently broken.onTableRefreshandgetNextPagecorrectlyawaitthe same function.♻️ Proposed fix
onMounted(() => { - compileAnalysisTable(status.value, analysisNodeResp.value); + compileAnalysisTable(status.value, analysisNodeResp.value).catch(console.error); pollIntervalId = setInterval(checkForUpdatesFromPodOrc, 15000); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 268 - 271, The onMounted callback currently calls the async compileAnalysisTable(status.value, analysisNodeResp.value) without awaiting or handling rejections; change the onMounted callback to either be async and await compileAnalysisTable(...) or explicitly attach a .catch(...) to the returned promise so errors are not swallowed (e.g., make onMounted(async () => { await compileAnalysisTable(...); pollIntervalId = setInterval(...); }) or compileAnalysisTable(...).catch(err => { /* log/handle */ });); reference compileAnalysisTable and onMounted so the promise is handled before/alongside setting pollIntervalId.
88-118: Unsafeas Typecasts hideundefinedfrom TypeScript ingetProjectsandgetKongRoutes.Both helpers use
.catch(() => undefined)) as Project[]/as ListRoutes, declaring a non-nullable return type while the actual value can beundefined. Callers guard correctly at runtime (if (projResp),if (kongRoutesResp && kongRoutesResp.data)), but the cast misleads TypeScript and repeats the same pattern that causes the crash ingetNextPage. Annotating the return types explicitly eliminates the mismatch:♻️ Suggested change
-async function getProjects() { +async function getProjects(): Promise<Project[] | undefined> { return (await useNuxtApp() .$hubApi("/projects", { method: "GET", query: { sort: "-updated_at", fields: "id,name" }, }) - .catch(() => undefined)) as Project[]; + .catch(() => undefined)) as Project[] | undefined; } -async function getKongRoutes() { +async function getKongRoutes(): Promise<void> { const kongRoutesResp = (await useNuxtApp() .$hubApi("/kong/project", { method: "GET" }) - .catch(() => undefined)) as ListRoutes; + .catch(() => undefined)) as ListRoutes | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 88 - 118, getProjects and getKongRoutes use unsafe "as Type" casts that hide possible undefined results from useNuxtApp().$hubApi; change them to return nullable types instead (e.g., getProjects returns Promise<Project[] | undefined> and getKongRoutes returns Promise<ListRoutes | undefined>), remove the "as Project[]" / "as ListRoutes" casts and let the actual value (possibly undefined) flow through from useNuxtApp().$hubApi, keeping the existing runtime guards (e.g., checks like if (kongRoutesResp && kongRoutesResp.data)); update any callers that assume non-nullable results to handle the new nullable return types if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 326-331: The DataTable filters use undefined as the "no filter"
sentinel which breaks PrimeVue 4; update the defaultFilters object in
AnalysesTable.vue (symbol: defaultFilters) to use value: null for global,
approval_status, "analysis.build_status", and run_status, and likewise modify
the clear/emit behavior in SearchBar.vue (symbol: the clear/emit function around
line 21) so it emits null instead of undefined when clearing filters to keep
UI/state in sync.
In `@test/mockapi/setup.ts`:
- Around line 57-60: The vi.mock factory is importing the mock using the
path-alias "@/test/mockapi/nuxt-auth-mock"; replace that with a relative import
(e.g. './nuxt-auth-mock') inside the async factory so Vitest resolves the module
explicitly. Update the vi.mock call for "@sidebase/nuxt-auth" to import the
local module via a relative path to the current file (referencing the
nuxt-auth-mock module name in the factory).
---
Outside diff comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 298-323: getNextPage currently assumes the $hubApi call returns an
array but .catch(() => undefined) can make nextSetResults undefined, causing
nextSetResults.length to throw; change the error handler to return an empty
array (e.g., .catch(() => []) or otherwise default nextSetResults = [] before
checking .length) so that the rest of getNextPage (including the
allResultsRetrieved flag, currentOffset increment and
compileAnalysisTable("success", nextSetResults)) behaves safely at runtime.
In `@app/plugins/api.ts`:
- Around line 43-51: Replace the unconditional server call to getSession()
inside the plugin's async onRequest handler by reading the cached session ref
from useAuth(): call const { data } = useAuth() and use data.value (instead of
await getSession()) when deciding token refresh and access, keeping the existing
shouldRefreshToken(), refreshToken(), signIn(idpProvider) flow; additionally, if
you choose to keep getSession() calls, update the test mock (nuxt-auth-mock.ts)
to export getSession (e.g., a vi.fn().mockResolvedValue({ accessToken:
"test-token" })) so tests won’t fail with "getSession is not a function".
---
Duplicate comments:
In @.github/actions/setup-and-cache/action.yaml:
- Line 42: The condition uses the wrong step id and checks the whole step object
instead of the version output; update the expression to reference the declared
step id resolve-package-versions and its outputs key (e.g.,
steps.resolve-package-versions.outputs.NUXT_VERSION) so the
contains(fromJSON(...), ...) call receives the version string to correctly
detect null/""/"null".
---
Nitpick comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 268-271: The onMounted callback currently calls the async
compileAnalysisTable(status.value, analysisNodeResp.value) without awaiting or
handling rejections; change the onMounted callback to either be async and await
compileAnalysisTable(...) or explicitly attach a .catch(...) to the returned
promise so errors are not swallowed (e.g., make onMounted(async () => { await
compileAnalysisTable(...); pollIntervalId = setInterval(...); }) or
compileAnalysisTable(...).catch(err => { /* log/handle */ });); reference
compileAnalysisTable and onMounted so the promise is handled before/alongside
setting pollIntervalId.
- Around line 88-118: getProjects and getKongRoutes use unsafe "as Type" casts
that hide possible undefined results from useNuxtApp().$hubApi; change them to
return nullable types instead (e.g., getProjects returns Promise<Project[] |
undefined> and getKongRoutes returns Promise<ListRoutes | undefined>), remove
the "as Project[]" / "as ListRoutes" casts and let the actual value (possibly
undefined) flow through from useNuxtApp().$hubApi, keeping the existing runtime
guards (e.g., checks like if (kongRoutesResp && kongRoutesResp.data)); update
any callers that assume non-nullable results to handle the new nullable return
types if necessary.
In `@app/plugins/api.ts`:
- Line 71: The assignment to errSvc in app/plugins/api.ts is using a redundant
nullish coalescing to undefined; remove the "?? undefined" part so errSvc is
simply assigned from response._data.detail?.service (i.e., delete the "??
undefined" token in the const errSvc = ... line) to clean up the code.
In `@test/mockapi/setup.ts`:
- Around line 87-89: The mock for useState currently returns a plain object {
value: "default" } but must return a real Vue Ref; update the useState mock
implementation to return a ref created with the imported ref helper (e.g.,
ref("default")) so components using reactivity helpers (watch, isRef, v-model)
behave correctly; locate the useState mock in test/mockapi/setup.ts and replace
the plain object return with a call to ref(...) preserving the same initial
value.
| const defaultFilters = { | ||
| global: { value: null, matchMode: FilterMatchMode.CONTAINS }, | ||
| approval_status: { value: null, matchMode: FilterMatchMode.EQUALS }, | ||
| "analysis.build_status": { value: null, matchMode: FilterMatchMode.IN }, | ||
| run_status: { value: null, matchMode: FilterMatchMode.IN }, | ||
| global: { value: undefined, matchMode: FilterMatchMode.CONTAINS }, | ||
| approval_status: { value: undefined, matchMode: FilterMatchMode.EQUALS }, | ||
| "analysis.build_status": { value: undefined, matchMode: FilterMatchMode.IN }, | ||
| run_status: { value: undefined, matchMode: FilterMatchMode.IN }, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PrimeVue 4 DataTable filters undefined vs null filter reset
💡 Result:
In PrimeVue v4 DataTable, you’ll get the most reliable “reset/clear filter” behavior by resetting filter values to null (not undefined) and keeping the filter keys/shape intact.
Why null works better than undefined:
- PrimeVue’s own DataTable filter examples initialize filter
valueasnull(e.g.,global: { value: null, ... }). Resetting back to that same state is what the component expects. [2] - Some built-in filter UIs treat
nullas the explicit “no selection / indeterminate” state (e.g. boolean Checkbox filter usesfilterModel.value === null). If you setundefined, you can end up with UI/state desync. [1] - The filter meta typing allows
matchModeto beundefined, but the usual “cleared” value in practice isvalue: null. [3]
Recommended reset patterns
1) Recreate the initial filters object (best)
const createFilters = () => ({
global: { value: null, matchMode: FilterMatchMode.CONTAINS },
name: { value: null, matchMode: FilterMatchMode.STARTS_WITH },
// ...
});
const filters = ref(createFilters());
function clearFilter() {
filters.value = createFilters();
}(PrimeVue docs use this “Clear” button approach with a clearFilter() handler.) [1]
2) If using menu filters with constraints/operators
Make sure you reset each constraint’s value to null while preserving operator/constraints structure.
filters.value.status.constraints[0].value = null;Sources: PrimeVue DataTable filtering docs (v4) [1], example filter model using null [2], DataTable filter meta typings [3].
🏁 Script executed:
fd AnalysesTable.vueRepository: PrivateAIM/node-ui
Length of output: 103
🏁 Script executed:
sed -n '320,335p' app/components/analysis/AnalysesTable.vueRepository: PrivateAIM/node-ui
Length of output: 577
🏁 Script executed:
sed -n '460,475p' app/components/analysis/AnalysesTable.vueRepository: PrivateAIM/node-ui
Length of output: 611
🏁 Script executed:
fd SearchBar.vueRepository: PrivateAIM/node-ui
Length of output: 96
🏁 Script executed:
cat -n app/components/table/SearchBar.vueRepository: PrivateAIM/node-ui
Length of output: 1693
Replace filter values of undefined with null to comply with PrimeVue 4 DataTable behavior.
PrimeVue 4 DataTable filters require value: null as the "no active filter" sentinel, not undefined. Using undefined causes UI/state desynchronization in filter operations. Update lines 326-331 in AnalysesTable.vue:
const defaultFilters = {
global: { value: null, matchMode: FilterMatchMode.CONTAINS },
approval_status: { value: null, matchMode: FilterMatchMode.EQUALS },
"analysis.build_status": { value: null, matchMode: FilterMatchMode.IN },
run_status: { value: null, matchMode: FilterMatchMode.IN },
};
Also update SearchBar.vue line 21 to emit null instead of undefined when clearing filters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/analysis/AnalysesTable.vue` around lines 326 - 331, The
DataTable filters use undefined as the "no filter" sentinel which breaks
PrimeVue 4; update the defaultFilters object in AnalysesTable.vue (symbol:
defaultFilters) to use value: null for global, approval_status,
"analysis.build_status", and run_status, and likewise modify the clear/emit
behavior in SearchBar.vue (symbol: the clear/emit function around line 21) so it
emits null instead of undefined when clearing filters to keep UI/state in sync.
| vi.mock( | ||
| "@sidebase/nuxt-auth", | ||
| async () => import("@/test/mockapi/nuxt-auth-mock"), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Confirm nuxt-auth-mock file exists
fd -t f "nuxt-auth-mock" --extension ts --extension js
# 2. List what it exports
rg -n "^export" --type ts -g "*/nuxt-auth-mock*"
# 3. Check what `@sidebase/nuxt-auth` symbols are actually consumed by test targets
rg -n "from ['\"]@sidebase/nuxt-auth['\"]" --type ts --type vue -C 2Repository: PrivateAIM/node-ui
Length of output: 286
🏁 Script executed:
#!/bin/bash
# 1. Read the nuxt-auth-mock file to see what it exports
echo "=== Content of nuxt-auth-mock.ts ==="
cat -n test/mockapi/nuxt-auth-mock.ts
# 2. Check the setup.ts context
echo "=== Content of setup.ts around lines 57-60 ==="
sed -n '50,70p' test/mockapi/setup.ts
# 3. Search for `@sidebase/nuxt-auth` imports (corrected syntax)
echo "=== Files importing `@sidebase/nuxt-auth` ==="
rg "@sidebase/nuxt-auth" --type ts -C 2Repository: PrivateAIM/node-ui
Length of output: 2493
🏁 Script executed:
#!/bin/bash
# Check tsconfig.json for path alias configuration
echo "=== tsconfig.json paths config ==="
cat tsconfig.json | grep -A 20 '"paths"'
# Check if there are any TypeScript compilation errors reported
echo "=== Check for tsc errors in setup.ts ==="
rg "setup.ts" --type ts -C 1 | head -20 || echo "No explicit errors found"
# List other uses of vi.mock in the codebase to see the pattern
echo "=== Other vi.mock usage patterns ==="
rg "vi\.mock" --type ts -C 1 | head -40Repository: PrivateAIM/node-ui
Length of output: 2305
Use a relative import path in the vi.mock factory for clarity.
The vi.mock factory currently uses the @ path alias to import the mock module, but Vitest recommends using relative paths within factory functions to ensure explicit, unambiguous module resolution. While Vite resolves the alias correctly at build time, using ./nuxt-auth-mock (relative to the current file) is the idiomatic pattern and matches the guidance in Vitest documentation.
♻️ Proposed fix
vi.mock(
"@sidebase/nuxt-auth",
- async () => import("@/test/mockapi/nuxt-auth-mock"),
+ async () => import("./nuxt-auth-mock"),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| vi.mock( | |
| "@sidebase/nuxt-auth", | |
| async () => import("@/test/mockapi/nuxt-auth-mock"), | |
| ); | |
| vi.mock( | |
| "@sidebase/nuxt-auth", | |
| async () => import("./nuxt-auth-mock"), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/mockapi/setup.ts` around lines 57 - 60, The vi.mock factory is importing
the mock using the path-alias "@/test/mockapi/nuxt-auth-mock"; replace that with
a relative import (e.g. './nuxt-auth-mock') inside the async factory so Vitest
resolves the module explicitly. Update the vi.mock call for
"@sidebase/nuxt-auth" to import the local module via a relative path to the
current file (referencing the nuxt-auth-mock module name in the factory).
1 participant
To improve security, Nuxt was updated to v4 and the dependencies were also updated accordingly. SSR is now disabled as it is not required and needs additional refactoring before it works properly
Summary by CodeRabbit
Refactor
New Features
Chores