Skip to content

Conversation

davemarco
Copy link
Contributor

@davemarco davemarco commented Jul 21, 2025

Description

Converts webui to npm workspace, and modifies taskfiles to deal with node modules in root and packages.

npm workspace allows us to install dependencies in common, like typebox, allowing for more shared types between client and server.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Package starts, and webui can compress and search logs.

Summary by CodeRabbit

  • New Features

    • Added a shared web UI "common" package with build, lint, and workspace integration.
  • Bug Fixes

    • Fixed server static path resolution to improve startup and asset serving reliability.
  • Chores

    • Renamed/scoped packages, updated dependencies/devDependencies, removed legacy path aliases, adjusted TypeScript/Vite/ESLint configs, migrated imports to the new common package, and expanded build/packaging and checksum orchestration to include common node_modules.

Copy link
Contributor

coderabbitai bot commented Jul 21, 2025

Walkthrough

Adds a new workspace package @webui/common, migrates many type-only imports from local/relative/alias paths to @webui/common, updates client/server package manifests and workspace scripts, removes local TS/Vite @common aliases, corrects launcher server/dist paths, and includes common in build/pack checksum and taskfile flows.

Changes

Cohort / File(s) Summary
Introduce @webui/common package
components/webui/common/package.json, components/webui/common/tsconfig.json, components/webui/common/eslint.config.mjs, components/webui/common/src/index.ts
Add new workspace package @webui/common with exports, strict TS config, ESLint config, build/lint scripts; export new TestTypeBoxSchema and add QueryId to exported types.
Client: manifest, imports, config
components/webui/client/package.json, components/webui/client/tsconfig/tsconfig.app.json, components/webui/client/vite.config.ts, components/webui/client/src/...
Rename package to @webui/client, add dependency on @webui/common, bump deps/devDeps, remove local ../common from TS includes and Vite @common alias/server.fs allow; update many type-only imports to @webui/common.
Server: manifest, imports, scripts, paths
components/webui/server/package.json, components/webui/server/tsconfig.json, components/webui/server/src/..., components/webui/server/src/routes/static.ts
Rename to @webui/server, add @webui/common dependency, move TypeScript to devDependencies, update start/dev/standalone scripts to use dist/src/* (was dist/server/*), remove @common TS paths/include, migrate many imports to @webui/common, adjust static root dirname (one fewer parent).
Launcher path corrections
components/clp-package-utils/clp_package_utils/scripts/start_clp.py
Fix start_webui() constants: server_settings_json_path → .../webui/server/dist/settings.json and node_cmd → .../webui/server/dist/src/main.js (remove extra server segment).
Workspace orchestration
components/webui/package.json
Add workspaces ["common","client","server"]; convert init and lint scripts to workspace-aware commands; add dev concurrency scripts; bump concurrently devDependency; remove per-package root scripts.
Build/packaging: include common in checksums and packaging
taskfile.yaml
Add G_WEBUI_COMMON_NODE_MODULES_CHECKSUM_FILE; include common in node_modules checksum compute/validate and packaging flows; add COMMON_OUTPUT_DIR and adjust rsync/build/clean/pack steps to handle common; parallelize checksum orchestration.
Socket / Mongo types import updates
components/webui/client/src/api/socket/*, components/webui/server/src/plugins/app/socket/*
Replace deep relative or old @common imports with @webui/common for socket event and QueryId/Response types (MongoSocketCollection, MongoSocketCursor, SocketSingleton, MongoWatcherCollection, typings).
Search / Presto type imports & routes
components/webui/client/src/pages/SearchPage/**, components/webui/server/src/routes/api/*, components/webui/server/src/plugins/app/Presto.ts
Switch imports of signals, types, and engine constants (SEARCH_SIGNAL, PRESTO_SEARCH_SIGNAL, CLP_QUERY_ENGINES, PrestoSearchResult, SearchResultsMetadataDocument, PrestoRowObject) to @webui/common.
Static & config path adjustments
components/webui/server/src/routes/static.ts, components/webui/client/src/config/index.ts
Adjust static root dirname resolution (one fewer ..) and update CLP_QUERY_ENGINES import to @webui/common.
ESLint adjustments
components/webui/server/eslint.config.mjs
Remove yscope-log-viewer/dist/ and client/dist/ from ESLint ignore so those directories are linted.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer
  participant Repo as Repository
  participant Common as @webui/common
  participant Client as @webui/client
  participant Server as @webui/server
  participant Launcher as start_clp.py

  Note over Dev,Repo: add common workspace, migrate type imports, update manifests, packaging, and launcher paths
  Dev->>Repo: add components/webui/common (package, tsconfig, eslint, src)
  Dev->>Client: update manifest, remove TS/Vite @common alias, change type imports -> @webui/common
  Dev->>Server: update manifest/scripts to dist/src/*, remove TS paths, change imports -> @webui/common
  Client->>Common: import types/schemas (ClientToServerEvents, Presto types, SEARCH_SIGNAL)
  Server->>Common: import signals/types (SEARCH_SIGNAL, QueryId, engine constants)
  Launcher->>Server: execute corrected node command: node dist/src/main.js
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • junhaoliao
  • hoophalab

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3039ab1 and 5e148d1.

📒 Files selected for processing (1)
  • taskfile.yaml (10 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#1119
File: taskfile.yaml:429-433
Timestamp: 2025-08-14T18:23:39.476Z
Learning: In the webui workspace, the exclude patterns ".tmp" and ".vite-temp" are Vite-specific temporary directories that only apply to the client package. The common and server packages use TypeScript compiler (tsc) directly and don't generate these Vite temporary files, so these exclude patterns should not be applied to their node_modules checksum computations.
📚 Learning: 2025-08-19T18:14:01.607Z
Learnt from: davemarco
PR: y-scope/clp#1119
File: taskfile.yaml:302-305
Timestamp: 2025-08-19T18:14:01.607Z
Learning: npm ci in workspaces can work without physical package.json files in all workspace directories if the workspace information is properly defined in package-lock.json. Built static assets (like from Vite builds) don't require runtime package.json files since they're self-contained. npm ci uses the lockfile for dependency resolution, not individual workspace manifests.

Applied to files:

  • taskfile.yaml
📚 Learning: 2025-08-14T18:23:39.476Z
Learnt from: junhaoliao
PR: y-scope/clp#1119
File: taskfile.yaml:429-433
Timestamp: 2025-08-14T18:23:39.476Z
Learning: In the webui workspace, the exclude patterns ".tmp" and ".vite-temp" are Vite-specific temporary directories that only apply to the client package. The common and server packages use TypeScript compiler (tsc) directly and don't generate these Vite temporary files, so these exclude patterns should not be applied to their node_modules checksum computations.

Applied to files:

  • taskfile.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: ubuntu-jammy-lint
  • GitHub Check: ubuntu-jammy-dynamic-linked-bins
  • GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
  • GitHub Check: ubuntu-jammy-static-linked-bins
  • GitHub Check: musllinux_1_2-x86_64-static-linked-bins
  • GitHub Check: centos-stream-9-static-linked-bins
  • GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
  • GitHub Check: centos-stream-9-dynamic-linked-bins
  • GitHub Check: manylinux_2_28-x86_64-static-linked-bins
  • GitHub Check: lint-check (macos-15)
  • GitHub Check: lint-check (ubuntu-24.04)
  • GitHub Check: build-macos (macos-15, true)
  • GitHub Check: build (macos-15)
  • GitHub Check: build-macos (macos-15, false)
🔇 Additional comments (10)
taskfile.yaml (10)

41-41: LGTM: checksum var added for common

Properly introduces G_WEBUI_COMMON_NODE_MODULES_CHECKSUM_FILE and is wired later.


88-88: LGTM: clean step covers common/node_modules

Keeps parity with other workspace packages.


269-269: LGTM: webui sources include common checksum

Ensures rebuilds trigger correctly when common’s node_modules change.


301-303: LGTM: server/package.json and root manifests copied

Matches the workspace layout expected by npm ci in the packaged webui.


355-361: LGTM: helpful note on five node_modules dirs

The comment accurately documents the checksum scope.


367-367: LGTM: COMMON_OUTPUT_DIR var

Consistent with other OUTPUT_DIR vars.


375-377: LGTM: consolidated sources globs

Brace globs for package.json and package-lock.json across packages are correct and keep sources concise.


380-381: LGTM: generates includes common checksum

Ensures checksum files are produced for all tracked node_modules, including common.


395-398: LGTM: common checksum validate without Vite excludes

Correctly omits .tmp/.vite-temp for common per prior guidance.


422-426: LGTM: common checksum compute

Consistent with validate step; no Vite-specific excludes applied to common.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@davemarco davemarco changed the title feat(webui): Combine server, client, and common into npm workspace; restructure taskfile to use new workspace feat(webui): Combine server, client, and common into npm workspace; restructure taskfile to use new workspace. Jul 21, 2025
Copy link
Member

@junhaoliao junhaoliao left a comment

Choose a reason for hiding this comment

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

review on the draft

Comment on lines +6 to +8
"common",
"client",
"server"
Copy link
Member

Choose a reason for hiding this comment

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

would it be better to put those under a packages/ directory? this seems to be the recommendation in the npm docs and a commonly used conventions in many projects.

Suggested change
"common",
"client",
"server"
"packages/common",
"packages/client",
"packages/server"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a preference, i dont mind changing but maybe in a different PR so we dont need to modify the taskfile that much in this pr

Copy link
Member

Choose a reason for hiding this comment

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

if we specify a components/webui/.npmrc file with

install-strategy = "nested"

("linked" might also work just fine though the command like to give a warning to say it might have bugs)

we can remove the init script in this file. any npm command like npm install and npm clean-install should install the node_modules inside the packages' own directory.

most importantly, the webui node_modules install task in taskfile.yaml will be much simplified

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried this and it dosent seem to actually install the node_modules in the packages directory.

Copy link
Member

@junhaoliao junhaoliao Aug 13, 2025

Choose a reason for hiding this comment

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

to get this install-strategy config into effect, we need to clean the existing node_modules and package-lock.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you know if this will keep nested package.lock? it if it dosent we still run into the same problem where we only have one lock file

Copy link
Member

Choose a reason for hiding this comment

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

i remember individual packages will have their own locks. this is fine to skip for now given the current structure already works

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these were not neccesary

@davemarco davemarco marked this pull request as ready for review August 11, 2025 17:01
@davemarco davemarco requested a review from a team as a code owner August 11, 2025 17:01
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 14

Caution

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

⚠️ Outside diff range comments (12)
components/webui/server/src/routes/static.ts (2)

48-51: Use the local variable when resolving clientDir (consistency/readability).

Use clientDir instead of settings.ClientDir to mirror the logViewerDir block and avoid accidental divergence if the variable is mutated earlier.

Apply this diff:

-    if (false === path.isAbsolute(clientDir)) {
-        clientDir = path.resolve(rootDirname, settings.ClientDir);
-    }
+    if (false === path.isAbsolute(clientDir)) {
+        clientDir = path.resolve(rootDirname, clientDir);
+    }

42-47: Consider SPA fallback for /log-viewer if deep-linking is needed.

If the log viewer is an SPA that uses client-side routing, direct navigation to /log-viewer/ will 404 without a fallback. You can add a catch-all to serve its index.html. Note: reply.sendFile is available because you set decorateReply: true on the root bundle; pass an explicit root to target the logViewerDir.

For example (place after the /log-viewer static registration):

fastify.get("/log-viewer/*", (_, reply) => {
  // Serve the log viewer's SPA entry for deep links
  reply.sendFile("index.html", { root: logViewerDir });
});

Please confirm @fastify/static version supports the options-root override for sendFile in your environment; if not, we can instead register a second decorated instance scoped to /log-viewer and use that decoration.

components/webui/common/src/index.ts (1)

35-49: Avoid the broad object type in public event payloads

Using object weakens type safety for payloads (hard to safely access). Prefer Record<string, unknown> or introduce generics parameterised by document shape.

For a minimal improvement:

 "collection::find::subscribe": (
   requestArgs: {
     collectionName: string;
-    query: object;
-    options: object;
+    query: Record<string, unknown>;
+    options: Record<string, unknown>;
   },
-  callback: (res: Response<{queryId: QueryId; initialDocuments: object[]}>) => void) => void;
+  callback: (res: Response<{queryId: QueryId; initialDocuments: Record<string, unknown>[]}>) => void) => void;
 …
 "collection::find::update": (respArgs: {
   queryId: QueryId;
-  data: object[];
+  data: Record<string, unknown>[];
 }) => void;

Optionally, make ClientToServerEvents generic, e.g., <TDoc extends Record<string, unknown> = Record<string, unknown>>, if you want stricter typing per collection.

Also applies to: 58-62

components/webui/client/src/config/index.ts (1)

14-16: Replace unsafe casts with a guarded enum extractor (fixes no-unsafe-assignment).

Direct casting from settings to enums violates @typescript-eslint/no-unsafe-assignment. Use a small type guard to validate at runtime.

Apply:

 enum CLP_STORAGE_ENGINES {
   CLP = "clp",
   CLP_S = "clp-s",
 }
 
-const SETTINGS_STORAGE_ENGINE = settings.ClpStorageEngine as CLP_STORAGE_ENGINES;
-const SETTINGS_QUERY_ENGINE = settings.ClpQueryEngine as CLP_QUERY_ENGINES;
+function ensureEnum<T extends string>(
+  value: unknown,
+  e: Record<string, T>,
+  name: string
+): T {
+  if ("string" === typeof value && (Object.values(e) as string[]).includes(value as T)) {
+    return value as T;
+  }
+  throw new Error(`Invalid ${name}: ${String(value)}`);
+}
+
+const SETTINGS_STORAGE_ENGINE = ensureEnum(settings.ClpStorageEngine, CLP_STORAGE_ENGINES, "ClpStorageEngine");
+const SETTINGS_QUERY_ENGINE = ensureEnum(settings.ClpQueryEngine, CLP_QUERY_ENGINES, "ClpQueryEngine");

This keeps types precise and satisfies the linter.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/index.tsx (1)

25-53: Address unsafe usages on dataSource and rowKey (fixes clp-lint errors).

The linter flags stem from possibly-unknown result types and accessing _id. Normalise the dataSource to a well-typed array and avoid relying on a field that Presto rows may not guarantee.

 const PrestoResultsVirtualTable = ({tableHeight}: PrestoResultsVirtualTableProps) => {
   const {updateNumSearchResultsTable} = useSearchStore();
-  const prestoSearchResults = usePrestoSearchResults();
+  const prestoSearchResults = usePrestoSearchResults();
+  const dataSource: PrestoSearchResult[] = useMemo(
+    () => (Array.isArray(prestoSearchResults) ? prestoSearchResults : []),
+    [prestoSearchResults]
+  );
@@
-  const columns = useMemo(
-      () => getPrestoSearchResultsTableColumns(prestoSearchResults || []),
-      [prestoSearchResults]
-  );
+  const columns = useMemo(
+    () => getPrestoSearchResultsTableColumns(dataSource),
+    [dataSource]
+  );
@@
-  useEffect(() => {
-    const num = prestoSearchResults ?
-        prestoSearchResults.length :
-        0;
+  useEffect(() => {
+    const num = dataSource.length;
     updateNumSearchResultsTable(num);
-  }, [
-    prestoSearchResults,
-    updateNumSearchResultsTable,
-  ]);
+  }, [dataSource, updateNumSearchResultsTable]);
@@
   return (
     <VirtualTable<PrestoSearchResult>
       columns={columns}
-      dataSource={prestoSearchResults || []}
+      dataSource={dataSource}
       pagination={false}
-      rowKey={(record) => record._id}
+      rowKey={(_record, index) => String(index)}
       scroll={{y: tableHeight}}/>
   );

If a stable natural key exists in the Presto result (e.g., a composite of row values), favour that over the index. I can help wire that if you confirm the schema.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (1)

11-30: Avoid unsafe member access on .row by narrowing with a type guard.

Linter errors indicate PrestoSearchResult may be treated as unknown/error-typed. Guard before accessing row and derive columns from the narrowed value.

 const getPrestoSearchResultsTableColumns = (
   data: PrestoSearchResult[]
 ): NonNullable<TableProps<PrestoSearchResult>["columns"]> => {
-  if (0 === data.length ||
-      "undefined" === typeof data[0] ||
-      "undefined" === typeof data[0].row
-  ) {
-      return [];
-  }
-
-  return Object.keys(data[0].row)
+  if (0 === data.length) {
+    return [];
+  }
+  const first: unknown = data[0] as unknown;
+  const isRowRecord = (v: unknown): v is { row: Record<string, unknown> } =>
+    "object" === typeof v && null !== v &&
+    "row" in (v as Record<string, unknown>) &&
+    "object" === typeof (v as { row?: unknown }).row &&
+    null !== (v as { row?: unknown }).row;
+  if (false == isRowRecord(first)) {
+    return [];
+  }
+  const row = (first as { row: Record<string, unknown> }).row;
+  return Object.keys(row)
     .map((key) => ({
       dataIndex: [
         "row",
         key,
       ],
       key: key,
       title: key,
     }));
 };

This removes unsafe member access while preserving behaviour. Note the use of false == predicate per repo guideline.

components/webui/server/src/routes/api/search/utils.ts (2)

62-67: Remove no-op collection call.

mongoDb.collection(searchResultCollectionName) on Line 63 creates a Collection instance that isn’t used. You immediately call mongoDb.collection(...).countDocuments() below.

Apply:

-        mongoDb.collection(searchResultCollectionName);
         numResultsInCollection = await mongoDb.collection(
             searchResultCollectionName
         ).countDocuments();

75-89: Type filter/update payloads to satisfy @typescript-eslint/no-unsafe- and improve safety.*

Since searchResultsMetadataCollection is a Collection, explicitly typing filter and modifier avoids unsafe assignment/member access warnings and guards against schema drift.

Apply:

+import type {Filter, UpdateFilter} from "mongodb";
+import type {SearchResultsMetadataDocument} from "./typings.js";
@@
-    const filter = {
+    const filter: Filter<SearchResultsMetadataDocument> = {
         _id: searchJobId.toString(),
         lastSignal: SEARCH_SIGNAL.RESP_QUERYING,
     };
-    const modifier = {
+    const modifier: UpdateFilter<SearchResultsMetadataDocument> = {
         $set: {
             lastSignal: SEARCH_SIGNAL.RESP_DONE,
             errorMsg: errorMsg,
             numTotalResults: Math.min(
                 numResultsInCollection,
                 SEARCH_MAX_NUM_RESULTS
             ),
         },
     };

Note: Importing the type from ./typings.js avoids introducing a second direct dependency on @webui/common in this file.

components/webui/server/src/routes/api/search/index.ts (1)

204-216: Type the filter and update objects in updateOne.

This avoids unsafe-member-access/assignment warnings and locks the update shape to the metadata schema.

Apply:

+import type {Filter, UpdateFilter} from "mongodb";
@@
-                await searchResultsMetadataCollection.updateOne(
-                    {
-                        _id: searchJobId.toString(),
-                        lastSignal: SEARCH_SIGNAL.RESP_QUERYING,
-                    },
-                    {
-                        $set: {
-                            errorMsg: "Query cancelled before it could be completed.",
-                            lastSignal: SEARCH_SIGNAL.RESP_DONE,
-                        },
-                    }
-                );
+                const filter: Filter<SearchResultsMetadataDocument> = {
+                    _id: searchJobId.toString(),
+                    lastSignal: SEARCH_SIGNAL.RESP_QUERYING,
+                };
+                const update: UpdateFilter<SearchResultsMetadataDocument> = {
+                    $set: {
+                        errorMsg: "Query cancelled before it could be completed.",
+                        lastSignal: SEARCH_SIGNAL.RESP_DONE,
+                    },
+                };
+                await searchResultsMetadataCollection.updateOne(filter, update);
components/webui/server/src/routes/api/presto-search/index.ts (1)

126-149: Harden error handling: don’t assume shape of Presto error.

error may not always be an Error with message/errorName. Guard accesses to satisfy @typescript-eslint/no-unsafe-member-access and future-proof.

Apply:

-                        error: (error) => {
-                            request.log.info(error, "Presto search failed");
+                        error: (error: unknown) => {
+                            request.log.info(error, "Presto search failed");
                             if (false === isResolved) {
                                 isResolved = true;
-                                reject(new Error("Presto search failed"));
+                                reject(new Error("Presto search failed"));
                             } else {
                                 searchResultsMetadataCollection.updateOne(
                                     {_id: searchJobId},
                                     {
                                         $set: {
-                                            errorMsg: error.message,
-                                            errorName: ("errorName" in error) ?
-                                                error.errorName :
-                                                null,
+                                            errorMsg: (error instanceof Error) ? error.message : String(error),
+                                            errorName: (typeof error === "object" && null !== error && "errorName" in error) ?
+                                                // @ts-expect-error: narrowing by `"errorName" in error`
+                                                (error as {errorName: string}).errorName :
+                                                null,
                                             lastSignal: PRESTO_SEARCH_SIGNAL.FAILED,
                                         },
                                     }
                                 ).catch((err: unknown) => {
                                     request.log.error(
                                         err,
                                         "Failed to update Presto error metadata"
                                     );
                                 });
                             }
                         },

This removes unsafe property access while preserving behaviour.

components/webui/server/src/routes/api/presto-search/utils.ts (1)

21-27: Optional: validate row/column length match.

If row.length < columns.length, properties will be set to undefined. If that’s acceptable, ignore. Otherwise, add a guard to avoid partial documents.

Example:

     columns.forEach((col, idx) => {
-        obj[col.name] = row[idx];
+        obj[col.name] = row[idx] ?? null;
     });

Or throw/log when lengths mismatch.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/usePrestoSearchResults.ts (1)

25-27: Gate verbose logging behind build env and use console.debug

Avoid noisy logs and leaking job IDs in production builds. Use a DEV-only guard and console.debug.

-            console.log(
-                `Subscribing to updates to Presto search results with job ID: ${searchJobId}`
-            );
+            if (false == import.meta.env.PROD) {
+                console.debug(
+                    `Subscribing to updates to Presto search results with job ID: ${searchJobId}`
+                );
+            }
♻️ Duplicate comments (10)
components/webui/client/package.json (2)

21-21: Aligned @sinclair/typebox to ^0.34.38 — looks good.

This resolves the prior mismatch with @webui/common. If the client still directly imports TypeBox (and not only via @webui/common), keeping it as a direct dependency is correct.


46-46: Pinning eslint-config-yscope is correct to keep CI stable.

The fixed version avoids unexpected lint breakages from upstream config changes.

components/webui/server/src/routes/static.ts (1)

16-19: Make rootDirname robust across src vs dist builds (avoid dev off-by-one).

As written, resolving with "../../.." points to different bases depending on whether this runs from src (dev) or dist (prod):

  • From src/.../server/src/routes → "../../.." resolves to components/webui (likely wrong if settings.json uses ../client/dist etc. relative to server).
  • From dist/.../server/dist/src/routes → "../../.." resolves to components/webui/server (likely correct).

To keep behaviour consistent and base relative settings off the server directory in both cases, derive the number of parent hops from the actual runtime path:

Apply this diff:

-    const rootDirname = path.resolve(dirname, "../../..");
+    const isDistBuild = filename.includes(`${path.sep}dist${path.sep}`);
+    // In dist: .../server/dist/src/routes → go up 3 to reach .../server
+    // In src:  .../server/src/routes      → go up 2 to reach .../server
+    const rootDirname = path.resolve(dirname, isDistBuild ? "../../.." : "../..");

Run this audit to confirm what the effective paths will be for your current settings.json values and ensure they resolve under the intended base (server vs webui):

#!/bin/bash
set -euo pipefail

settings_file="components/webui/server/settings.json"
if [ ! -f "$settings_file" ]; then
  echo "settings.json not found at $settings_file"
  fd -a -t f settings.json components/webui/server || true
  exit 1
fi

python - << 'PY'
import os, json
with open("components/webui/server/settings.json") as f:
  s = json.load(f)

pairs = [
  ("StreamFilesDir", s.get("StreamFilesDir")),
  ("LogViewerDir",  s.get("LogViewerDir")),
  ("ClientDir",     s.get("ClientDir")),
]

def resolve(base, val):
  if val is None: return None
  return val if os.path.isabs(val) else os.path.normpath(os.path.join(base, val))

candidates = {
  "server_base": os.path.normpath("components/webui/server"),
  "webui_base":  os.path.normpath("components/webui"),
}

print("settings.json values:")
for k,v in pairs: print(f"  {k}: {v}")

for name, base in candidates.items():
  print(f"\nBase={name}: {base}")
  for k,v in pairs:
    print(f"  {k}: {resolve(base, v)}")
PY
components/webui/common/tsconfig.json (1)

1-34: Enable project references; set rootDir for cleaner incremental builds

For workspace builds using tsc -b and to generate declarations correctly from src, make this package composite and set rootDir.

Apply:

   "compilerOptions": {
     "esModuleInterop": true,
     "lib": [
       "DOM",
-      "ES2022",
+      "ES2022"
     ],
     "target": "ES2022",
     "module": "esnext",
     "moduleResolution": "bundler",
     "declaration": true,
     "declarationMap": true,
     "outDir": "dist",
+    "composite": true,
+    "rootDir": "src",
     "allowUnreachableCode": false,
     "allowUnusedLabels": false,
     "alwaysStrict": true,
     "exactOptionalPropertyTypes": true,
     "noFallthroughCasesInSwitch": true,
     "noImplicitAny": true,
     "noImplicitOverride": true,
     "noImplicitReturns": true,
     "noImplicitThis": true,
     "noPropertyAccessFromIndexSignature": true,
     "noUncheckedIndexedAccess": true,
     "noUncheckedSideEffectImports": true,
     "noUnusedLocals": true,
     "noUnusedParameters": true,
     "strict": true,
     "useUnknownInCatchVariables": true,
   },
   "include": [
     "src/**/*"
