-
Notifications
You must be signed in to change notification settings - Fork 82
feat(webui): Migrate to npm workspace; Update taskfile and the package scripts accordingly. #1119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a new workspace package Changes
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings
📚 Learning: 2025-08-19T18:14:01.607Z
Applied to files:
📚 Learning: 2025-08-14T18:23:39.476Z
Applied to files:
⏰ 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)
🔇 Additional comments (10)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
"common", | ||
"client", | ||
"server" |
There was a problem hiding this comment.
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.
"common", | |
"client", | |
"server" | |
"packages/common", | |
"packages/client", | |
"packages/server" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these were not neccesary
There was a problem hiding this 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 broadobject
type in public event payloadsUsing
object
weakens type safety for payloads (hard to safely access). PreferRecord<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.debugAvoid 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)}") PYcomponents/webui/common/tsconfig.json (1)
1-34
: Enable project references; set rootDir for cleaner incremental buildsFor 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 suppressionEmpty 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 schemaUse 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 wellSame 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.
⛔ 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 goodTarget 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/commonGood 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” checkThe 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 dependencyAll 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
definesinterface PrestoSearchResult
and re-exports it viaexport type { PrestoSearchResult }
.- components/webui/client/package.json declares a dependency on
@webui/common
("*"
).The
import type { PrestoSearchResult } from "@webui/common";
will resolve correctly.
get_clp_home() / "var" / "www" / "webui" / "server" / "dist" / "settings.json" | ||
) |
There was a problem hiding this comment.
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 copysettings.json
intodist/
.
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 -->
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"), | ||
] |
There was a problem hiding this comment.
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 thatmain.js
is emitted underdist/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", |
There was a problem hiding this comment.
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": "*", |
There was a problem hiding this comment.
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.
"@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"; |
There was a problem hiding this comment.
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 bothparserOptions.project
andsettings.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 necessarycompilerOptions.baseUrl
orpaths
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"; |
There was a problem hiding this comment.
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.
} from "@webui/common"; | ||
import {StatusCodes} from "http-status-codes"; | ||
|
There was a problem hiding this comment.
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
(orpnpm-workspace.yaml
) listing"workspaces": ["components/webui/common", "components/webui/server"]
- If you use Yarn or pnpm workspaces, add a root
- In components/webui/server/tsconfig.json, add a
paths
section undercompilerOptions
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 acreateTsConfigOverride
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 ),
- Create a specialized tsconfig (e.g.
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"; |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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 anytsconfig*.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 incomponents/webui/server/tsconfig.json
or in a newtsconfig.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 attsconfig.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.
cd "{{.OUTPUT_DIR}}/var/www/webui" | ||
PATH="{{.G_NODEJS_22_BIN_DIR}}":$PATH npm ci --omit=dev | ||
# This command must be last |
There was a problem hiding this comment.
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.
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.
@junhaoliao - there was an issue with the linter script due to workspaces, i believe i just fixed |
There was a problem hiding this 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.
📒 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 commandsThe 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.
"name": "webui", | ||
"version": "0.1.0", |
There was a problem hiding this comment.
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.
components/webui/package.json
Outdated
"lint:check": "npm --workspace common run build && npm run lint:check --workspaces", | ||
"lint:fix": "npm --workspace common run build && npm run lint:fix --workspaces", |
There was a problem hiding this comment.
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.
"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.
There was a problem hiding this 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.
components/webui/package.json
Outdated
"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\"" |
There was a problem hiding this comment.
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
"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
There was a problem hiding this comment.
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
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
Co-authored-by: Junhao Liao <junhao@junhao.ca>
There was a problem hiding this 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.
📒 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.
"init": "npm clean-install --workspaces && npm --workspace common run build", | ||
"lint:check": "npm run lint:check --workspaces", | ||
"lint:fix": "npm run lint:fix --workspaces", |
There was a problem hiding this comment.
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.
"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", | |
// … | |
} | |
// … | |
} |
There was a problem hiding this 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.
⛔ 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.
"lint:check": "npm run lint:check --workspaces", | ||
"lint:fix": "npm run lint:fix --workspaces", |
There was a problem hiding this comment.
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.
"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.
There was a problem hiding this 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.
⛔ 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.
"name": "webui", | ||
"version": "0.1.0", | ||
"description": "", |
There was a problem hiding this comment.
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.
"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.
components/webui/package.json
Outdated
"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" |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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.
"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.
components/webui/package.json
Outdated
"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:*\"", |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this 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 distThe 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 missingMany 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.
⛔ 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
androw
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", |
There was a problem hiding this comment.
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.
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
breaking change.
Validation performed
Package starts, and webui can compress and search logs.
Summary by CodeRabbit
New Features
Bug Fixes
Chores