-
Notifications
You must be signed in to change notification settings - Fork 0
chore: cleanup lint errors and typecheck errors #95
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
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis set of changes removes unused configuration files, updates and simplifies TypeScript and test configurations, refines type-checking scripts and workflows, and improves type safety and clarity in both frontend and backend code. Several test files and cases are deleted or revised for type correctness, and minor refactors are made for error handling and prop defaults. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CI
participant Bun
participant TypeScript
Developer->>CI: Push or PR triggers workflow
CI->>Bun: bun install --frozen-lockfile
CI->>TypeScript: bun typecheck (runs tsc/vue-tsc)
TypeScript-->>CI: Reports typecheck results
CI-->>Developer: Pass/fail status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (11)📓 Common learnings📚 Learning: applies to src/**/*.vue : use reactive objects for form state and errors...Applied to files:
📚 Learning: applies to src/**/*.vue : use storetorefs for state in components...Applied to files:
📚 Learning: applies to src/**/*.vue : validate on client and handle server errors for forms...Applied to files:
📚 Learning: applies to src/**/*.{vue,ts} : use useapi composable (elysia eden) for all api calls...Applied to files:
📚 Learning: applies to src/**/*.{vue,ts} : use pinia stores for global state...Applied to files:
📚 Learning: applies to src/**/*.vue : use v-memo, shallowref, markraw, and suspense for performance optimization...Applied to files:
📚 Learning: applies to src/**/*.{vue,ts} : handle errors and loading states reactively when making api calls...Applied to files:
📚 Learning: applies to src/composables/**/*.ts : use composables for logic that is not global state...Applied to files:
📚 Learning: applies to src/composables/**/*.ts : do not export store state from composables...Applied to files:
📚 Learning: applies to src/composables/**/*.ts : do not proxy or export pinia store state/actions from composabl...Applied to files:
🔇 Additional comments (2)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
frontend/src/components/WikibaseSchemaEditor.vue (1)
257-263: Fix missing 'id' property in statement updateThe pipeline failure indicates that the statement object being assigned is missing the required
idproperty. The type system expects statements to have anidfield.Apply this fix to include the existing statement ID:
schemaStore.statements[statementIndex] = { - ...schemaStore.statements[statementIndex], + id: schemaStore.statements[statementIndex].id, property: currentStatement.property, value: currentStatement.value, rank: currentStatement.rank, qualifiers: currentStatement.qualifiers || [], }Or alternatively, preserve all existing fields and only update the necessary ones:
schemaStore.statements[statementIndex] = { + ...schemaStore.statements[statementIndex], - ...schemaStore.statements[statementIndex], property: currentStatement.property, value: currentStatement.value, rank: currentStatement.rank, qualifiers: currentStatement.qualifiers || [], }frontend/src/composables/__tests__/useValueMapping.test.ts (1)
199-211: Removed non-null assertion re-introduces a type error undernoUncheckedIndexedAccess
testColumns[0]isColumnInfo | undefinedwhennoUncheckedIndexedAccess=true.
Without the!,autoSuggestDataTypenow receives a possibleundefined, which fails type-check unless the function explicitly allows it.-const suggestedType = autoSuggestDataType(stringProperty, testColumns[0]) +const suggestedType = autoSuggestDataType(stringProperty, testColumns[0]!)Do the same for the
fallbackTypecall below.Alternatively, wrap the lookup in a guard:
const first = testColumns[0] expect(first).toBeDefined() const suggestedType = autoSuggestDataType(stringProperty, first)
🧹 Nitpick comments (11)
.github/workflows/typecheck.yml (1)
15-24: Consider adding cache + un-pinned Bun version for faster & safer CI runs
oven-sh/setup-bunsupports acache: trueflag that restores Bun’s global cache layer (~~/.bun/install/cache) between jobs – it cuts install time from ~20-25 s to <5 s.- Hard-pinning
bun-version: 1.2.19means breaking builds the day you drop the lockfile for 1.2.20 features. Preferbun-version: "latest"(or an env/variable) unless you have reproducibility requirements.- Optional: add
to auto-cancel outdated runs on force-pushes.concurrency: group: typecheck-${{ github.head_ref }} cancel-in-progress: trueThese tweaks keep the workflow lean without losing determinism.
frontend/src/composables/useSchemaApi.ts (4)
39-41:data && data.data– could be simplified & typed to drop theanycast
dataalready comes typed from the generated client. Propagate that generic:const { data, error } = await api .project({ projectId }) .schemas({ schemaId }) .get<WikibaseSchemaMapping>() // <= tell the client what to expectThen:
if (data?.data) loadSchemaIntoStore(data.data)No
as any, no redundant null-checks.
65-67: AligncreateSchemawith the cleaner patternYou kept the old
(data as any).datacast here. After adding proper generics (see previous comment) this can become:- loadSchemaIntoStore((data as any).data as WikibaseSchemaMapping) - return (data as any).data + loadSchemaIntoStore(data.data) + return data.data
88-91: Same cast repetition inupdateSchemaMirror the fix:
- loadSchemaIntoStore(data.data as WikibaseSchemaMapping) - schemaStore.markAsSaved() - return data.data + loadSchemaIntoStore(data.data) + schemaStore.markAsSaved() + return data.data
123-124: Return type can be declared for stronger guarantees
loadAllSchemascurrently returnsunknown[] | []. Add an explicit return type so callers are typed correctly:const loadAllSchemas = async (projectId: string): Promise<WikibaseSchemaMapping[]> => { … return data?.data ?? [] }frontend/tsconfig.json (2)
6-9: Exclude list misses Cypress & storybook configsIf you add end-to-end tests or Storybook later, they’ll creep into type-check. You can future-proof:
"exclude": [ "vite.config.*", "vitest.config.*", "cypress", ".storybook" ]
19-22:libalready declared intsconfig.dom.json
@vue/tsconfig/tsconfig.dom.jsonsets"lib": ["dom", "esnext"].
You can drop this override unless you need the explicit ordering.frontend/src/composables/__tests__/useDragDropHandlers.test.ts (1)
197-199: Consider improving type handling instead of suppressing errors.The
@ts-expect-errorcomments suggest TypeScript can't properly infer the types here. Consider using proper type assertions or null checks instead of suppressing type errors.For example, you could use:
-expect(droppedColumn).not.toBeNull() -// @ts-expect-error -expect(droppedColumn).toEqual(validColumn) +expect(droppedColumn).not.toBeNull() +expect(droppedColumn!).toEqual(validColumn)Or with proper type narrowing:
-expect(droppedColumn).not.toBeNull() -// @ts-expect-error -expect(droppedColumn).toEqual(validColumn) +expect(droppedColumn).not.toBeNull() +if (droppedColumn !== null) { + expect(droppedColumn).toEqual(validColumn) +}Also applies to: 255-257
frontend/src/composables/__tests__/useStatementConfig.test.ts (1)
133-134: Consider addressing the TypeScript error instead of suppressing it.The
@ts-expect-errorcomment suggests a known type issue. While this might be acceptable for cleanup purposes, consider if this can be resolved with proper typing.frontend/src/composables/__tests__/useDragDropContext.test.ts (1)
376-377: Consider resolving the TypeScript error.The
@ts-expect-errorcomment suggests a type incompatibility. While acceptable for cleanup, consider if this can be resolved with proper typing or if the test assertion logic needs adjustment.frontend/src/composables/__tests__/useColumnConversion.test.ts (1)
39-50: Consider using non-null assertions instead of optional chaining after length checks.While optional chaining improves type safety, it may be unnecessary here since you're asserting
result.toHaveLength(2)on line 36. After confirming the array has exactly 2 elements,result[0]andresult[1]are guaranteed to exist.Consider using non-null assertions for clearer intent:
- expect(result[0]?.name).toBe('name') - expect(result[0]?.dataType).toBe('VARCHAR') - expect(result[0]?.sampleValues).toEqual(['John Doe', 'Jane Smith', 'Bob Johnson']) - expect(result[0]?.nullable).toBe(false) - expect(result[0]?.uniqueCount).toBe(3) + expect(result[0]!.name).toBe('name') + expect(result[0]!.dataType).toBe('VARCHAR') + expect(result[0]!.sampleValues).toEqual(['John Doe', 'Jane Smith', 'Bob Johnson']) + expect(result[0]!.nullable).toBe(false) + expect(result[0]!.uniqueCount).toBe(3)This pattern would apply to similar cases throughout the file where array length is pre-validated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
.env.example(0 hunks).github/workflows/test.yml(1 hunks).github/workflows/typecheck.yml(1 hunks)backend/package.json(1 hunks)frontend/package.json(1 hunks)frontend/src/components/ColumnPalette.vue(2 hunks)frontend/src/components/ItemEditor.vue(0 hunks)frontend/src/components/PropertySelector.vue(1 hunks)frontend/src/components/QualifiersEditor.vue(1 hunks)frontend/src/components/StatementEditor.vue(0 hunks)frontend/src/components/WikibaseSchemaEditor.vue(4 hunks)frontend/src/components/__tests__/ItemEditor.integration.test.ts(0 hunks)frontend/src/components/__tests__/ItemEditor.test.ts(1 hunks)frontend/src/components/__tests__/StatementsEditor.test.ts(2 hunks)frontend/src/components/__tests__/WikibaseSchemaEditor.test.ts(0 hunks)frontend/src/composables/__tests__/useColumnConversion.test.ts(7 hunks)frontend/src/composables/__tests__/useDataTypeCompatibility.test.ts(0 hunks)frontend/src/composables/__tests__/useDragDropContext.test.ts(5 hunks)frontend/src/composables/__tests__/useDragDropHandlers.test.ts(8 hunks)frontend/src/composables/__tests__/useLanguageDropZone.test.ts(4 hunks)frontend/src/composables/__tests__/useSchemaApi.test.ts(2 hunks)frontend/src/composables/__tests__/useSchemaDropZone.test.ts(10 hunks)frontend/src/composables/__tests__/useStatementConfig.test.ts(3 hunks)frontend/src/composables/__tests__/useStatementDataTypeValidation.test.ts(8 hunks)frontend/src/composables/__tests__/useStatementDropZone.test.ts(8 hunks)frontend/src/composables/__tests__/useStatementEditor.test.ts(1 hunks)frontend/src/composables/__tests__/useValueMapping.test.ts(2 hunks)frontend/src/composables/useProjectCreationComposable.ts(2 hunks)frontend/src/composables/useSchemaApi.ts(4 hunks)frontend/src/composables/useStatementConfig.ts(2 hunks)frontend/src/composables/useStatementEditor.ts(1 hunks)frontend/src/composables/useValueMapping.ts(4 hunks)frontend/src/plugins/api.ts(1 hunks)frontend/src/stores/__tests__/drag-drop.store.test.ts(3 hunks)frontend/src/stores/__tests__/schema.store.test.ts(1 hunks)frontend/src/stores/drag-drop.store.ts(1 hunks)frontend/src/stores/project-list.store.ts(2 hunks)frontend/src/stores/project.store.ts(1 hunks)frontend/src/types/__tests__/drag-drop-context.test.ts(3 hunks)frontend/src/types/__tests__/drag-drop.test.ts(1 hunks)frontend/tsconfig.app.json(0 hunks)frontend/tsconfig.json(1 hunks)frontend/tsconfig.vite.json(0 hunks)tsconfig.base.json(2 hunks)
💤 Files with no reviewable changes (8)
- frontend/src/composables/tests/useDataTypeCompatibility.test.ts
- frontend/src/components/ItemEditor.vue
- .env.example
- frontend/tsconfig.vite.json
- frontend/src/components/tests/ItemEditor.integration.test.ts
- frontend/src/components/tests/WikibaseSchemaEditor.test.ts
- frontend/tsconfig.app.json
- frontend/src/components/StatementEditor.vue
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{ts,tsx,js,jsx}: Usebun <file>instead ofnode <file>orts-node <file>for running TypeScript or JavaScript files
Do not use dotenv; Bun automatically loads .env files
UseBun.serve()for HTTP servers and WebSockets instead ofexpress
Usebun:sqlitefor SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead ofws
PreferBun.fileovernode:fs's readFile/writeFile
UseBun.$(e.g.,Bun.$ls``) instead of execa for running shell commands
Files:
frontend/src/composables/__tests__/useDragDropContext.test.tsfrontend/src/composables/useSchemaApi.tsfrontend/src/composables/__tests__/useValueMapping.test.tsfrontend/src/composables/__tests__/useStatementDropZone.test.tsfrontend/src/stores/__tests__/schema.store.test.tsfrontend/src/composables/__tests__/useStatementDataTypeValidation.test.tsfrontend/src/stores/project.store.tsfrontend/src/composables/__tests__/useSchemaDropZone.test.tsfrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/src/types/__tests__/drag-drop.test.tsfrontend/src/stores/project-list.store.tsfrontend/src/composables/__tests__/useDragDropHandlers.test.tsfrontend/src/composables/__tests__/useStatementConfig.test.tsfrontend/src/composables/useStatementConfig.tsfrontend/src/types/__tests__/drag-drop-context.test.tsfrontend/src/plugins/api.tsfrontend/src/composables/__tests__/useStatementEditor.test.tsfrontend/src/stores/drag-drop.store.tsfrontend/src/composables/__tests__/useColumnConversion.test.tsfrontend/src/components/__tests__/StatementsEditor.test.tsfrontend/src/composables/__tests__/useLanguageDropZone.test.tsfrontend/src/composables/useValueMapping.tsfrontend/src/composables/useProjectCreationComposable.tsfrontend/src/components/__tests__/ItemEditor.test.tsfrontend/src/stores/__tests__/drag-drop.store.test.tsfrontend/src/composables/useStatementEditor.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun testinstead ofjestfor running tests
Files:
frontend/src/composables/__tests__/useDragDropContext.test.tsfrontend/src/composables/__tests__/useValueMapping.test.tsfrontend/src/composables/__tests__/useStatementDropZone.test.tsfrontend/src/stores/__tests__/schema.store.test.tsfrontend/src/composables/__tests__/useStatementDataTypeValidation.test.tsfrontend/src/composables/__tests__/useSchemaDropZone.test.tsfrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/src/types/__tests__/drag-drop.test.tsfrontend/src/composables/__tests__/useDragDropHandlers.test.tsfrontend/src/composables/__tests__/useStatementConfig.test.tsfrontend/src/types/__tests__/drag-drop-context.test.tsfrontend/src/composables/__tests__/useStatementEditor.test.tsfrontend/src/composables/__tests__/useColumnConversion.test.tsfrontend/src/components/__tests__/StatementsEditor.test.tsfrontend/src/composables/__tests__/useLanguageDropZone.test.tsfrontend/src/components/__tests__/ItemEditor.test.tsfrontend/src/stores/__tests__/drag-drop.store.test.ts
**/*.{html,ts,tsx,css}
📄 CodeRabbit Inference Engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuildfor building HTML, TypeScript, or CSS files
Files:
frontend/src/composables/__tests__/useDragDropContext.test.tsfrontend/src/composables/useSchemaApi.tsfrontend/src/composables/__tests__/useValueMapping.test.tsfrontend/src/composables/__tests__/useStatementDropZone.test.tsfrontend/src/stores/__tests__/schema.store.test.tsfrontend/src/composables/__tests__/useStatementDataTypeValidation.test.tsfrontend/src/stores/project.store.tsfrontend/src/composables/__tests__/useSchemaDropZone.test.tsfrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/src/types/__tests__/drag-drop.test.tsfrontend/src/stores/project-list.store.tsfrontend/src/composables/__tests__/useDragDropHandlers.test.tsfrontend/src/composables/__tests__/useStatementConfig.test.tsfrontend/src/composables/useStatementConfig.tsfrontend/src/types/__tests__/drag-drop-context.test.tsfrontend/src/plugins/api.tsfrontend/src/composables/__tests__/useStatementEditor.test.tsfrontend/src/stores/drag-drop.store.tsfrontend/src/composables/__tests__/useColumnConversion.test.tsfrontend/src/components/__tests__/StatementsEditor.test.tsfrontend/src/composables/__tests__/useLanguageDropZone.test.tsfrontend/src/composables/useValueMapping.tsfrontend/src/composables/useProjectCreationComposable.tsfrontend/src/components/__tests__/ItemEditor.test.tsfrontend/src/stores/__tests__/drag-drop.store.test.tsfrontend/src/composables/useStatementEditor.ts
🧠 Learnings (40)
📓 Common learnings
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{ts,vue} : Type safety everywhere
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `bun test` instead of `jest` for running tests
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Always use backend-inferred types for API data
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `bun <file>` instead of `node <file>` or `ts-node <file>` for running TypeScript or JavaScript files
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Props and emits must use explicit TypeScript interfaces
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{html,ts,tsx,css} : Use `bun build <file.html|file.ts|file.css>` instead of `webpack` or `esbuild` for building HTML, TypeScript, or CSS files
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use Vue 3 with Composition API and <script setup lang="ts"> in all Vue components
📚 Learning: applies to package.json : use `bun install` instead of `npm install`, `yarn install`, or `pnpm insta...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to package.json : Use `bun install` instead of `npm install`, `yarn install`, or `pnpm install` for dependency installation
Applied to files:
.github/workflows/test.ymlbackend/package.json
📚 Learning: applies to **/*.test.{ts,tsx,js,jsx} : use `bun test` instead of `jest` for running tests...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `bun test` instead of `jest` for running tests
Applied to files:
.github/workflows/test.ymlbackend/package.json.github/workflows/typecheck.ymlfrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/package.jsonfrontend/src/types/__tests__/drag-drop-context.test.tsfrontend/src/stores/__tests__/drag-drop.store.test.tstsconfig.base.json
📚 Learning: applies to package.json : use `bun run <script>` instead of `npm run <script>`, `yarn run <script>`,...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to package.json : Use `bun run <script>` instead of `npm run <script>`, `yarn run <script>`, or `pnpm run <script>` for running scripts
Applied to files:
.github/workflows/test.ymlbackend/package.json
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : use `bun ` instead of `node ` or `ts-node ` for ...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `bun <file>` instead of `node <file>` or `ts-node <file>` for running TypeScript or JavaScript files
Applied to files:
.github/workflows/test.ymlbackend/package.json.github/workflows/typecheck.ymlfrontend/package.jsonfrontend/tsconfig.jsonfrontend/src/types/__tests__/drag-drop-context.test.tstsconfig.base.json
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : use `bun.$` (e.g., `bun.$`ls``) instead of execa for running shell...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `Bun.$` (e.g., `Bun.$`ls``) instead of execa for running shell commands
Applied to files:
.github/workflows/test.ymlbackend/package.jsonfrontend/src/types/__tests__/drag-drop-context.test.tstsconfig.base.json
📚 Learning: applies to **/*.{html,ts,tsx,css} : use `bun build ` instead of `webpack...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{html,ts,tsx,css} : Use `bun build <file.html|file.ts|file.css>` instead of `webpack` or `esbuild` for building HTML, TypeScript, or CSS files
Applied to files:
backend/package.jsonfrontend/package.jsonfrontend/tsconfig.jsontsconfig.base.json
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : use `bun:sqlite` for sqlite instead of `better-sqlite3`...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `bun:sqlite` for SQLite instead of `better-sqlite3`
Applied to files:
backend/package.jsontsconfig.base.json
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : use `bun.serve()` for http servers and websockets instead of `expr...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `Bun.serve()` for HTTP servers and WebSockets instead of `express`
Applied to files:
backend/package.json
📚 Learning: applies to src/**/*.{vue,ts} : always use backend-inferred types for api data...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Always use backend-inferred types for API data
Applied to files:
backend/package.jsonfrontend/src/composables/useSchemaApi.tsfrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/package.jsonfrontend/tsconfig.jsonfrontend/src/stores/project-list.store.tsfrontend/src/plugins/api.tsfrontend/src/composables/useValueMapping.tsfrontend/src/composables/useProjectCreationComposable.ts
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : use `bun.redis` for redis instead of `ioredis`...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `Bun.redis` for Redis instead of `ioredis`
Applied to files:
backend/package.jsontsconfig.base.json
📚 Learning: for more information, read the official bun documentation at https://bun.sh/docs or check `node_modu...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: For more information, read the official Bun documentation at https://bun.sh/docs or check `node_modules/bun-types/docs/**.md` if available locally
Applied to files:
backend/package.jsonfrontend/src/types/__tests__/drag-drop-context.test.tstsconfig.base.json
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : do not use dotenv; bun automatically loads .env files...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Do not use dotenv; Bun automatically loads .env files
Applied to files:
backend/package.jsontsconfig.base.json
📚 Learning: applies to src/**/*.vue : no inline style attributes in templates...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : No inline style attributes in templates
Applied to files:
frontend/src/components/ColumnPalette.vuefrontend/src/components/WikibaseSchemaEditor.vue
📚 Learning: applies to src/**/*.{vue,ts} : use useapi composable (elysia eden) for all api calls...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Use useApi composable (Elysia Eden) for all API calls
Applied to files:
frontend/src/composables/useSchemaApi.tsfrontend/src/stores/__tests__/schema.store.test.tsfrontend/src/components/WikibaseSchemaEditor.vuefrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/package.jsonfrontend/src/plugins/api.tsfrontend/src/composables/useValueMapping.tsfrontend/src/composables/useProjectCreationComposable.ts
📚 Learning: applies to src/**/*.{ts,vue} : type safety everywhere...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{ts,vue} : Type safety everywhere
Applied to files:
frontend/src/composables/useSchemaApi.tsfrontend/src/composables/__tests__/useValueMapping.test.tsfrontend/src/composables/__tests__/useStatementDataTypeValidation.test.tsfrontend/src/stores/project.store.tsfrontend/package.jsonfrontend/src/types/__tests__/drag-drop.test.tsfrontend/tsconfig.jsonfrontend/src/stores/project-list.store.tsfrontend/src/types/__tests__/drag-drop-context.test.tsfrontend/src/plugins/api.tsfrontend/src/composables/__tests__/useLanguageDropZone.test.tsfrontend/src/composables/useValueMapping.tsfrontend/src/composables/useProjectCreationComposable.tsfrontend/src/stores/__tests__/drag-drop.store.test.tstsconfig.base.json
📚 Learning: applies to src/composables/**/*.ts : do not export store state from composables...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Do not export store state from composables
Applied to files:
frontend/src/stores/__tests__/schema.store.test.tsfrontend/src/composables/__tests__/useSchemaDropZone.test.tsfrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/src/plugins/api.tsfrontend/src/stores/drag-drop.store.tsfrontend/src/composables/useProjectCreationComposable.tsfrontend/src/components/__tests__/ItemEditor.test.tsfrontend/src/stores/__tests__/drag-drop.store.test.ts
📚 Learning: applies to src/composables/**/*.ts : do not proxy or export pinia store state/actions from composabl...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Do not proxy or export Pinia store state/actions from composables
Applied to files:
frontend/src/stores/__tests__/schema.store.test.tsfrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/src/plugins/api.tsfrontend/src/stores/drag-drop.store.tsfrontend/src/components/__tests__/StatementsEditor.test.tsfrontend/src/composables/useProjectCreationComposable.tsfrontend/src/components/__tests__/ItemEditor.test.tsfrontend/src/stores/__tests__/drag-drop.store.test.ts
📚 Learning: applies to src/**/*.{vue,ts} : handle errors and loading states reactively when making api calls...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Handle errors and loading states reactively when making API calls
Applied to files:
frontend/src/stores/project.store.tsfrontend/src/stores/project-list.store.tsfrontend/src/composables/useProjectCreationComposable.ts
📚 Learning: applies to src/**/*.vue : use vue 3 with composition api and <script setup lang="ts"> in all vue com...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use Vue 3 with Composition API and <script setup lang="ts"> in all Vue components
Applied to files:
frontend/src/components/WikibaseSchemaEditor.vuefrontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/package.jsonfrontend/tsconfig.jsonfrontend/src/types/__tests__/drag-drop-context.test.tsfrontend/src/composables/useProjectCreationComposable.tstsconfig.base.json
📚 Learning: applies to src/**/*.vue : use auto-imports for vue, pinia, composables, and utilities...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use auto-imports for Vue, Pinia, composables, and utilities
Applied to files:
frontend/src/components/WikibaseSchemaEditor.vuefrontend/package.jsonfrontend/tsconfig.jsonfrontend/src/plugins/api.tsfrontend/src/stores/drag-drop.store.ts
📚 Learning: applies to src/**/*.vue : use v-memo, shallowref, markraw, and suspense for performance optimization...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use v-memo, shallowRef, markRaw, and Suspense for performance optimization
Applied to files:
frontend/src/components/WikibaseSchemaEditor.vuefrontend/src/types/__tests__/drag-drop-context.test.ts
📚 Learning: applies to src/**/*.vue : prefer composables over methods in vue components...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Prefer composables over methods in Vue components
Applied to files:
frontend/src/components/WikibaseSchemaEditor.vue
📚 Learning: applies to src/composables/**/*.ts : use composables for logic that is not global state...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Use composables for logic that is not global state
Applied to files:
frontend/src/composables/__tests__/useSchemaApi.test.tsfrontend/src/composables/useValueMapping.tsfrontend/src/composables/useProjectCreationComposable.tsfrontend/src/components/__tests__/ItemEditor.test.ts
📚 Learning: applies to src/**/*.vue : use <script setup lang="ts"> at the top, template second, style last (rare...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use <script setup lang="ts"> at the top, template second, style last (rarely used) in Vue components
Applied to files:
frontend/package.jsonfrontend/tsconfig.json
📚 Learning: applies to src/**/*.vue : props and emits must use explicit typescript interfaces...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Props and emits must use explicit TypeScript interfaces
Applied to files:
frontend/package.jsonfrontend/tsconfig.jsonfrontend/src/plugins/api.tsfrontend/src/composables/useValueMapping.tsfrontend/src/composables/useProjectCreationComposable.ts
📚 Learning: applies to src/**/*.vue : build reusable, well-structured components...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Build reusable, well-structured components
Applied to files:
frontend/package.jsonfrontend/tsconfig.jsonfrontend/src/types/__tests__/drag-drop-context.test.ts
📚 Learning: applies to **/*.{html,tsx,jsx,js,css} : use html imports with `bun.serve()` instead of `vite` for fr...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{html,tsx,jsx,js,css} : Use HTML imports with `Bun.serve()` instead of `vite` for frontend development
Applied to files:
frontend/package.jsonfrontend/tsconfig.jsonfrontend/src/plugins/api.tstsconfig.base.json
📚 Learning: applies to src/**/*.{vue,ts} : use readonly and shallowreactive for large or expensive data...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Use readonly and shallowReactive for large or expensive data
Applied to files:
frontend/src/types/__tests__/drag-drop.test.tsfrontend/src/types/__tests__/drag-drop-context.test.ts
📚 Learning: applies to src/**/*.{vue,ts} : use pinia stores for global state...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.{vue,ts} : Use Pinia stores for global state
Applied to files:
frontend/tsconfig.jsonfrontend/src/stores/drag-drop.store.tsfrontend/src/components/__tests__/StatementsEditor.test.tsfrontend/src/components/__tests__/ItemEditor.test.tsfrontend/src/stores/__tests__/drag-drop.store.test.ts
📚 Learning: applies to **/*.html : html files can import .tsx, .jsx, or .js files directly and bun's bundler wil...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.html : HTML files can import .tsx, .jsx, or .js files directly and Bun's bundler will transpile & bundle automatically
Applied to files:
frontend/tsconfig.jsontsconfig.base.json
📚 Learning: applies to src/**/*.vue : prefer reactivity over computed unless necessary...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Prefer reactivity over computed unless necessary
Applied to files:
frontend/src/types/__tests__/drag-drop-context.test.ts
📚 Learning: applies to src/**/*.vue : use storetorefs for state in components...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use storeToRefs for state in components
Applied to files:
frontend/src/types/__tests__/drag-drop-context.test.tsfrontend/src/stores/drag-drop.store.ts
📚 Learning: applies to src/**/*.vue : use reactive objects for form state and errors...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use reactive objects for form state and errors
Applied to files:
frontend/src/types/__tests__/drag-drop-context.test.ts
📚 Learning: applies to **/*.{tsx,jsx,js} : import .css files directly in .tsx, .jsx, or .js files and bun will h...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{tsx,jsx,js} : Import .css files directly in .tsx, .jsx, or .js files and Bun will handle them
Applied to files:
frontend/src/plugins/api.tstsconfig.base.json
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : use built-in `websocket` instead of `ws`...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use built-in `WebSocket` instead of `ws`
Applied to files:
frontend/src/plugins/api.ts
📚 Learning: applies to src/**/*.vue : use pinia for state management in all vue components...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/**/*.vue : Use Pinia for state management in all Vue components
Applied to files:
frontend/src/stores/drag-drop.store.tsfrontend/src/components/__tests__/StatementsEditor.test.tsfrontend/src/components/__tests__/ItemEditor.test.tsfrontend/src/stores/__tests__/drag-drop.store.test.ts
📚 Learning: applies to src/composables/**/*.ts : place composables in the composables/ directory...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/frontend-setup.rule.md:0-0
Timestamp: 2025-07-20T14:13:24.706Z
Learning: Applies to src/composables/**/*.ts : Place composables in the composables/ directory
Applied to files:
frontend/src/composables/useProjectCreationComposable.ts
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : use `bun.sql` for postgres instead of `pg` or `postgres.js`...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use `Bun.sql` for Postgres instead of `pg` or `postgres.js`
Applied to files:
tsconfig.base.json
📚 Learning: applies to **/*.{ts,tsx,js,jsx} : prefer `bun.file` over `node:fs`'s readfile/writefile...
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Prefer `Bun.file` over `node:fs`'s readFile/writeFile
Applied to files:
tsconfig.base.json
🧬 Code Graph Analysis (3)
frontend/src/composables/__tests__/useDragDropContext.test.ts (3)
frontend/src/types/wikibase-schema.ts (1)
ColumnInfo(167-173)frontend/auto-imports.d.ts (2)
ColumnInfo(375-375)DropTarget(369-369)frontend/src/types/drag-drop.ts (1)
DropTarget(35-42)
frontend/src/composables/__tests__/useSchemaApi.test.ts (1)
frontend/src/stores/schema.store.ts (1)
useSchemaStore(14-220)
frontend/src/composables/useProjectCreationComposable.ts (1)
backend/src/types/error-schemas.ts (2)
ApiError(41-44)ApiError(50-50)
🪛 GitHub Actions: Typecheck code
frontend/src/components/WikibaseSchemaEditor.vue
[error] 257-257: TypeScript error TS2322: Type '{ property: { id: PropertyId; label?: string | undefined; dataType: string; }; value: { type: "column"; source: { columnName: string; dataType: string; transformation?: { type: "constant" | "expression" | "lookup"; value: string; parameters?: Record<string, unknown> | undefined; } | undefined; }; dataType: WikibaseD... }' is not assignable to type '{ id: string; property: { id: P${number}; label?: string | undefined; dataType: string; }; value: { type: "column"; source: { columnName: string; dataType: string; transformation?: { type: "constant" | "expression" | "lookup"; value: string; parameters?: Record<...> | undefined; } | undefined; }; dataType: Wikibas... }'. Types of property 'id' are incompatible. Type 'string | undefined' is not assignable to type 'string'. Type 'undefined' is not assignable to type 'string'.
🔇 Additional comments (57)
frontend/src/stores/__tests__/schema.store.test.ts (1)
24-24: LGTM! Excellent improvement to async mocking.Adding
awaitto themock.modulecall ensures the module mock is properly established before tests run, preventing potential race conditions and flaky test behavior.frontend/src/stores/drag-drop.store.ts (1)
3-3: LGTM! Unused import cleanup.Removing the unused
WikibaseDataTypeimport improves code cleanliness and resolves lint warnings.frontend/src/components/ColumnPalette.vue (1)
96-96: LGTM! Template syntax cleanup.Converting
<i>elements to self-closing syntax improves template consistency and follows Vue 3 best practices.Also applies to: 109-109
frontend/src/components/PropertySelector.vue (1)
10-10: LGTM! Explicit prop default improves type safety.Setting an explicit
nulldefault for the optionalmodelValueprop ensures consistent behavior and prevents undefined values.frontend/src/plugins/api.ts (1)
2-2: LGTM! Type-only import syntax improvement.Using
import typeensures the import is erased from the emitted JavaScript and used only for type checking, following TypeScript best practices.frontend/src/composables/__tests__/useSchemaApi.test.ts (1)
40-50: LGTM! Type assertions for test mocking.Using
(globalThis as any)type assertions appropriately bypasses TypeScript strict checking for global mock assignments in tests, resolving typecheck errors without affecting test functionality.Also applies to: 110-125
frontend/src/types/__tests__/drag-drop.test.ts (2)
316-316: LGTM! Excellent type safety improvement.Replacing
anywithHTMLElementprovides better type safety and aligns with the codebase's emphasis on type safety everywhere.
316-316: Excellent type safety improvement.Changing from
{} as anyto{} as HTMLElementprovides better type safety and aligns with the type safety requirements from the retrieved learnings.frontend/src/components/__tests__/StatementsEditor.test.ts (3)
5-9: LGTM! Excellent type safety improvements.Adding the explicit
WikibaseDataTypeimport and typing thedataTypeparameter improves type safety and provides better documentation of expected values. This aligns perfectly with the codebase's emphasis on type safety everywhere.Also applies to: 39-39
5-9: Enhanced type safety with explicit WikibaseDataType import.Adding the
WikibaseDataTypeimport supports the explicit typing improvement seen in thecreateColumnValueMappingfunction.
39-39: Excellent explicit typing for better type safety.Explicitly typing the
dataTypeparameter asWikibaseDataTypeinstead of relying on string literals improves type safety and aligns with the "Type safety everywhere" principle from the retrieved learnings.frontend/src/composables/__tests__/useLanguageDropZone.test.ts (4)
69-71: Good approach - explicit assertions are better than optional chaining.This pattern of explicitly asserting
toBeDefined()followed by non-null assertion (!) is much better than the optional chaining approach used in other test files. It makes test expectations clear and will fail loudly if elements are unexpectedly undefined.However, consider whether these additional assertions are necessary when you're already validating array lengths (e.g.,
expect(mappingDisplayData.value).toHaveLength(1)), which should guarantee element existence.Also applies to: 109-110, 146-148, 231-232
69-71: Excellent testing pattern with explicit assertions.This approach of first asserting
.toBeDefined()and then using non-null assertions (!) is superior to optional chaining in tests. It makes expectations explicit and will fail clearly if assumptions are violated, providing better test feedback than optional chaining which could mask issues.
146-148: Consistent application of the superior testing pattern.The same pattern is applied consistently for alias term type testing, maintaining clear expectations and explicit assertions throughout the test suite.
231-232: Pattern maintains consistency in reactivity tests.The explicit assertion pattern continues to be applied consistently in reactivity tests, ensuring clear failure modes when expectations aren't met.
frontend/src/components/__tests__/ItemEditor.test.ts (1)
49-49: LGTM - Test assertion is correctThe test properly verifies that the store has the expected
enproperty after adding a label mapping. The assertion follows proper testing patterns.frontend/src/stores/project.store.ts (1)
27-27: Excellent type safety improvementRemoving the unnecessary
as anytype assertion is a good cleanup that improves type safety. TheshowErrorfunction should handle the proper error type without needing explicit casting.frontend/src/components/QualifiersEditor.vue (1)
129-129: Good type safety cleanupRemoving the explicit
as WikibaseDataTypecast is appropriate sincegetCompatibleWikibaseTypes()should already return the correct type. This change improves type safety by relying on proper typing..github/workflows/test.yml (1)
21-21: Excellent CI improvementAdding
--frozen-lockfileensures deterministic dependency installation in CI by using exact versions from the lockfile. This prevents potential version drift issues between local development and CI environments.backend/package.json (1)
14-14: Typecheck flags are preserved via tsconfig.json; CI uses Bun’s typecheckThe
noEmitandskipLibCheckoptions are already set in backend/tsconfig.json, and the GitHub Actions workflow invokesbun typecheck(not thenpm run typecheckscript). Removing the Bun prefix and flags from the package.json script won’t affect CI or lose those settings.frontend/src/composables/__tests__/useSchemaDropZone.test.ts (2)
165-165: LGTM - Improved type assertion safetyThe change from direct
as DragEventtoas unknown as DragEventfollows TypeScript best practices for type assertions when casting between significantly different types. This makes the casting more explicit and safer.Also applies to: 193-193, 210-210, 242-242, 301-301
307-308: LGTM - Enhanced null safety with optional chainingAdding optional chaining (
?.) to schemaStore property access prevents potential runtime errors when nested properties might be undefined. This improves the robustness of the test assertions.Also applies to: 361-362, 382-383, 404-405, 429-429, 456-457
frontend/src/composables/__tests__/useStatementEditor.test.ts (1)
235-235: LGTM - Improved test assertion styleUsing
.toHaveProperty('columnName', 'birth_date')instead of direct property access is better practice as it provides clearer error messages and explicitly checks for property existence before comparing values.frontend/src/composables/useStatementEditor.ts (1)
205-205: LGTM - Removed unnecessary type assertionThe explicit cast to
WikibaseDataTypeis unnecessary since TypeScript can properly infer the type from the expression. This simplifies the code while maintaining type safety.frontend/package.json (1)
12-12: No changes needed: CLI flags inherited from tsconfig.base.jsonThe
vue-tscinvocation in frontend/package.json no longer includes--noEmitor--skipLibCheck, but both flags are defined in tsconfig.base.json (and inherited by frontend/tsconfig.json). As a result, the behavior of the typecheck script remains the same—no files will be emitted and library declaration files will still be skipped.• frontend/package.json:
"typecheck": "vue-tsc"
• tsconfig.base.json:
–"noEmit": true
–"skipLibCheck": trueLikely an incorrect or invalid review comment.
frontend/src/components/WikibaseSchemaEditor.vue (2)
31-32: LGTM - Cleaned up unused importsRemoving unused destructured properties (
isValidStatement,getStatement) from theuseStatementEditorcomposable improves code cleanliness and aligns with the PR's cleanup objectives.
312-312: LGTM - Consistent self-closing tag syntaxConverting to self-closing tags (
<i ... />) follows Vue 3 best practices and maintains consistency across the codebase.Also applies to: 371-371, 452-452
tsconfig.base.json (1)
4-6:module/targetbumped toesnext– verify backend runtime compatibility
esnextunlocks TLA,import.meta, etc., which Bun handles gracefully, but any remaining Node-18 executions (scripts, test runners, older Lambdas) will now emit bareimportstatements. Make sure:
- Backend scripts are executed with Bun or an ES-module-ready Node runtime.
- Your published package.json sets
"type":"module"to avoid mixed CJS/ESM confusion.If that is already enforced across the repo, all good.
frontend/tsconfig.json (1)
2-5: MultipleextendsOK, but ensure order won’t override stricter base rules
@vue/tsconfig/tsconfig.dom.jsonsets its ownstrict:false. In most versions the last config wins; you listed it second, so strictness fromtsconfig.base.jsonsurvives – great. If the@vuepackage ever flips to last-one-wins semantics, tests will silently loosen. Consider pinning its major version or moving DOM libs tocompilerOptions.libinstead.frontend/src/composables/useProjectCreationComposable.ts (3)
1-2: LGTM! Improved type safety with backend-inferred types.The import of
ApiErrorfrom backend schemas aligns with the coding guidelines to use backend-inferred types for API data.
37-37: LGTM! Better type safety with explicit ApiError cast.Using the specific
ApiErrortype instead of a generic cast improves type safety and provides better IDE support.
42-46: LGTM! Improved navigation with async/await pattern.The use of
async/awaitwithrouter.pushand the explicit route object format improves code clarity and follows Vue Router best practices.frontend/src/composables/__tests__/useStatementDropZone.test.ts (2)
54-54: LGTM! Safer type assertions in test mocks.Using the
as unknown as DragEventpattern is a safer approach for casting mock objects in tests, as it acknowledges that the mock doesn't fully implement the DragEvent interface.Also applies to: 71-71, 86-86, 109-109, 134-134, 153-153, 180-180, 253-253
151-151: LGTM! More explicit test data.Changing from an empty string to
'invalid json'makes the test intention clearer and more explicit about testing invalid JSON parsing.frontend/src/stores/__tests__/drag-drop.store.test.ts (3)
4-4: LGTM! Added proper type import.Adding the
WikibaseDataTypeimport provides proper typing for the test cases.
302-302: LGTM! Explicit typing improves clarity.Explicitly typing the
testCasesarray withdataTypeas string andexpectedTargetsasWikibaseDataType[]makes the test structure clearer and provides better type safety.
327-327: LGTM! Direct type usage instead of casting.Using the
typedirectly instead of forced casts is safer and more type-correct.frontend/src/composables/__tests__/useDragDropHandlers.test.ts (1)
30-30: LGTM! Consistent safer type assertions.Using the
as unknown as DragEventpattern consistently across all test mocks improves type safety and follows the same pattern as other test files.Also applies to: 54-54, 78-78, 92-92, 109-109, 193-193, 227-227, 251-251
frontend/src/stores/project-list.store.ts (2)
18-18: LGTM! Removed unnecessary type assertions.Directly using
apiError.valueanddata.datawithout type casting improves type safety and relies on proper type inference from the API.Also applies to: 20-20
31-32: LGTM! Better null safety with optional chaining.Using optional chaining (
apiError?.value) provides better null safety and the direct usage ofapiError.valuewithout casting improves type correctness.frontend/src/types/__tests__/drag-drop-context.test.ts (2)
2-2: Good cleanup of unused import.The removal of the unused
computedimport aligns well with the lint cleanup objectives.
48-48: Improved test assertions for better semantics.Using
.toBeNull()instead of.toBe(null)provides more specific and semantically meaningful assertions. This is a good improvement for test readability and specificity.Also applies to: 64-67
frontend/src/composables/__tests__/useStatementConfig.test.ts (2)
6-6: Good addition of explicit type imports and assertions.Adding the
StatementRanktype import and explicit type assertion improves type safety and aligns with the learning that emphasizes "type safety everywhere."Also applies to: 30-30
28-28: Correct update to match new source structure.The change from expecting an empty string to an object
{ columnName: '', dataType: 'VARCHAR' }properly reflects the updated data structure in the composable implementation.Also applies to: 131-131
frontend/src/composables/__tests__/useDragDropContext.test.ts (3)
344-351: Excellent improvement to callback type safety.Adding explicit parameter types
(column: ColumnInfo, target: DropTarget)to the callback signature significantly improves type safety and makes the interface clearer.
370-370: Safer type casting approach.Using
as unknown as DragEventinstead of direct casting is a safer approach that acknowledges the type conversion is intentional and potentially unsafe, which is appropriate for test mocks.Also applies to: 401-401, 430-430, 448-448
406-406: Appropriate use of non-null assertions in tests.The non-null assertions (
!) when accessingdataTransferproperties are appropriate in test contexts where you control the mock data and can guarantee the properties exist.Also applies to: 435-435, 453-453
frontend/src/composables/useStatementConfig.ts (2)
14-14: Good standardization of source structure.Changing the default source from an empty string to an object
{ columnName: '', dataType: 'VARCHAR' }provides better consistency and type safety for column-type statements.
120-137: Improved clarity and type safety in statement saving.The refactored
saveCurrentStatementfunction with explicit variable extraction and conditional logic for different value types significantly improves code readability and ensures proper typing. The explicit handling of column vs non-column types is much clearer.frontend/src/composables/useValueMapping.ts (5)
55-55: Excellent removal of unnecessary type assertions.Replacing type assertions with optional chaining (
?.) is much safer and cleaner. This properly handles cases where the source might be undefined without unsafe casting.Also applies to: 57-57
133-147: Much clearer conditional logic.The refactored
updateValueTypefunction with explicit if/else branches is significantly more readable than the previous conditional expression approach. This makes the different handling for column vs non-column types much clearer.
150-150: Better parameter naming for clarity.Renaming the parameter from
columntocolumnInfois more descriptive and aligns with theColumnInfotype, making the function signature clearer.
160-162: Good safety check for array access.Adding the explicit check
compatibleTypes[0]before assignment prevents potential undefined access when the array is empty. This is a solid defensive programming practice.
71-71: Consistent removal of type assertions throughout.The systematic removal of type assertions (
as any,as string) in favor of proper optional chaining and type-safe access patterns significantly improves the code quality and type safety.Also applies to: 76-76, 174-174, 179-179, 186-186
frontend/src/composables/__tests__/useColumnConversion.test.ts (1)
74-76: Consistent pattern applied correctly.The optional chaining usage is consistent with the broader pattern across the test file. The approach ensures type safety even when array elements might be undefined.
frontend/src/composables/__tests__/useStatementDataTypeValidation.test.ts (2)
85-88: Good defensive programming for error object access.Using optional chaining when accessing error object properties is appropriate here since error structures may vary, and this improves type safety without changing the test logic.
205-207: Consistent application of safe property access.The optional chaining pattern is applied consistently across warning object property access, maintaining the same defensive approach used for error objects.
| expect(result[0]?.name).toBe('name') | ||
| expect(result[0]?.dataType).toBe('VARCHAR') | ||
| expect(result[0]?.sampleValues).toEqual(['John Doe', 'Jane Smith', 'Bob Johnson']) | ||
| expect(result[0]?.nullable).toBe(false) | ||
| expect(result[0]?.uniqueCount).toBe(3) | ||
|
|
||
| // Test second column (age) | ||
| expect(result[1].name).toBe('age') | ||
| expect(result[1].dataType).toBe('INTEGER') | ||
| expect(result[1].sampleValues).toEqual(['25', '30', '35']) | ||
| expect(result[1].nullable).toBe(false) | ||
| expect(result[1].uniqueCount).toBe(3) | ||
| expect(result[1]?.name).toBe('age') | ||
| expect(result[1]?.dataType).toBe('INTEGER') | ||
| expect(result[1]?.sampleValues).toEqual(['25', '30', '35']) | ||
| expect(result[1]?.nullable).toBe(false) | ||
| expect(result[1]?.uniqueCount).toBe(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.
🛠️ Refactor suggestion
Reconsider optional chaining in test assertions - it may mask bugs.
Using optional chaining (?.) in test assertions can mask actual implementation bugs. Since you're asserting expect(result).toHaveLength(2) before accessing result[0] and result[1], these elements should be guaranteed to exist. If they're undefined, that indicates a bug in convertProjectColumnsToColumnInfo that should cause the test to fail loudly.
Consider reverting to direct property access since the length assertion guarantees element existence:
- expect(result[0]?.name).toBe('name')
- expect(result[0]?.dataType).toBe('VARCHAR')
- expect(result[0]?.sampleValues).toEqual(['John Doe', 'Jane Smith', 'Bob Johnson'])
- expect(result[0]?.nullable).toBe(false)
- expect(result[0]?.uniqueCount).toBe(3)
+ expect(result[0].name).toBe('name')
+ expect(result[0].dataType).toBe('VARCHAR')
+ expect(result[0].sampleValues).toEqual(['John Doe', 'Jane Smith', 'Bob Johnson'])
+ expect(result[0].nullable).toBe(false)
+ expect(result[0].uniqueCount).toBe(3)📝 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.
| expect(result[0]?.name).toBe('name') | |
| expect(result[0]?.dataType).toBe('VARCHAR') | |
| expect(result[0]?.sampleValues).toEqual(['John Doe', 'Jane Smith', 'Bob Johnson']) | |
| expect(result[0]?.nullable).toBe(false) | |
| expect(result[0]?.uniqueCount).toBe(3) | |
| // Test second column (age) | |
| expect(result[1].name).toBe('age') | |
| expect(result[1].dataType).toBe('INTEGER') | |
| expect(result[1].sampleValues).toEqual(['25', '30', '35']) | |
| expect(result[1].nullable).toBe(false) | |
| expect(result[1].uniqueCount).toBe(3) | |
| expect(result[1]?.name).toBe('age') | |
| expect(result[1]?.dataType).toBe('INTEGER') | |
| expect(result[1]?.sampleValues).toEqual(['25', '30', '35']) | |
| expect(result[1]?.nullable).toBe(false) | |
| expect(result[1]?.uniqueCount).toBe(3) | |
| expect(result[0].name).toBe('name') | |
| expect(result[0].dataType).toBe('VARCHAR') | |
| expect(result[0].sampleValues).toEqual(['John Doe', 'Jane Smith', 'Bob Johnson']) | |
| expect(result[0].nullable).toBe(false) | |
| expect(result[0].uniqueCount).toBe(3) | |
| // Test second column (age) | |
| expect(result[1]?.name).toBe('age') | |
| expect(result[1]?.dataType).toBe('INTEGER') | |
| expect(result[1]?.sampleValues).toEqual(['25', '30', '35']) | |
| expect(result[1]?.nullable).toBe(false) | |
| expect(result[1]?.uniqueCount).toBe(3) |
🤖 Prompt for AI Agents
In frontend/src/composables/__tests__/useColumnConversion.test.ts around lines
39 to 50, remove the optional chaining (?.) from all property accesses on
result[0] and result[1] in the test assertions. Since you already assert that
result has length 2, these elements must exist, and direct property access will
ensure that any missing elements cause the test to fail loudly instead of
silently passing.
| expect(result[0]?.nullable).toBe(true) | ||
| expect(result[0]?.sampleValues).toEqual(['value1', 'value2']) | ||
| expect(result[0]?.uniqueCount).toBe(2) // Only non-null values counted |
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
Apply consistent approach to array element access in tests.
The same issue applies to all other test cases - optional chaining may hide implementation bugs rather than improve safety when array lengths are pre-validated.
Consider removing optional chaining throughout the test file for consistent and more reliable test assertions.
Also applies to: 93-95, 116-117, 142-143, 167-168, 187-189
🤖 Prompt for AI Agents
In frontend/src/composables/__tests__/useColumnConversion.test.ts around lines
74 to 76 and similarly at lines 93-95, 116-117, 142-143, 167-168, and 187-189,
remove the optional chaining when accessing array elements in test assertions.
Since array lengths are already validated, directly access elements without
optional chaining to ensure tests fail loudly on unexpected undefined values,
improving test reliability and consistency.
| expect(result.errors[0]?.code).toBe('INCOMPATIBLE_DATA_TYPE') | ||
| expect(result.errors[0]?.context?.columnName).toBe('population') | ||
| expect(result.errors[0]?.context?.dataType).toBe('INTEGER') | ||
| expect(result.errors[0]?.context?.targetType).toBe('string') |
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
Reconsider optional chaining in error validation tests.
Using optional chaining (?.) when testing error objects can mask implementation bugs. Since you're asserting expect(result.errors).toHaveLength(1) before accessing result.errors[0], the element should be guaranteed to exist. If it's undefined, that indicates a bug in the validation logic that should cause the test to fail loudly.
Consider reverting to direct property access for more reliable error testing:
- expect(result.errors[0]?.code).toBe('INCOMPATIBLE_DATA_TYPE')
- expect(result.errors[0]?.context?.columnName).toBe('population')
- expect(result.errors[0]?.context?.dataType).toBe('INTEGER')
- expect(result.errors[0]?.context?.targetType).toBe('string')
+ expect(result.errors[0].code).toBe('INCOMPATIBLE_DATA_TYPE')
+ expect(result.errors[0].context?.columnName).toBe('population')
+ expect(result.errors[0].context?.dataType).toBe('INTEGER')
+ expect(result.errors[0].context?.targetType).toBe('string')📝 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.
| expect(result.errors[0]?.code).toBe('INCOMPATIBLE_DATA_TYPE') | |
| expect(result.errors[0]?.context?.columnName).toBe('population') | |
| expect(result.errors[0]?.context?.dataType).toBe('INTEGER') | |
| expect(result.errors[0]?.context?.targetType).toBe('string') | |
| expect(result.errors[0].code).toBe('INCOMPATIBLE_DATA_TYPE') | |
| expect(result.errors[0].context?.columnName).toBe('population') | |
| expect(result.errors[0].context?.dataType).toBe('INTEGER') | |
| expect(result.errors[0].context?.targetType).toBe('string') |
🤖 Prompt for AI Agents
In frontend/src/composables/__tests__/useStatementDataTypeValidation.test.ts
around lines 85 to 88, remove the optional chaining operators when accessing
properties of result.errors[0] since the test already asserts that result.errors
has length 1. Replace all instances of ?. with direct property access (.) to
ensure that if the error object or its properties are missing, the test fails
immediately, revealing potential bugs in the validation logic.
| expect(result.isValid).toBe(false) | ||
| expect(result.errors).toHaveLength(1) | ||
| expect(result.errors[0].code).toBe('INCOMPATIBLE_DATA_TYPE') | ||
| expect(result.errors[0]?.code).toBe('INCOMPATIBLE_DATA_TYPE') |
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
Apply consistent approach to error/warning object access.
The same issue applies to all other error and warning validation tests - optional chaining on array elements that should be guaranteed to exist may hide bugs rather than improve safety.
Also applies to: 122-122, 170-170, 187-187, 205-207, 321-321, 341-341
🤖 Prompt for AI Agents
In frontend/src/composables/__tests__/useStatementDataTypeValidation.test.ts at
line 105 and similarly at lines 122, 170, 187, 205-207, 321, and 341, remove the
optional chaining when accessing error and warning objects in the test
assertions. Since these array elements should always exist in the test context,
directly access them without optional chaining to ensure any missing elements
cause test failures and reveal bugs.
No description provided.