-  ]
+  ],
+  "exclude": ["dist", "node_modules"]
components/webui/common/src/index.ts (2)

67-69: Replace empty interface with a type alias; remove lint suppression

Empty interfaces trigger Biome/TS lint. Prefer a zero‑property map alias and drop the disable.

Apply:

-// eslint-disable-next-line @typescript-eslint/no-empty-object-type
-interface InterServerEvents {
-}
+// Empty by design; Socket.IO requires a map of inter-server events (none used here).
+type InterServerEvents = Record<string, never>;

151-159: Prefer JSDoc @deprecated over TODO + lint suppression for the temporary schema

Use a deprecation notice for consumers; avoid disabling warning-comments.

Apply:

-/**
- * Test TypeBox schema for testing dependency.
- */
-// eslint-disable-next-line no-warning-comments
-// TODO: Will be removed once shared server/client route types are migrated into common.
+/**
+ * @deprecated Temporary helper to validate dependency wiring.
+ * Will be removed once shared server/client route types are migrated into @webui/common.
+ */
 const TestTypeBoxSchema = Type.Object({
     type: Type.String(),
 });
components/webui/server/src/plugins/app/Presto.ts (1)

1-1: import/no-unresolved for @webui/common on the server as well

Same root cause as the client: ESLint can’t resolve the workspace package. Apply the resolver/workspace adjustments noted in the client comment so server lint passes too.

Use the same verification script; additionally ensure server tsconfig includes the project reference to components/webui/common/tsconfig.json or that the ESLint resolver points at it.

taskfile.yaml (1)

289-293: Add sanity checks for common artefacts copied to OUTPUT_DIR.

Fail fast if common didn’t emit the expected dist files before proceeding.

         cd common
         PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm run build
         rsync -a package.json dist "{{.OUTPUT_DIR}}/common/"
+        test -f "{{.OUTPUT_DIR}}/common/dist/index.js" || { echo "common dist/index.js missing"; exit 1; }
+        test -f "{{.OUTPUT_DIR}}/common/dist/index.d.ts" || { echo "common dist/index.d.ts missing"; exit 1; }
components/webui/server/package.json (2)

9-13: Fix dist path mismatch in scripts (points to dist/src/ but tsc emits to dist/*).*

With include set to src/**/* and no rootDir, TypeScript infers rootDir=src and emits to dist/*. Update script paths.

-    "dev:start": "fastify start --ignore-watch=.ts$ -w -l info -P dist/src/app.js",
+    "dev:start": "fastify start --ignore-watch=.ts$ -w -l info -P dist/app.js",
-    "start": "npm run build && fastify start -l info dist/src/app.js",
+    "start": "npm run build && fastify start -l info dist/app.js",
-    "standalone": "npm run build && node --env-file=.env dist/src/main.js",
+    "standalone": "npm run build && node --env-file=.env dist/main.js",

33-36: Dependency hygiene: move @types/node to devDependencies.

Runtime doesn’t need @types/node; moving it reduces package size and aligns with #1116.

   "dependencies": {
@@
-    "@types/node": "^22.13.9",
     "@webui/common": "*",
@@
   "devDependencies": {
+    "@types/node": "^22.13.9",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 73e87be and 0ca50ed.

⛔ Files ignored due to path filters (1)
  • components/webui/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (19)
  • components/clp-package-utils/clp_package_utils/scripts/start_clp.py (2 hunks)
  • components/webui/client/package.json (3 hunks)
  • components/webui/client/src/config/index.ts (1 hunks)
  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/index.tsx (1 hunks)
  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/usePrestoSearchResults.ts (1 hunks)
  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (1 hunks)
  • components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts (1 hunks)
  • components/webui/client/vite.config.ts (0 hunks)
  • components/webui/common/src/index.ts (3 hunks)
  • components/webui/common/tsconfig.json (1 hunks)
  • components/webui/server/package.json (2 hunks)
  • components/webui/server/src/plugins/app/Presto.ts (1 hunks)
  • components/webui/server/src/routes/api/presto-search/index.ts (1 hunks)
  • components/webui/server/src/routes/api/presto-search/utils.ts (1 hunks)
  • components/webui/server/src/routes/api/search/index.ts (1 hunks)
  • components/webui/server/src/routes/api/search/typings.ts (1 hunks)
  • components/webui/server/src/routes/api/search/utils.ts (1 hunks)
  • components/webui/server/src/routes/static.ts (1 hunks)
  • taskfile.yaml (10 hunks)
💤 Files with no reviewable changes (1)
  • components/webui/client/vite.config.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}

⚙️ CodeRabbit configuration file

  • Prefer false == <expression> rather than !<expression>.

Files:

  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/index.tsx
  • components/webui/server/src/plugins/app/Presto.ts
  • components/webui/server/src/routes/api/presto-search/index.ts
  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts
  • components/webui/server/src/routes/api/search/index.ts
  • components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts
  • components/webui/client/src/config/index.ts
  • components/webui/server/src/routes/api/presto-search/utils.ts
  • components/webui/server/src/routes/api/search/typings.ts
  • components/webui/server/src/routes/api/search/utils.ts
  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/usePrestoSearchResults.ts
  • components/webui/common/src/index.ts
  • components/webui/server/src/routes/static.ts
🧠 Learnings (4)
📚 Learning: 2024-12-31T19:19:55.032Z
Learnt from: junhaoliao
PR: y-scope/clp#647
File: components/log-viewer-webui/server/src/main.ts:8-8
Timestamp: 2024-12-31T19:19:55.032Z
Learning: When "moduleResolution": "node16" is used in a TypeScript project's tsconfig.json, the import file extension is typically expected to be ".js" (or ".cjs"/".mjs"), even if the source code is originally in TypeScript (.ts). This is a part of node16’s resolution behavior.

Applied to files:

  • components/webui/common/tsconfig.json
📚 Learning: 2025-08-14T18:23:39.476Z
Learnt from: junhaoliao
PR: y-scope/clp#1119
File: taskfile.yaml:429-433
Timestamp: 2025-08-14T18:23:39.476Z
Learning: In the webui workspace, the exclude patterns ".tmp" and ".vite-temp" are Vite-specific temporary directories that only apply to the client package. The common and server packages use TypeScript compiler (tsc) directly and don't generate these Vite temporary files, so these exclude patterns should not be applied to their node_modules checksum computations.

Applied to files:

  • components/webui/common/tsconfig.json
  • taskfile.yaml
📚 Learning: 2025-06-16T13:05:27.349Z
Learnt from: davemarco
PR: y-scope/clp#1015
File: components/log-viewer-webui/server/src/routes/static.ts:65-70
Timestamp: 2025-06-16T13:05:27.349Z
Learning: In components/log-viewer-webui/server/src/routes/static.ts, when decorateReply is set to true in fastifyStatic configuration, the reply.sendFile() method automatically uses the root directory configured in the static plugin registration, eliminating the need to pass the root directory as a second parameter.

Applied to files:

  • components/webui/server/src/routes/static.ts
📚 Learning: 2025-08-19T18:14:01.607Z
Learnt from: davemarco
PR: y-scope/clp#1119
File: taskfile.yaml:302-305
Timestamp: 2025-08-19T18:14:01.607Z
Learning: npm ci in workspaces can work without physical package.json files in all workspace directories if the workspace information is properly defined in package-lock.json. Built static assets (like from Vite builds) don't require runtime package.json files since they're self-contained. npm ci uses the lockfile for dependency resolution, not individual workspace manifests.

Applied to files:

  • components/webui/server/package.json
  • taskfile.yaml
🧬 Code graph analysis (1)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
components/clp-package-utils/clp_package_utils/general.py (1)
  • get_clp_home (109-125)
🪛 GitHub Actions: clp-lint
components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/index.tsx

[error] 50-50: ESLint: Unsafe return of a value of type error (@typescript-eslint/no-unsafe-return)


[error] 50-50: ESLint: Unsafe member access ._id on an 'error' typed value (@typescript-eslint/no-unsafe-member-access)

components/webui/server/src/plugins/app/Presto.ts

[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)

components/webui/server/src/routes/api/presto-search/index.ts

[error] 9-9: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts

[error] 16-16: ESLint: Unsafe member access .row on an 'error' typed value. (@typescript-eslint/no-unsafe-member-access)


[error] 21-21: ESLint: Unsafe argument of type 'error' typed assigned to a parameter of type '{}'. (@typescript-eslint/no-unsafe-argument)


[error] 21-21: ESLint: Unsafe member access .row on an 'error' typed value. (@typescript-eslint/no-unsafe-member-access)

components/webui/server/src/routes/api/search/index.ts

[error] 9-9: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an 'error' typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an 'error' typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an 'error' typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 118-118: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 118-118: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 209-209: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 212-212: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 212-212: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)

components/webui/client/src/pages/SearchPage/SearchState/useUpdateStateWithMetadata.ts

[error] 6-6: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 28-28: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 35-35: ESLint: Unsafe member access .numTotalResults on an 'error' typed value (@typescript-eslint/no-unsafe-member-access)


[error] 36-36: ESLint: Unsafe argument of type error typed assigned to a parameter of type number (@typescript-eslint/no-unsafe-argument)


[error] 36-36: ESLint: Unsafe member access .numTotalResults on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 39-39: ESLint: Unsafe member access .lastSignal on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 40-40: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 41-41: ESLint: Unsafe member access .FINISHED on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 44-44: ESLint: Unsafe member access .FAILED on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 47-47: ESLint: Unsafe member access .errorName on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 54-54: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 54-54: ESLint: Unsafe member access .errorMsg on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 56-56: ESLint: Unsafe member access .numTotalResults on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 57-57: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 57-57: ESLint: Unsafe member access .errorName on an error typed value (@typescript-eslint/no-unsafe-member-access)

components/webui/client/src/config/index.ts

[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 15-15: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)

components/webui/server/src/routes/api/search/typings.ts

[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)

components/webui/server/src/routes/api/search/utils.ts

[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 77-77: ESLint: Unsafe member access .RESP_QUERYING on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 81-81: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)


[error] 81-81: ESLint: Unsafe member access .RESP_DONE on an error typed value (@typescript-eslint/no-unsafe-member-access)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 77-77: ESLint: Unsafe assignment of an error typed value (@typescript-eslint/no-unsafe-assignment)

🪛 Biome (2.1.2)
components/webui/common/src/index.ts

[error] 67-69: An empty interface is equivalent to {}.

Safe fix: Use a type alias instead.

(lint/suspicious/noEmptyInterface)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: build-macos (macos-15, false)
  • GitHub Check: build-macos (macos-15, true)
  • GitHub Check: centos-stream-9-dynamic-linked-bins
  • GitHub Check: centos-stream-9-static-linked-bins
  • GitHub Check: ubuntu-jammy-lint
  • GitHub Check: ubuntu-jammy-static-linked-bins
  • GitHub Check: ubuntu-jammy-dynamic-linked-bins
🔇 Additional comments (17)
components/webui/client/package.json (1)

48-48: Vite ^6.2.6 and plugin-react compatibility confirmed

  • Local inspection shows only one vite declaration in the workspace (components/webui/client/package.json uses "vite": "^6.2.6").
  • The @vitejs/plugin-react@^4.3.4 release explicitly added Vite 6 to its peerDependencies range (git.zsinfo.nl).

No further alignment required.

components/webui/server/src/routes/static.ts (1)

53-63: SPA catch-all correctly leverages decorateReply: true.

Using reply.sendFile("index.html") without an explicit root is correct here since the "/" static registration decorates the reply and anchors to clientDir. Also, the boolean checks adhere to our guideline (prefer false == expr over !expr). LGTM.

components/webui/common/tsconfig.json (1)

8-13: Target/lib alignment looks good

Target ES2022 with matching lib set keeps emitted JS and types in sync.

taskfile.yaml (9)

41-41: Good addition: dedicated checksum for common node_modules.

This aligns common with the other workspace packages and enables correct cache invalidation.


88-88: Clean step completeness.

Removing common/node_modules here avoids stale caches when switching branches.


269-271: WebUI sources now include common checksum – LGTM.

This ensures webui rebuilds when common’s node_modules change.


301-304: Packaging manifests flow—LGTM.

Copying server/package.json into OUTPUT_DIR/server and root {package.json, package-lock.json} into OUTPUT_DIR ensures npm ci resolves workspaces correctly at package time.


355-359: Docs comment updated to list common—LGTM.


367-371: COMMON_OUTPUT_DIR variable—LGTM.


375-377: Consolidated glob patterns for package manifests—LGTM.

The brace expansion keeps the sources tidy and accurate.


395-398: Checksum validate for common without Vite excludes—LGTM.

Consistent with the earlier learning: common uses tsc, so Vite-specific excludes don’t apply.


422-426: Compute checksum for common—LGTM.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/index.tsx (1)

6-7: Type-only import from @webui/common—LGTM.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (1)

1-1: Type-only import from @webui/common—LGTM.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/usePrestoSearchResults.ts (3)

1-1: LGTM: centralised type import from @webui/common

Good move to a workspace export and using a type-only import. This reduces fragile relative paths and keeps the runtime bundle clean.


21-23: Confirm sentinel equality is the intended “no job” check

The early return hinges on equality with SEARCH_STATE_DEFAULT.searchJobId. If the store ever emits undefined/null/"" for “no job”, this would subscribe unexpectedly. Verify the store guarantees this sentinel.


1-1: Verified @webui/common exports and client dependency

All checks passed—no further action needed:

  • components/webui/common/package.json exists and uses an “exports” field mapping "types": "./dist/index.d.ts".
  • src/index.ts defines interface PrestoSearchResult and re-exports it via export type { PrestoSearchResult }.
  • components/webui/client/package.json declares a dependency on @webui/common ("*").

The import type { PrestoSearchResult } from "@webui/common"; will resolve correctly.

Comment on lines +845 to 846
get_clp_home() / "var" / "www" / "webui" / "server" / "dist" / "settings.json"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Path correction to server settings.json looks right; verify dist packaging includes it

You removed the redundant "server" segment and now target .../webui/server/dist/settings.json. This aligns with the workspace/dist reshuffle. Please double‑check the server build produces and packages settings.json at dist root so this path won’t 404 at runtime.

Run:


🏁 Script executed:

#!/bin/bash
# Verify references to the new dist paths exist in the server sources/config.
rg -nC2 -e 'dist/settings\.json' -e 'dist/src/main\.js' components/webui/server || true

# Optionally, confirm the server package metadata aligns (main/exports/scripts) if present:
if [ -f components/webui/server/package.json ]; then
  cat components/webui/server/package.json
fi

Length of output: 2569


Ensure server build includes settings.json in dist

It looks like the server’s package.json defines the build step as just running tsc, with no provision to copy non-TS assets. Since TypeScript compilation alone won’t emit JSON files, settings.json won’t end up under dist/, causing a 404 at runtime when start_clp.py points to …/server/dist/settings.json.

Please address this by:

  • Updating the server’s build script (in components/webui/server/package.json) to copy settings.json into dist/.
    Example:
     "scripts": {
  • "build": "tsc",
  • "build": "tsc && cp src/settings.json dist/settings.json",

    }
- Alternatively, configure your build pipeline (or `tsconfig.json`) to include JSON assets so they’re emitted into `dist/`.
- After making this change, confirm that `components/webui/server/dist/settings.json` exists and that your CLP start-up script no longer 404s.



> Committable suggestion skipped: line range outside the PR's diff.

<details>
<summary>🤖 Prompt for AI Agents</summary>

In components/clp-package-utils/clp_package_utils/scripts/start_clp.py around
lines 845-846, the script expects server/dist/settings.json but the server build
only runs tsc and doesn't emit non-TS assets; update
components/webui/server/package.json build script to copy settings.json into
dist (or add a postbuild step or use a build tool to include JSON assets), or
configure tsconfig/build pipeline to include JSON files in the emitted output,
then run the build and verify components/webui/server/dist/settings.json exists
so start_clp.py will not 404.


</details>

<!-- fingerprinting:phantom:poseidon:chinchilla -->

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines 963 to 966
node_cmd = [
str(CONTAINER_CLP_HOME / "bin" / "node-22"),
str(container_webui_dir / "server" / "dist" / "server" / "src" / "main.js"),
str(container_webui_dir / "server" / "dist" / "src" / "main.js"),
]
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Entrypoint path updated to dist/src/main.js; confirm server build emits this file

Pointing node to server/dist/src/main.js matches the ESM layout change. Please ensure the server’s tsconfig/build scripts emit main.js under dist/src and that any Docker image or packaging step copies it (and its sourcemap if needed).

If you have a local build, quickly sanity‑check:


🏁 Script executed:

#!/bin/bash
# After building the server locally, confirm the entrypoint exists:
test -f components/webui/server/dist/src/main.js && echo "OK: main.js exists" || (echo "Missing dist/src/main.js"; exit 1)

Length of output: 118


Server entrypoint missing: critical fix required

The verification script indicates that components/webui/server/dist/src/main.js does not exist after building the server. Without this file, the CLP startup will fail at runtime.

Please address the issue by either:

  • Updating the server’s build configuration (e.g. tsconfig.json, build scripts) so that main.js is emitted under dist/src/.
  • Or, if the file is output elsewhere, adjusting the entrypoint path in components/clp-package-utils/clp_package_utils/scripts/start_clp.py to match the actual location.

Next steps:

  • Inspect components/webui/server/tsconfig.json and build scripts to confirm the "outDir" and entry file settings.
  • Re-run the build locally and verify with:
    test -f components/webui/server/dist/src/main.js \
      && echo "OK: main.js exists" \
      || (echo "Missing dist/src/main.js"; exit 1)
  • Update start_clp.py once the correct path is determined.
🤖 Prompt for AI Agents
In components/clp-package-utils/clp_package_utils/scripts/start_clp.py around
lines 963-966, the node_cmd entrypoint points to
components/webui/server/dist/src/main.js which the build does not produce;
inspect components/webui/server/tsconfig.json and the server build scripts to
confirm the configured outDir and emitted entry file (or run a local build),
then either update the build config so main.js is emitted to dist/src/ or change
the node_cmd path to the actual built file location (e.g., dist/main.js or
dist/server/main.js as determined); after adjusting, rebuild and verify
existence with the provided test command before committing the path change.

@@ -1,5 +1,5 @@
{
"name": "webui-client",
"name": "@webui/client",
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Good rename to scoped package; consider marking the package as private.

The new name @webui/client aligns with the workspace structure. To avoid accidental publishes from a leaf app package, add "private": true.

Outside-the-hunk example (apply near the top of the manifest):

{
  "name": "@webui/client",
  "private": true,
  ...
}
🤖 Prompt for AI Agents
In components/webui/client/package.json around line 2, the package was renamed
to the scoped name "@webui/client" but is not marked private; add a "private":
true field near the top-level of the manifest (alongside name and version) to
prevent accidental npm publishes from this leaf app package.

"@sinclair/typebox": "^0.34.38",
"@tanstack/react-query": "^5.81.5",
"@tanstack/react-query-devtools": "^5.81.5",
"@webui/common": "*",
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace wildcard dependency on @webui/common with a pinned workspace range.

Using "*" risks pulling a registry version if it ever exists and hurts reproducibility. Prefer a pinned semver matching @webui/common’s version so npm links the workspace package deterministically.

Apply one of the following (recommended first):

  • If using npm workspaces (no workspace: protocol): set to the exact version range declared in components/webui/common/package.json (example shown).
-    "@webui/common": "*",
+    "@webui/common": "^0.1.0",
  • If your toolchain supports it (Yarn/pnpm), use the workspace protocol:
-    "@webui/common": "*",
+    "@webui/common": "workspace:*",
📝 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.

Suggested change
"@webui/common": "*",
"@webui/common": "^0.1.0",
🤖 Prompt for AI Agents
In components/webui/client/package.json around line 24, the dependency uses the
wildcard "*" for "@webui/common" which can pull a registry package and breaks
reproducibility; replace it with a pinned workspace range by reading the version
field from components/webui/common/package.json and setting "@webui/common" to
that exact semver (e.g. "1.2.3") or, if your toolchain supports workspace
protocol, set it to the workspace reference (e.g. "workspace:^1.2.3" or
"workspace:*"); ensure the selected form matches your monorepo package manager
(npm workspaces use the exact version range, Yarn/pnpm can use the workspace
protocol) and update package.json accordingly.

@@ -1,4 +1,5 @@
import {CLP_QUERY_ENGINES} from "../../../common/index.js";
import {CLP_QUERY_ENGINES} from "@webui/common";
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Fix module resolution for @webui/common (import/no-unresolved).

ESLint can’t resolve the workspace package. Ensure the client declares and resolves the workspace dependency and that lint runs against the correct TS project.

  • Add @webui/common as a dependency of client (use "*" for npm workspaces).
  • Ensure the common package exposes compiled JS and types via "exports" and "types".
  • Point ESLint’s typescript resolver at the client tsconfig (or a root tsconfig.eslint.json that includes client).

Run this script to confirm the wiring:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check client depends on @webui/common =="
jq -r '.dependencies["@webui/common"], .devDependencies["@webui/common"]' components/webui/client/package.json 2>/dev/null

echo "== Check common package exposes exports and types =="
jq '{name, main, module, types, exports}' components/webui/common/package.json

echo "== Confirm tsconfig path for client (module resolution) =="
jq '.compilerOptions | {moduleResolution, baseUrl, paths, types}' components/webui/client/tsconfig.json

echo "== Print ESLint config resolver hints (if present) =="
# These files are common; adapt if your repo uses a different path
for f in .eslintrc.cjs .eslintrc.js components/webui/client/.eslintrc.cjs components/webui/.eslintrc.cjs; do
  [ -f "$f" ] && { echo "--- $f ---"; cat "$f"; }
done

Length of output: 616


🏁 Script executed:

#!/bin/bash
set -u -o pipefail

echo "== Check client depends on @webui/common =="
jq -r '.dependencies["@webui/common"], .devDependencies["@webui/common"]' components/webui/client/package.json 2>/dev/null || echo "not found"

echo
echo "== Check common package exposes exports and types =="
jq '{name: .name, main: .main, module: .module, types: .types, exports: .exports}' components/webui/common/package.json 2>/dev/null || echo "failed to parse common/package.json"

echo
echo "== Confirm tsconfig path for client (module resolution) =="
jq '.compilerOptions | {moduleResolution: .moduleResolution, baseUrl: .baseUrl, paths: .paths, types: .types}' components/webui/client/tsconfig.json 2>/dev/null || echo "failed to parse client/tsconfig.json"

echo
echo "== Print ESLint resolver settings (if present) =="
for f in .eslintrc.cjs .eslintrc.js components/webui/client/.eslintrc.cjs components/webui/.eslintrc.cjs; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    sed -n '1,200p' "$f"
    echo
  fi
done

Length of output: 1732


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check existence and permissions of tsconfig.json =="
ls -l components/webui/client/tsconfig.json || { echo "tsconfig.json not found"; exit 1; }

echo
echo "== Print first 200 lines of tsconfig.json =="
sed -n '1,200p' components/webui/client/tsconfig.json

Length of output: 729


Configure ESLint for TypeScript project resolution in client

The client already declares "@webui/common": "*" in its dependencies, and the common package exposes compiled JS and types via its "exports" field. However, ESLint’s import resolver isn’t pointed at the client’s TS project, so imports of workspace packages (e.g. @webui/common) trigger import/no-unresolved.

Actions:

  • Create or update components/webui/client/.eslintrc.cjs with:
    module.exports = {
      parserOptions: {
        project: ['./tsconfig/tsconfig.app.json']
      },
      settings: {
        'import/resolver': {
          typescript: {
            project: ['./tsconfig/tsconfig.app.json']
          }
        }
      }
    };
  • Or, in your root ESLint config (e.g. .eslintrc.cjs or .eslintrc.js), add the client’s TS project under both parserOptions.project and settings.import/resolver.typescript.project:
    parserOptions: {
    + project: ['components/webui/client/tsconfig/tsconfig.app.json', /* other projects */],
    },
    settings: {
      'import/resolver': {
        typescript: {
    +     project: ['components/webui/client/tsconfig/tsconfig.app.json', /* other projects */],
        }
      }
    }
  • Verify that components/webui/client/tsconfig/tsconfig.app.json includes any necessary compilerOptions.baseUrl or paths entries if you’re using custom path aliases for workspace packages.

These changes will allow ESLint to resolve workspace packages correctly and eliminate the import/no-unresolved errors.

🧰 Tools
🪛 GitHub Actions: clp-lint

[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)

🤖 Prompt for AI Agents
components/webui/client/src/config/index.ts lines 1-1: ESLint is flagging
imports of workspace packages like "@webui/common" as unresolved because the
client's ESLint parser/import resolver isn't pointed at the client's TypeScript
project; update the client's ESLint config
(components/webui/client/.eslintrc.cjs) or the root ESLint config to set
parserOptions.project and settings.import/resolver.typescript.project to
['./tsconfig/tsconfig.app.json'] so the resolver uses the client tsconfig, and
ensure components/webui/client/tsconfig/tsconfig.app.json has any required
baseUrl/paths for workspace aliases.

@@ -1,10 +1,9 @@
import type {PrestoRowObject} from "@webui/common";
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Resolve @webui/common import for PrestoRowObject type.

Same workspace import resolution fix required. Ensure @webui/common exports PrestoRowObject in its public API and ESLint can resolve it via the tsconfig used for lint.


🏁 Script executed:

#!/bin/bash
# Locate the package.json that declares @webui/common
echo "Locating @webui/common in package.json files..."
rg -n '"name"\s*:\s*"@webui/common"' -g '*/package.json'

# Determine its directory
COMMON_PKG_JSON=$(rg -l '"name"\s*:\s*"@webui/common"' -g '*/package.json')
COMMON_DIR=$(dirname "$COMMON_PKG_JSON")
echo "Found @webui/common in: $COMMON_DIR"

# Search for PrestoRowObject type exports within that package
echo "Searching for PrestoRowObject exports in $COMMON_DIR..."
rg -nP 'export\s+(?:type\s+)?\{?PrestoRowObject\}?' "$COMMON_DIR"

Length of output: 787


#!/bin/bash
set -e

echo "Locating @webui/common package.json files..."
# Find all package.json files and check for the @webui/common name
fd -t f -e json package.json | while read pkg; do
  if grep -q '"name"\s*:\s*"@webui/common"' "$pkg"; then
    echo "→ Found @webui/common in: $pkg"
    PKG_DIR=$(dirname "$pkg")

    echo "Checking public exports in $PKG_DIR/src/index.ts (if exists)..."
    if [ -f "$PKG_DIR/src/index.ts" ]; then
      rg -n 'export.*PrestoRowObject' "$PKG_DIR/src/index.ts" \
        || echo "  – No direct export of PrestoRowObject in index.ts"
    else
      echo "  – No src/index.ts found"
    fi

    echo "Searching for PrestoRowObject definition within @webui/common package..."
    rg -n 'type\s\+PrestoRowObject' -C2 "$PKG_DIR" \
      || rg -n 'interface\s\+PrestoRowObject' -C2 "$PKG_DIR" \
      || echo "  – No type or interface named PrestoRowObject in @webui/common"
  fi
done

echo "Performing global search for PrestoRowObject across the entire repo..."
rg -n 'PrestoRowObject' -C2 .

🏁 Script executed:

#!/bin/bash
set -e

echo "🔍 Searching for PrestoRowObject definition across the repository..."
rg -nP "type\s+PrestoRowObject|interface\s+PrestoRowObject" -C3 .

echo
echo "🔍 Searching for PrestoRowObject in export statements..."
rg -n "export\s+.*PrestoRowObject" -C3 .

Length of output: 862


Export PrestoRowObject in @webui/common public API

The PrestoRowObject interface is defined at components/webui/common/src/index.ts:140–142 but isn’t exported, so importing it from @webui/common will fail. Please update the public API to expose this type and ensure ESLint/TS can resolve it.

• File needing change:
components/webui/common/src/index.ts

• Suggested fix:

--- a/components/webui/common/src/index.ts
+++ b/components/webui/common/src/index.ts
@@ -138,7 +138,7 @@
  * Presto row wrapped in a `row` property to prevent conflicts with MongoDB's `_id` field.
  */
-interface PrestoRowObject {
+export interface PrestoRowObject {
   row: Record<string, unknown>;
 }

Verify that after this change you can successfully import with:

import type { PrestoRowObject } from "@webui/common";

and that ESLint/TS resolves it without errors.

🤖 Prompt for AI Agents
In components/webui/common/src/index.ts (top-level public API), the
PrestoRowObject type defined at components/webui/common/src/index.ts:140-142 is
not exported so importing it from "@webui/common" fails; fix by re-exporting the
type from the public index (for example add an export line that exposes
PrestoRowObject, e.g. export type { PrestoRowObject } from
"./<relative-module-containing-definition>" or move the interface into this
index and export it), then run TypeScript/ESLint to verify import with import
type { PrestoRowObject } from "@webui/common" resolves without errors.

Comment on lines +9 to +11
} from "@webui/common";
import {StatusCodes} from "http-status-codes";

Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Resolve @webui/common import for lint/build in the workspace.

Same unresolved workspace module problem as in utils.ts. Please:

  • Add @webui/common to server/package.json (workspace:*).
  • Ensure @webui/common/package.json declares exports/types.
  • Point ESLint to a tsconfig that can resolve @webui/common during lint (paths just for lint).

See repo-level diffs below.

Run the verification script included below.


🏁 Script executed:

#!/bin/bash
set -e

echo "=== Locating and displaying components/webui/server/package.json ==="
if [ -f components/webui/server/package.json ]; then
  cat components/webui/server/package.json
else
  echo "components/webui/server/package.json not found"
fi

echo
echo "=== Locating and displaying @webui/common/package.json ==="
common_pkg=$(fd --type f -g 'common/package.json' | grep 'webui/common' | head -n1 || true)
if [ -n "$common_pkg" ] && [ -f "$common_pkg" ]; then
  echo "Found at $common_pkg:"
  cat "$common_pkg"
else
  echo "@webui/common/package.json not found in workspace"
fi

echo
echo "=== Displaying root and server tsconfig files ==="
if [ -f tsconfig.json ]; then
  echo "tsconfig.json:"
  cat tsconfig.json
else
  echo "Root tsconfig.json not found"
fi

echo
if [ -f components/webui/server/tsconfig.json ]; then
  echo "components/webui/server/tsconfig.json:"
  cat components/webui/server/tsconfig.json
else
  echo "components/webui/server/tsconfig.json not found"
fi

echo
echo "=== Displaying ESLint configuration files ==="
fd --type f -g ".eslintrc*" | while read -r file; do
  echo "---- $file ----"
  cat "$file"
done

Length of output: 4140


🏁 Script executed:

#!/bin/bash
set -e

echo "=== Root package.json ==="
if [ -f package.json ]; then
  cat package.json
else
  echo "package.json not found at root"
fi

echo
echo "=== Workspaces field in root package.json ==="
grep -R '"workspaces"' -n package.json || echo "No workspaces section in root package.json"

echo
echo "=== Locating any common modules under components/webui ==="
fd --type d --max-depth 3 -g 'common' components/webui || echo "No common directory under components/webui"

echo
echo "=== Locating any package.json named common/package.json anywhere ==="
fd --type f -g 'common/package.json' || echo "No common/package.json found in workspace"

Length of output: 864


🏁 Script executed:

#!/bin/bash
set -e

echo "=== Checking monorepo workspace config files ==="
for file in pnpm-workspace.yaml yarn.lock pnpm-lock.yaml package.json; do
  echo "--- $file ---"
  if [ -f $file ]; then
    cat $file
  else
    echo "not found"
  fi
done

echo
echo "=== Listing contents of components/webui/common ==="
if [ -d components/webui/common ]; then
  ls -R components/webui/common
else
  echo "components/webui/common directory not found"
fi

echo
echo "=== Searching for tsconfig files under components/webui/common ==="
fd --type f -g 'tsconfig*.json' components/webui/common || echo "none found"

Length of output: 1395


🏁 Script executed:

#!/bin/bash
set -e

echo "=== components/webui/common/package.json ==="
cat components/webui/common/package.json

echo
echo "=== components/webui/common/tsconfig.json ==="
cat components/webui/common/tsconfig.json

Length of output: 1680


🏁 Script executed:

#!/bin/bash
set -e

echo "=== Searching for ESLint config files in components/webui ==="
find components/webui -maxdepth 4 -type f \( -name ".eslintrc*" -o -name "eslint.config.*" \) -print -exec sed -n '1,200p' {} \;

Length of output: 4254


Resolve missing path mapping for @webui/common in server

The dependency and package exports are correctly declared—@webui/common is listed in components/webui/server/package.json and its own package.json provides "exports" and "types". The remaining blocker is that TypeScript and ESLint can’t locate the local package without explicit path mappings or workspace linkage.

Please apply the following fixes:

  • Ensure your monorepo tool links components/webui/common into the server:
    • If you use Yarn or pnpm workspaces, add a root package.json (or pnpm-workspace.yaml) listing
      "workspaces": ["components/webui/common", "components/webui/server"]
      
  • In components/webui/server/tsconfig.json, add a paths section under compilerOptions to point at the local source:
     {
       "compilerOptions": {
         "baseUrl": ".",
  •  "paths": {
    
  •    "@webui/common": ["../common/src/index.ts"],
    
  •    "@webui/common/*": ["../common/src/*"]
    
  •  },
     /* existing options */
    
    },
    "include": ["src/**/*"]
    }
  • To get ESLint to recognise these paths, either:
    • Create a specialized tsconfig (e.g. tsconfig.lint.json) that extends the above and then in components/webui/server/eslint.config.mjs add a createTsConfigOverride entry pointing at it, or
    • Extend the existing ESLint config’s TsConfigArray with a custom override:
      import {createTsConfigOverride} from "eslint-config-yscope/TsConfigArray.mjs";
      
      // inside EslintConfig array:
      createTsConfigOverride(
        ["src/**/*.ts"],
        "./tsconfig.json"  // now includes your paths
      ),

This will allow both the TypeScript compiler and ESLint to resolve imports from @webui/common without breaking the build or lint.

🧰 Tools
🪛 GitHub Actions: clp-lint

[error] 9-9: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)

🤖 Prompt for AI Agents
In components/webui/server/src/routes/api/search/index.ts around lines 9-11 the
import from "@webui/common" cannot be resolved by TypeScript/ESLint because the
server package lacks workspace/path mappings to the local common package; fix by
ensuring your monorepo links components/webui/common into the server (add
workspaces in root package.json or pnpm-workspace.yaml including both
components/webui/common and components/webui/server), then add a "paths" mapping
in components/webui/server/tsconfig.json under compilerOptions to point the
"@webui/common" alias to the local source, and update ESLint to use a tsconfig
that includes these paths (either create a tsconfig.lint.json that extends
tsconfig.json and reference it via createTsConfigOverride in eslint.config.mjs,
or add a TsConfigArray override pointing to the updated tsconfig.json) so both
TypeScript and ESLint can resolve the import.

@@ -1,3 +1,4 @@
import {type SearchResultsMetadataDocument} from "@webui/common";
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Resolve @webui/common type import.

As with the other files, ESLint can’t resolve @webui/common. Implement the workspace + ESLint resolver fixes. Also confirm that this file re-exports SearchResultsMetadataDocument intentionally (it does), and keep the single source-of-truth in @webui/common.


🏁 Script executed:

#!/bin/bash
# List ESLint configuration to verify import resolver settings
fd .eslintrc.* -x cat

# Search for import/resolver configuration in ESLint
rg -n "import/resolver" -C3 .eslintrc.js

# Inspect tsconfig.json for path mappings, especially @webui/common
sed -n '1,200p' tsconfig.json
grep -R '"@webui/common"' -n tsconfig.json

# Show the contents of the typings file to confirm its exports and imports
sed -n '1,200p' components/webui/server/src/routes/api/search/typings.ts

Length of output: 1224


🏁 Script executed:

#!/bin/bash
# Find any ESLint config files and display them
fd .eslintrc* -x cat

# Locate all tsconfig.json files
fd tsconfig*.json

# Display root package.json and check for workspaces & eslintConfig
if [ -f package.json ]; then
  sed -n '1,200p' package.json
  echo "----- Workspaces in package.json -----"
  grep -R '"workspaces"' -n package.json || echo "no workspaces field found"
  echo "----- eslintConfig in package.json -----"
  grep -R '"eslintConfig"' -n package.json || echo "no eslintConfig field found"
fi

# Show contents of each tsconfig.json
for cfg in $(fd tsconfig*.json); do
  echo "===== $cfg ====="
  sed -n '1,200p' "$cfg"
done

Length of output: 2923


🏁 Script executed:

#!/bin/bash
# Find package.json in each subproject under components/webui
fd components/webui -g package.json -x echo "Found package.json: {}"
# Display workspaces and eslintConfig fields for each package.json
for f in $(fd components/webui -g package.json); do
  echo "### $f ###"
  grep -R '"workspaces"' -n "$f" || echo "no workspaces field"
  grep -R '"eslintConfig"' -n "$f" || echo "no eslintConfig field"
done

# Locate any ESLint configuration files in subprojects
fd components/webui -g ".eslintrc*" -x echo "Found ESLint config: {}"
for cfg in $(fd components/webui -g ".eslintrc*"); do
  echo "=== $cfg ==="
  sed -n '1,200p' "$cfg"
done

# Check tsconfig.json for path mappings to @webui/common
for cfg in $(fd components/webui -g tsconfig.json); do
  echo "===== $cfg ====="
  grep -R '"paths"' -n "$cfg" || echo "no paths section"
  grep -R '"@webui/common"' -n "$cfg" || echo "no alias for @webui/common"
done

Length of output: 757


🏁 Script executed:

#!/bin/bash
# Inspect root package.json for workspaces and ESLint config
if [ -f package.json ]; then
  echo "===== Root package.json ====="
  sed -n '1,200p' package.json
  echo "----- workspaces -----"
  grep -n '"workspaces"' package.json || echo "no workspaces field"
  echo "----- eslintConfig -----"
  grep -n '"eslintConfig"' package.json || echo "no eslintConfig field"
else
  echo "No root package.json found"
fi

# Inspect root tsconfig.json for path mappings
if [ -f tsconfig.json ]; then
  echo "===== Root tsconfig.json ====="
  sed -n '1,200p' tsconfig.json
  echo "----- paths -----"
  grep -n '"paths"' tsconfig.json || echo "no paths section"
else
  echo "No root tsconfig.json found"
fi

# Locate any ESLint configuration files at root
for file in .eslintrc.js .eslintrc.json .eslintrc.yaml .eslintrc; do
  if [ -f "$file" ]; then
    echo "===== ESLint config: $file ====="
    sed -n '1,200p' "$file"
  fi
done

Length of output: 553


Resolve @webui/common import for TypeScript and ESLint
ESLint and the TypeScript compiler currently can’t locate the @webui/common package in the server code. Please implement the workspace and resolver fixes below, and note that re-exporting the SearchResultsMetadataDocument type is intentional to maintain a single source-of-truth in @webui/common.

• In components/webui/server/tsconfig.json, add a paths mapping for @webui/common under compilerOptions:

 {
   "compilerOptions": {
     "baseUrl": ".",
+    "paths": {
+      "@webui/common": ["../common/src"]
+    },
     // …existing settings…
   },
   "include": [
     "src/**/*",
   ]
 }

• In your ESLint configuration (e.g. .eslintrc.js at the repo root), configure the TypeScript import resolver to cover all workspaces:

module.exports = {
  // …other ESLint settings…
  settings: {
    'import/resolver': {
      typescript: {
        project: [
          'components/webui/common/tsconfig.json',
          'components/webui/server/tsconfig.json',
          'components/webui/client/tsconfig.json',
        ],
      },
    },
  },
};

• Confirm that the bottom of components/webui/server/src/routes/api/search/typings.ts intentionally re-exports

export { SearchResultsMetadataDocument };

to surface the common metadata type without redefining it.

🧰 Tools
🪛 GitHub Actions: clp-lint

[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)

🤖 Prompt for AI Agents
In components/webui/server/src/routes/api/search/typings.ts around lines 1 to 1,
ESLint and TypeScript cannot resolve the @webui/common import; add a "paths"
entry for "@webui/common" in components/webui/server/tsconfig.json under
compilerOptions to point to the common workspace (so the server TS project can
locate the package), update the root ESLint config (e.g. .eslintrc.js)
import/resolver typescript.project array to include the three workspace
tsconfig.json files listed in the review so ESLint's resolver covers all
workspaces, and leave the final line re-exporting SearchResultsMetadataDocument
in this typings.ts file unchanged to intentionally surface the shared type.

@@ -1,6 +1,6 @@
import {SEARCH_SIGNAL} from "@webui/common";
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Fix unresolved import to @webui/common (ESLint import/no-unresolved) and cascade type errors.

ESLint can’t resolve the workspace package. This also cascades into “unsafe assignment/member access” on SEARCH_SIGNAL because the symbol becomes any/unknown when the import fails.

Recommend:

  • Ensure @webui/common is a real workspace package with proper exports/types.
  • Add @webui/common as a dependency of the server package (workspace:*).
  • Point ESLint’s import resolver at a tsconfig that knows how to resolve @webui/common during lint. Easiest: add a tsconfig.eslint.json with a paths entry only for lint, and update ESLint settings to use it.

See diffs below under “Repo-level config” for concrete patches. After this, the no-unresolved and the unsafe-member-access on SEARCH_SIGNAL should disappear.

Run the verification script at the end of this review to confirm resolution.


🏁 Script executed:

#!/bin/bash
# Check workspace definitions in root package.json
rg -n '"workspaces"' -g 'package.json' || echo "No workspaces found"

# Check paths in root tsconfig.json
rg -n '"paths"' -g 'tsconfig*.json' || echo "No paths found in tsconfig"

# Find server package.json
fd --type f --max-depth 4 'package.json' components/webui/server || echo "Server package.json not found"

# Find common package.json
fd --type f --max-depth 4 'package.json' components/webui/common || echo "Common package.json not found"

# Find server tsconfig.json
fd --type f --max-depth 4 'tsconfig*.json' components/webui/server || echo "Server tsconfig not found"

# Find ESLint configuration files
fd --hidden --type f --max-depth 3 '.eslintrc' || echo "ESLint config not found"

Length of output: 547


🏁 Script executed:

#!/bin/bash
# List all tsconfig files in the repo
fd --type f -g 'tsconfig*.json'

# Show the workspace config in components/webui/package.json
sed -n '1,200p' components/webui/package.json

echo "--- Common package.json ---"
sed -n '1,200p' components/webui/common/package.json

echo "--- Server package.json ---"
sed -n '1,200p' components/webui/server/package.json

echo "--- Server tsconfig.json ---"
sed -n '1,200p' components/webui/server/tsconfig.json

echo "--- ESLint config files ---"
fd --type f -g '.eslintrc*' || echo "No ESLint config found"

Length of output: 4888


Action Required: Address workspace import resolution and lint configuration

The verification shows that:

  • The root components/webui/package.json correctly lists "common", "client", and "server" as workspaces.
  • components/webui/common/package.json exports both JavaScript and TypeScript types.
  • components/webui/server/package.json declares a dependency on @webui/common using "*" rather than the workspace protocol.
  • There are no TS path mappings for @webui/common in any tsconfig*.json, nor is there an ESLint config file to guide import resolution.

Next steps:

  • In components/webui/server/package.json, change the dependency specifier for @webui/common from "*" to "workspace:*" to ensure it is symlinked as a workspace dependency.
  • Add a paths mapping in a TS config (either in components/webui/server/tsconfig.json or in a new tsconfig.eslint.json) so that TypeScript (and ESLint’s TypeScript resolver) can resolve:
    // tsconfig.eslint.json
    {
      "extends": "./tsconfig.json",
      "compilerOptions": {
        "baseUrl": ".",
        "paths": {
          "@webui/common": ["../common/src"]
        }
      }
    }
  • Create or update the project’s ESLint configuration (e.g. .eslintrc.json at the repo root) to use the TypeScript resolver and point it at tsconfig.eslint.json:
    {
      "settings": {
        "import/resolver": {
          "typescript": {
            "project": "tsconfig.eslint.json"
          }
        }
      }
    }

Once these changes are in place, running ESLint should no longer emit import/no-unresolved for @webui/common, nor should TS type errors cascade from an untyped import.

🧰 Tools
🪛 GitHub Actions: clp-lint

[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)


[error] 1-1: ESLint: Unable to resolve path to module '@webui/common' (import/no-unresolved)

🤖 Prompt for AI Agents
In components/webui/server/src/routes/api/search/utils.ts around lines 1 to 1,
the review flags that @webui/common is not resolved because the server package
uses "*" instead of a workspace dependency and TypeScript/ESLint lack path
mappings; update components/webui/server/package.json to change the
@webui/common dependency specifier from "*" to "workspace:*", add a tsconfig
(either components/webui/server/tsconfig.json or
components/webui/server/tsconfig.eslint.json) that extends the server tsconfig
and includes compilerOptions.baseUrl="./" with a paths mapping "@webui/common":
["../common/src"], and create/update an ESLint config (e.g., .eslintrc.json at
repo root) to configure import/resolver typescript pointed at the
tsconfig.eslint.json so ESLint and TypeScript can resolve the workspace import.

Comment on lines +159 to 161
cd "{{.OUTPUT_DIR}}/var/www/webui"
PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm ci --omit=dev
# This command must be last
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

npm ci in packaged webui looks correct; consider hardening for CI.

Running npm ci with Node 22 at the packaged root is consistent with the workspace flow. To make CI less flaky, you can disable audit/fund and enforce production-only in env.

-        PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm ci --omit=dev
+        NPM_CONFIG_AUDIT=false NPM_CONFIG_FUND=false \
+        PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm ci --omit=dev
📝 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.

Suggested change
cd "{{.OUTPUT_DIR}}/var/www/webui"
PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm ci --omit=dev
# This command must be last
cd "{{.OUTPUT_DIR}}/var/www/webui"
NPM_CONFIG_AUDIT=false NPM_CONFIG_FUND=false \
PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm ci --omit=dev
# This command must be last
🤖 Prompt for AI Agents
In taskfile.yaml around lines 159 to 161, the npm ci invocation should be
hardened for CI: run it with production-only and disable audit/fund to avoid
flakiness. Update the environment for that command to include
NPM_CONFIG_PRODUCTION=true (or npm_config_production=true) and disable
audits/funding with npm_config_audit=false and npm_config_fund=false while
preserving the PATH and the current working directory; keep the command as the
last step.

@davemarco
Copy link
Contributor Author

@junhaoliao - there was an issue with the linter script due to workspaces, i believe i just fixed

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
components/webui/package.json (2)

5-9: Workspaces list looks fine; note on directory convention (already discussed).

The explicit list is OK. Prior feedback about moving under packages/ remains a style preference and can be deferred as agreed.


11-11: Prefer deterministic installs with npm ci.

Replace clean-install with ci for lockfile-only reproducible installs across workspaces.

Apply this diff:

-    "init": "npm clean-install --workspaces",
+    "init": "npm ci --workspaces",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0ca50ed and 9741b66.

📒 Files selected for processing (1)
  • components/webui/package.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T18:14:01.607Z
Learnt from: davemarco
PR: y-scope/clp#1119
File: taskfile.yaml:302-305
Timestamp: 2025-08-19T18:14:01.607Z
Learning: npm ci in workspaces can work without physical package.json files in all workspace directories if the workspace information is properly defined in package-lock.json. Built static assets (like from Vite builds) don't require runtime package.json files since they're self-contained. npm ci uses the lockfile for dependency resolution, not individual workspace manifests.

Applied to files:

  • components/webui/package.json
🔇 Additional comments (1)
components/webui/package.json (1)

14-14: Update root “start” script to invoke workspace start commands

The top-level “client:start” and “server:start” composites were removed—each package now exposes its own start script. We’ve verified that:

  • @webui/client defines start: “vite”
  • @webui/server defines start: “npm run build && fastify start -l info dist/src/app.js”

Adjust the root components/webui/package.json accordingly:

--- a/components/webui/package.json
+++ b/components/webui/package.json
@@ lines 14
-    "start": "concurrently \"npm run client:start\" \"npm run server:start\""
+    "start": "concurrently \"npm --workspace @webui/client run start\" \"npm --workspace @webui/server run start\""

This ensures npm start correctly spins up both client and server via their workspace scripts.

Comment on lines 2 to 3
"name": "webui",
"version": "0.1.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Optional: align root package name with the scope used elsewhere.

Consider renaming to @webui/webui for consistency with @webui/client and @webui/server. Non-blocking.

🤖 Prompt for AI Agents
In components/webui/package.json around lines 2 to 3, the package name "webui"
is inconsistent with the scoped names used elsewhere; change the "name" field to
"@webui/webui" and update any local references (workspaces, import paths,
README, CI/publish config) to the new scoped name so tooling and dependency
resolution remain correct; ensure package.json's other fields
(private/publishConfig) are adjusted if necessary for publishing under the
scope.

Comment on lines 12 to 13
"lint:check": "npm --workspace common run build && npm run lint:check --workspaces",
"lint:fix": "npm --workspace common run build && npm run lint:fix --workspaces",
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Harden lint scripts and address coupling to folder name.

  • Use workspace selector by package name (stable) instead of folder path "common".
  • Add --if-present so packages without the script don’t error.

Apply this diff:

-    "lint:check": "npm --workspace common run build && npm run lint:check --workspaces",
-    "lint:fix": "npm --workspace common run build && npm run lint:fix --workspaces",
+    "lint:check": "npm --workspace @webui/common run build && npm run --workspaces --if-present lint:check",
+    "lint:fix": "npm --workspace @webui/common run build && npm run --workspaces --if-present lint:fix",
📝 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.

Suggested change
"lint:check": "npm --workspace common run build && npm run lint:check --workspaces",
"lint:fix": "npm --workspace common run build && npm run lint:fix --workspaces",
"lint:check": "npm --workspace @webui/common run build && npm run --workspaces --if-present lint:check",
"lint:fix": "npm --workspace @webui/common run build && npm run --workspaces --if-present lint:fix",
🤖 Prompt for AI Agents
In components/webui/package.json around lines 12 to 13, the lint scripts are
hard-coded to the workspace folder name "common" and don't use --if-present;
change them to target the workspace by package name and add --if-present so
missing scripts won't fail. Replace "npm --workspace common run build" with "npm
--workspace=\"<package-name>\" --if-present run build" and append "--if-present"
to the downstream npm run calls (e.g. "npm run lint:check --workspaces
--if-present" and "npm run lint:fix --workspaces --if-present"); set
<package-name> to the actual package name for the common package as declared in
the root package.json.

Copy link
Member

@junhaoliao junhaoliao left a comment

Choose a reason for hiding this comment

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

for the PR title, how about

feat(webui): Migrate to npm workspace; Update taskfile and the package scripts accordingly.

"init": "npm clean-install --workspaces",
"lint:check": "npm --workspace common run build && npm run lint:check --workspaces",
"lint:fix": "npm --workspace common run build && npm run lint:fix --workspaces",
"start": "concurrently \"npm run client:start\" \"npm run server:start\""
Copy link
Member

Choose a reason for hiding this comment

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

let's fix this broken script

i believe a more elegant way is possible once we unify the dev script names, for now this

Suggested change
"start": "concurrently \"npm run client:start\" \"npm run server:start\""
"start": "concurrently \"npm run start:*\"",
"start:client": "npm --workspace client run start",
"start:common": "npm --workspace common run build:watch",
"start:server": "npm --workspace server run dev"

also we should update the concurrently version to match the server's version (see below). otherwise, concurrently won't be installed due to version conflict.

    "concurrently": "^9.1.2"

then

rm -rf node_modules package-lock.json
npm i --workspaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i used dev instead since i think better than start. I dont think i need to remove, i think i can just write npm i --save-dev concurrently@9.12

@davemarco davemarco changed the title feat(webui): Combine server, client, and common into npm workspace; restructure taskfile to use new workspace. feat(webui): Migrate to npm workspace; Update taskfile and the package scripts accordingly. Aug 28, 2025
davemarco and others added 3 commits August 28, 2025 11:51
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (5)
components/webui/client/tsconfig/tsconfig.app.json (1)

4-4: Include glob looks correct; resolves prior ask.

Using "../src/**/*" is the right scope for the app TS inputs and aligns with removing the old @common alias.

components/webui/package.json (2)

14-14: Start script is broken; invoke package starts via workspace selectors.

Root no longer has client:start/server:start. Call each package’s start directly.

-    "start": "concurrently \"npm run client:start\" \"npm run server:start\""
+    "start": "concurrently \"npm --workspace @webui/client run start\" \"npm --workspace @webui/server run start\""

5-9: Optional: pin concurrently to match other packages and avoid version conflicts.

If server/common use concurrently 9.x, align here to prevent dedupe churn. Non-blocking.

-    "concurrently": "^8.2.2"
+    "concurrently": "^9.1.2"
components/webui/client/package.json (2)

2-2: Mark app package as private to prevent accidental publish.

 {
-  "name": "@webui/client",
+  "name": "@webui/client",
+  "private": true,

24-24: Replace wildcard on @webui/common with a pinned workspace version.

"*" harms reproducibility and can pull a registry artefact if it ever exists. Pin to common’s version.

-    "@webui/common": "*",
+    "@webui/common": "^0.1.0",

Adjust the version to match components/webui/common/package.json if different.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9741b66 and 44451b7.

📒 Files selected for processing (3)
  • components/webui/client/package.json (3 hunks)
  • components/webui/client/tsconfig/tsconfig.app.json (1 hunks)
  • components/webui/package.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T18:14:01.607Z
Learnt from: davemarco
PR: y-scope/clp#1119
File: taskfile.yaml:302-305
Timestamp: 2025-08-19T18:14:01.607Z
Learning: npm ci in workspaces can work without physical package.json files in all workspace directories if the workspace information is properly defined in package-lock.json. Built static assets (like from Vite builds) don't require runtime package.json files since they're self-contained. npm ci uses the lockfile for dependency resolution, not individual workspace manifests.

Applied to files:

  • components/webui/package.json
🔇 Additional comments (2)
components/webui/client/package.json (2)

21-21: Good: synced @sinclair/typebox with common.


46-47: Tooling versions look stable and aligned.

Comment on lines +11 to +13
"init": "npm clean-install --workspaces && npm --workspace common run build",
"lint:check": "npm run lint:check --workspaces",
"lint:fix": "npm run lint:fix --workspaces",
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Prefer lockfile-only installs and target the workspace by package name.

Use npm ci for reproducible installs and refer to @webui/common by package name (not folder).

-    "init": "npm clean-install --workspaces && npm --workspace common run build",
+    "init": "npm ci --workspaces && npm --workspace @webui/common run build",
📝 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.

Suggested change
"init": "npm clean-install --workspaces && npm --workspace common run build",
"lint:check": "npm run lint:check --workspaces",
"lint:fix": "npm run lint:fix --workspaces",
// components/webui/package.json
{
// …
"scripts": {
"init": "npm ci --workspaces && npm --workspace @webui/common run build",
"lint:check": "npm run lint:check --workspaces",
"lint:fix": "npm run lint:fix --workspaces",
// …
}
// …
}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
components/webui/package.json (3)

5-9: Workspace layout: prior suggestion still stands (packages/ or globs).

Using folder names is fine, but adopting a packages/ directory and/or a glob (e.g., "packages/*") is the common convention and scales better. Non-blocking.


11-11: Use lockfile-only installs and target the workspace by package name.

Deterministic installs via npm ci; address coupling to folder name.

-    "init": "npm clean-install --workspaces && npm --workspace common run build",
+    "init": "npm ci --workspaces && npm --workspace @webui/common run build",

Note: Based on the retrieved learnings for this repo, npm ci works well with workspaces and lockfile-driven resolution.


14-14: Fix start script to actually target workspace packages.

client:start/server:start aren’t root scripts; current command will fail. Use npm’s workspace selector.

-    "start": "concurrently \"npm run client:start\" \"npm run server:start\""
+    "start": "concurrently \"npm -w @webui/client run start\" \"npm -w @webui/server run start\""
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 44451b7 and 4026fe9.

⛔ Files ignored due to path filters (1)
  • components/webui/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • components/webui/package.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T18:14:01.607Z
Learnt from: davemarco
PR: y-scope/clp#1119
File: taskfile.yaml:302-305
Timestamp: 2025-08-19T18:14:01.607Z
Learning: npm ci in workspaces can work without physical package.json files in all workspace directories if the workspace information is properly defined in package-lock.json. Built static assets (like from Vite builds) don't require runtime package.json files since they're self-contained. npm ci uses the lockfile for dependency resolution, not individual workspace manifests.

Applied to files:

  • components/webui/package.json
🔇 Additional comments (1)
components/webui/package.json (1)

19-19: concurrently version bump aligns with server; LGTM.

Comment on lines +12 to +13
"lint:check": "npm run lint:check --workspaces",
"lint:fix": "npm run lint:fix --workspaces",
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Harden lint scripts; add --if-present and correct flag placement.

Prevents failures when some packages lack lint scripts and uses the supported flag order.

-    "lint:check": "npm run lint:check --workspaces",
-    "lint:fix": "npm run lint:fix --workspaces",
+    "lint:check": "npm --workspaces run --if-present lint:check",
+    "lint:fix": "npm --workspaces run --if-present lint:fix",

If type-aware linting needs built types, consider prepending: npm -w @webui/common run build.

📝 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.

Suggested change
"lint:check": "npm run lint:check --workspaces",
"lint:fix": "npm run lint:fix --workspaces",
"lint:check": "npm --workspaces run --if-present lint:check",
"lint:fix": "npm --workspaces run --if-present lint:fix",
🤖 Prompt for AI Agents
In components/webui/package.json around lines 12-13, the lint scripts should
include the --if-present option and place the flags before the run command;
change them to run with flags first (e.g., npm --workspaces --if-present run
lint:check and npm --workspaces --if-present run lint:fix) so npm won't fail if
some workspaces lack the scripts and the flag order is valid; if type-aware
linting requires built types, optionally prepend an npm -w @webui/common run
build step.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (1)
components/webui/server/package.json (1)

19-19: Optionally declare Node engine.
Helps CI and devs align with Node 22.

   "type": "module",
+  "engines": {
+    "node": ">=22 <23"
+  },
♻️ Duplicate comments (4)
components/webui/server/package.json (2)

8-12: Fix compiled path: TypeScript emits to dist/, not dist/src.
Update script targets or builds will fail to start.

Apply:

-    "dev:start": "fastify start --ignore-watch=.ts$ -w -l info -P dist/src/app.js",
+    "dev:start": "fastify start --ignore-watch=.ts$ -w -l info -P dist/app.js",
-    "start": "npm run build && fastify start -l info dist/src/app.js",
+    "start": "npm run build && fastify start -l info dist/app.js",
-    "standalone": "npm run build && node --env-file=.env dist/src/main.js",
+    "standalone": "npm run build && node --env-file=.env dist/main.js",
#!/bin/bash
# Inspect TS config to confirm emit layout
sed -n '1,200p' components/webui/server/tsconfig.json | sed -n '1,120p'
rg -nP '"(outDir|rootDir)"' components/webui/server/tsconfig.json || true
# Check entry sources exist
rg -n 'src/(app|main)\.ts' components/webui/server -S -C1 || true

34-34: Move @types/node to devDependencies.
Avoids bundling types at runtime (see #1116).

   "dependencies": {
-    "@types/node": "^22.13.9",
   },
   "devDependencies": {
+    "@types/node": "^22.13.9",
   }
components/webui/package.json (2)

11-11: Use lockfile-only installs and target workspace by package name.
More reproducible; decouples from folder names.

-    "init": "npm clean-install --workspaces && npm --workspace common run build",
+    "init": "npm ci --workspaces && npm -w @webui/common run build",

12-13: Harden lint scripts; add --if-present and correct flag order.
Prevents failures if a package lacks lint scripts.

-    "lint:check": "npm run lint:check --workspaces",
-    "lint:fix": "npm run lint:fix --workspaces",
+    "lint:check": "npm --workspaces run --if-present lint:check",
+    "lint:fix": "npm --workspaces run --if-present lint:fix",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between bb868d9 and 5873146.

⛔ Files ignored due to path filters (1)
  • components/webui/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • components/webui/package.json (1 hunks)
  • components/webui/server/package.json (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T18:14:01.607Z
Learnt from: davemarco
PR: y-scope/clp#1119
File: taskfile.yaml:302-305
Timestamp: 2025-08-19T18:14:01.607Z
Learning: npm ci in workspaces can work without physical package.json files in all workspace directories if the workspace information is properly defined in package-lock.json. Built static assets (like from Vite builds) don't require runtime package.json files since they're self-contained. npm ci uses the lockfile for dependency resolution, not individual workspace manifests.

Applied to files:

  • components/webui/package.json
  • components/webui/server/package.json
🔇 Additional comments (6)
components/webui/server/package.json (4)

2-2: Package rename—LGTM.
Scoped name matches the workspace.


33-33: TypeBox version bump—LGTM.
Matches prior suggestion.


35-35: Workspace dep on @webui/common—LGTM.
Using "*" with npm workspaces resolves to the local package.


47-51: Tooling bumps—LGTM.
Pinned eslint config, updated concurrently, tap, and typescript are reasonable.

components/webui/package.json (2)

5-9: Workspaces declaration—LGTM.
Simple, clear layout for common/client/server.


22-22: concurrently ^9.2.1—LGTM.
Matches server’s version; resolves the install conflict noted earlier.

Comment on lines 2 to 4
"name": "webui",
"version": "0.1.0",
"description": "",
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Mark root as private and (optionally) add Node engine.
Prevents accidental publish; documents required Node.

   "name": "webui",
   "version": "0.1.0",
   "description": "",
+  "private": true,
+  "engines": { "node": ">=22 <23" },
📝 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.

Suggested change
"name": "webui",
"version": "0.1.0",
"description": "",
"name": "webui",
"version": "0.1.0",
"description": "",
"private": true,
"engines": { "node": ">=22 <23" },
// … the rest of your existing fields …
🤖 Prompt for AI Agents
In components/webui/package.json around lines 2 to 4, the package.json is
missing the "private": true flag (and optionally an engines field) which allows
accidental publish and doesn't document Node requirements; add "private": true
at the top-level (e.g., immediately after "version") to prevent publishing, and
optionally add an "engines" entry such as "engines": { "node": ">=16" } (or your
supported range) to document the required Node version.

Comment on lines 14 to 17
"dev": "concurrently --raw \"npm run dev:*\"",
"dev:client": "npm --workspace client run start",
"dev:common": "npm --workspace common run build:watch",
"dev:server": "npm --workspace server run dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Select workspaces by package name, not folder.
More robust to renames and matches scoped package IDs.

-    "dev": "concurrently --raw \"npm run dev:*\"",
-    "dev:client": "npm --workspace client run start",
-    "dev:common": "npm --workspace common run build:watch",
-    "dev:server": "npm --workspace server run dev"
+    "dev": "concurrently --raw \"npm run dev:*\"",
+    "dev:client": "npm -w @webui/client run start",
+    "dev:common": "npm -w @webui/common run build:watch",
+    "dev:server": "npm -w @webui/server run dev"

🏁 Script executed:

#!/bin/bash
# Verify workspace package names exist
rg -nP '^\s*"name":\s*"@webui/(client|common|server)"' -S components/webui -C1

Length of output: 1169


Use workspace package names for run scripts
Verified that @webui/client, @webui/common and @webui/server exist; you may optionally switch to npm -w @webui/<pkg> (e.g. npm -w @webui/client run start) for greater resilience to folder renames.

🤖 Prompt for AI Agents
In components/webui/package.json around lines 14 to 17, the dev scripts use
workspace folder names (npm --workspace client/common/server ...) which can
break if folders are renamed; update the scripts to use workspace package names
with npm -w @webui/client run start, npm -w @webui/common run build:watch and
npm -w @webui/server run dev respectively so the commands target the package by
its workspace name rather than the folder path.

"name": "@webui/server",
"version": "0.1.0",
"description": "",
"main": "src/main.ts",
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Nit: point “main” to built output (or drop it).
As an app, either set dist/main.js or omit “main”.

-  "main": "src/main.ts",
+  "main": "dist/main.js",
📝 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.

Suggested change
"main": "src/main.ts",
"main": "dist/main.js",
🤖 Prompt for AI Agents
In components/webui/server/package.json around line 5, the "main" field points
to the TypeScript source ("src/main.ts") which is incorrect for a
published/built app; update the field to the built output (e.g., "dist/main.js")
or remove the "main" entry entirely if you don't need it, and ensure your build
script outputs to the referenced path (or adjust the build output path
accordingly) so the package.json points to the actual runtime artifact.

@davemarco davemarco requested a review from junhaoliao August 28, 2025 17:05
junhaoliao
junhaoliao previously approved these changes Aug 28, 2025
"init": "npm clean-install --workspaces && npm --workspace common run build",
"lint:check": "npm run lint:check --workspaces",
"lint:fix": "npm run lint:fix --workspaces",
"dev": "concurrently --raw \"npm run dev:*\"",
Copy link
Member

Choose a reason for hiding this comment

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

qq: any reason we want "raw"? were the logs a bit clustered without --raw?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was playing around with console printing. this one i can remove. I also added --preserveWatchOutput fyi, since it was annoying that tsc would keep clearing terminal

Copy link
Member

Choose a reason for hiding this comment

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

nice. i've been finding that annoying as well

@davemarco davemarco requested a review from junhaoliao August 28, 2025 20:18
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

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 (2)
components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (2)

14-18: Simplify the guard with optional chaining; drop brittle typeof checks.

Current checks are redundant and rely on string "undefined". Prefer a single nullish check.

Apply:

-    if (0 === data.length ||
-        "undefined" === typeof data[0] ||
-        "undefined" === typeof data[0].row
-    ) {
-        return [];
-    }
+    const firstRow = data[0]?.row;
+    if (undefined === firstRow) {
+        return [];
+    }

21-30: Consider stable column ordering.

If server/object key insertion order isn’t guaranteed, sort keys for deterministic column layout. Skip if order is intentionally meaningful.

-    return Object.keys(data[0].row)
-        .map((key) => ({
+    const keys = Object.keys(firstRow).sort();
+    return keys.map((key) => ({
             dataIndex: [
                 "row",
                 key,
             ],
             key: key,
             title: key,
             width: 100,
         }));
♻️ Duplicate comments (5)
components/webui/client/package.json (3)

2-3: Mark the app package as private to prevent accidental publish.

 {
   "name": "@webui/client",
+  "private": true,
   "version": "0.1.0",

24-24: Replace "*" with a pinned workspace version for @webui/common.
Using "*" hurts reproducibility and could pull a registry copy if one appears. Pin to the version declared in components/webui/common/package.json.

-    "@webui/common": "*",
+    "@webui/common": "^0.1.0",

49-49: Vite ^6.3.5 — confirm against prior “revert” request; consider pinning exact.
If we keep 6.3.5, prefer exact pin to reduce churn and ensure plugin parity.

-    "vite": "^6.3.5"
+    "vite": "6.3.5"
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (2)

845-847: Path fix to server/dist/settings.json is correct; ensure the build copies settings.json into dist

The runtime expects server/dist/settings.json, but tsc won’t emit JSON. Add an assets copy step to the server build to prevent a 404 at startup.

Apply in components/webui/server/package.json:

 {
   "scripts": {
-    "build": "tsc"
+    "build": "tsc && mkdir -p dist && cp src/settings.json dist/settings.json"
   }
 }

Quick check after building:

#!/bin/bash
# Verify server settings.json is emitted to dist
test -f components/webui/server/dist/settings.json && echo "OK: settings.json present" || (echo "Missing: components/webui/server/dist/settings.json"; exit 1)

965-968: Entrypoint updated to dist/src/main.js; verify build emits it and fail fast if missing

Many tsconfig setups output dist/main.js (rootDir: "src"). Either adjust tsconfig or the path, and add a host-side existence check for clearer errors.

Option A — keep dist/src/main.js: set rootDir to "." in components/webui/server/tsconfig.json.

 {
   "compilerOptions": {
-    "rootDir": "src",
+    "rootDir": ".",
     "outDir": "dist"
   }
 }

Option B — if you prefer dist/main.js, change the Python path back accordingly.

Also add a preflight check in this script:

     node_cmd = [
         str(CONTAINER_CLP_HOME / "bin" / "node-22"),
-        str(container_webui_dir / "server" / "dist" / "src" / "main.js"),
+        str(container_webui_dir / "server" / "dist" / "src" / "main.js"),
     ]
+    # Fail fast if entrypoint is missing on host
+    main_js_host_path = (
+        get_clp_home() / "var" / "www" / "webui" / "server" / "dist" / "src" / "main.js"
+    )
+    if not main_js_host_path.exists():
+        raise FileNotFoundError(
+            f"WebUI server entrypoint not found at {main_js_host_path}. "
+            "Build the server package before starting (e.g., `task webui:build` or `npm -w components/webui/server run build`)."
+        )

Quick check after building:

#!/bin/bash
# Verify server entrypoint exists
test -f components/webui/server/dist/src/main.js && echo "OK: main.js present" || (echo "Missing: components/webui/server/dist/src/main.js"; exit 1)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3fee734 and 3039ab1.

⛔ Files ignored due to path filters (1)
  • components/webui/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • components/clp-package-utils/clp_package_utils/scripts/start_clp.py (2 hunks)
  • components/webui/client/package.json (3 hunks)
  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/index.tsx (1 hunks)
  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}

⚙️ CodeRabbit configuration file

  • Prefer false == <expression> rather than !<expression>.

Files:

  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts
  • components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/index.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/clp#1119
File: taskfile.yaml:429-433
Timestamp: 2025-08-14T18:23:39.476Z
Learning: In the webui workspace, the exclude patterns ".tmp" and ".vite-temp" are Vite-specific temporary directories that only apply to the client package. The common and server packages use TypeScript compiler (tsc) directly and don't generate these Vite temporary files, so these exclude patterns should not be applied to their node_modules checksum computations.
🧬 Code graph analysis (1)
components/clp-package-utils/clp_package_utils/scripts/start_clp.py (1)
components/clp-package-utils/clp_package_utils/general.py (1)
  • get_clp_home (109-125)
🔇 Additional comments (5)
components/webui/client/package.json (2)

21-21: Aligned TypeBox to ^0.34.38 — good.


47-47: Pinning eslint-config-yscope is correct — avoids CI drift.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/index.tsx (2)

50-51: Verify rowKey compatibility with the centralised type.

Ensure PrestoSearchResult in @webui/common guarantees a unique string/number _id; otherwise VirtualTable row identity may break.


6-6: Type import migration to @webui/common verified
PrestoSearchResult is exported in @webui/common with both _id and row as expected; no further action required.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (1)

1-1: Type import switch to @webui/common is correct.

Matches the workspace type centralisation; no behavioural change.

"eslint-config-yscope": "latest",
"typescript": "~5.6.2",
"eslint-config-yscope": "1.1.1",
"typescript": "~5.7.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Confirm TS version alignment across workspace.
Ensure all packages use the same operator (~5.7.3) to avoid duplicate installs.


🏁 Script executed:

#!/bin/bash
rg -n --hidden --glob '!node_modules' -e '"typescript"\s*:\s*"[^"]+"' -g '*package.json' -C1

Length of output: 667


Align TypeScript specifier across all packages
In components/webui/common/package.json and components/webui/server/package.json, change

"typescript": "^5.7.3"

to

"typescript": "~5.7.3"

so all workspaces use the same operator and avoid duplicate installs.

🤖 Prompt for AI Agents
In components/webui/client/package.json around line 48, update the TypeScript
dependency specifier to match the other webui packages: change occurrences using
the caret operator to use the tilde operator so all workspaces declare
"typescript" with "~5.7.3" to ensure consistent versions and avoid duplicate
installs; modify the specifier in components/webui/common/package.json and
components/webui/server/package.json accordingly.

@davemarco davemarco merged commit bcf44ee into y-scope:main Sep 3, 2025
23 checks passed
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