Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughRefactors Wikibase backend APIs: removes instance-routes, introduces a consolidated instances API under /api/wikibase, reorganizes barrel exports, and rewires app usage. Updates tests to new API shapes and mounts. Adjusts Wikidata predefined config. Migrates ESLint configs to defineConfig with eslint-config-prettier and adds root tsconfig. Marks docs tasks as completed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as Elysia /api/wikibase
participant Plugins as cors/db/errorHandler/wikibasePlugin
participant WB as Wikibase Service
participant CVS as ConstraintValidationService
Client->>API: GET /:instanceId/properties/search?q=...
API->>Plugins: Apply middleware stack
API->>WB: searchProperties(instanceId, query, options)
WB-->>API: results
API-->>Client: { data: { results, searchMetadata, usageStats? } }
Client->>API: GET /:instanceId/properties/:propertyId
API->>Plugins: Validate IDs, apply error handler
API->>WB: getProperty(instanceId, propertyId)
alt includeConstraints
API->>CVS: getPropertyConstraints(instanceId, propertyId)
CVS-->>API: constraints
end
API-->>Client: { data: property(+constraints?) }
Client->>API: POST /:instanceId/validate/property
API->>CVS: validateProperty(instanceId, propertyId, value[])
CVS-->>API: validationResult
API-->>Client: { data: validationResult }
Client->>API: GET /:instanceId/items/:itemId
API->>WB: getItem(instanceId, itemId)
WB-->>API: item or not found
API-->>Client: { data: item } or error (404)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (16)
.kiro/specs/mwn-wikibase-integration/tasks.md (4)
58-63: Document status codes and error payload for property details endpoint.Callers need explicit behavior for 404/5xx and error shape; add a couple bullets.
Apply:
- - Implement error handling for property not found scenarios + - Implement error handling for property not found scenarios + - Status codes: 200 (OK), 404 (property not found), 502/504 (upstream error/timeout) + - Error payload (example): { "error": "NotFound", "message": "Property P123 not found", "requestId": "<uuid>" }
64-69: Add minimal request/response examples for validation endpoints.Tiny examples help consumers and test authors align on shapes.
Apply:
- - Add /api/wikibase/:instanceId/validate/property endpoint for real-time validation + - Add /api/wikibase/:instanceId/validate/property endpoint for real-time validation + - Example request: { "propertyId": "P123", "value": "foo", "lang": "en" } + - Example response: { "ok": false, "violations": [{ "type": "FormatConstraint", "message": "Must match ^[A-Z]+$" }] } - - Implement /api/wikibase/:instanceId/validate/schema endpoint for schema validation + - Implement /api/wikibase/:instanceId/validate/schema endpoint for schema validation + - Example request: { "schema": { "properties": [...] } } + - Example response: { "ok": true, "violations": [] }
70-75: Note authN/Z, timeouts, and health-check semantics for instance management.Docs should capture operational expectations for SREs and clients.
Apply:
- - Add connectivity testing using nodemw client validation + - Add connectivity testing using nodemw client validation + - Authentication/authorization: require instance-scoped RBAC; redact secrets in logs - - Implement health check endpoints for instance monitoring + - Implement health check endpoints for instance monitoring + - Health semantics: 200 when upstream ping < 1s and auth valid; include { "latencyMs": N, "upstream": "ok|degraded|down" } + - Timeouts/retries: client timeout 2–5s with jittered retry (max 2); surface 504 on exhaustion
51-57: Clarify completion details and add verification pointers for Section 4 intro/search.Marking these as done is fine. Please add a brief provenance note and pointers to source-of-truth to reduce ambiguity for future readers.
Apply this small doc tweak:
- - [x] 4. Update API endpoints to use NodemwWikibaseService + - [x] 4. Update API endpoints to use NodemwWikibaseService + - Completion: PR #152 (2025-09-07). References: unified router mount under /api/wikibase and updated tests.Optional: verify the routes exist and are mounted once under the unified surface:
#!/bin/bash # Verify unified mount and search route wiring rg -nP -C2 '/api/wikibase(?=["'\''])' rg -nP -C2 '/api/wikibase/.+?/properties/search\b'tsconfig.json (1)
1-5: Root tsconfig scope looks good.Covers root-level TS/config files and cleanly excludes app packages. Minor: if you use Bun’s TS config file (bunfig.ts), consider including it so type-aware tools pick it up.
"include": ["*.config.*", "*.ts"], + // Include Bun config if present + // "include": ["*.config.*", "*.ts", "bunfig.ts"],frontend/eslint.config.ts (2)
11-19: Ignore Bun’s actual lockfile name.Bun’s lockfile is bun.lockb (binary), not bun.lock.
'*.json', - 'bun.lock', + 'bun.lockb', 'dist/**',
9-54: Place eslint-config-prettier last to ensure it wins over prior rule sets.Having your own rule block after prettierConfig can re-enable formatting rules. Put prettierConfig last.
-export default defineConfig( +export default defineConfig( { ignores: [ '*.config.*', '*.d.ts', '*.json', - 'bun.lock', + 'bun.lockb', 'dist/**', 'node_modules/**', 'src/**/*.test.ts', ], }, js.configs.recommended, ...pluginVue.configs['flat/recommended'], ...tseslint.configs.recommendedTypeChecked, - prettierConfig, { files: ['**/*.{ts,vue}'], languageOptions: { parser: vueParser, parserOptions: { ecmaVersion: 'latest', parser: tseslint.parser, sourceType: 'module', projectService: true, tsconfigRootDir: import.meta.dirname, extraFileExtensions: ['.vue'], }, globals: { ...autoImportGlobals.globals, }, }, plugins: { '@typescript-eslint': tseslint.plugin, }, rules: { 'vue/multi-word-component-names': 'off', 'vue/singleline-html-element-content-newline': 'off', 'vue/html-indent': 'off', '@typescript-eslint/ban-ts-comment': 'off', '@typescript-eslint/no-unsafe-assignment': 'off', '@typescript-eslint/no-explicit-any': 'off', 'comma-dangle': ['error', 'always-multiline'], }, }, + prettierConfig, )eslint.config.ts (3)
9-10: Also ignore Bun’s lockfile.- ignores: ['dist', 'node_modules', '**/*.d.ts'], + ignores: ['dist', 'node_modules', '**/*.d.ts', 'bun.lockb'],
12-18: Add Node globals at root to avoid false undefineds in scripts.Root config often targets repo scripts; merging browser + node globals is safe here.
- languageOptions: { - globals: globals.browser, + languageOptions: { + globals: { ...globals.browser, ...globals.node }, parserOptions: { projectService: true, tsconfigRootDir: import.meta.dirname, },
7-33: Place eslint-config-prettier last.Keeps formatting rules consistently disabled after all other configs.
export default defineConfig( { ignores: ['dist', 'node_modules', '**/*.d.ts', 'bun.lockb'], }, { languageOptions: { globals: { ...globals.browser, ...globals.node }, parserOptions: { projectService: true, tsconfigRootDir: import.meta.dirname, }, }, }, pluginJs.configs.recommended, ...tseslint.configs.recommended, - prettierConfig, { rules: { '@typescript-eslint/no-explicit-any': 'off', '@typescript-eslint/ban-ts-comment': 'off', '@typescript-eslint/no-unused-vars': [ 'warn', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }, ], }, }, + prettierConfig, )backend/eslint.config.ts (1)
1-3: Backend: add Node globals and keep Prettier last.Prevents “undefined” for Node built-ins and ensures Prettier disables formatting rules after all configs.
-import prettierConfig from 'eslint-config-prettier' -import { defineConfig } from 'eslint/config' +import prettierConfig from 'eslint-config-prettier' +import { defineConfig } from 'eslint/config' +import globals from 'globals' import tseslint from 'typescript-eslint' export default defineConfig( { ignores: ['node_modules/**', 'dist/**', '*.d.ts', 'dataforge.db'], }, ...tseslint.configs.recommended, - prettierConfig, { files: ['**/*.ts'], languageOptions: { - parserOptions: { + globals: { ...globals.node }, + parserOptions: { projectService: true, tsconfigRootDir: import.meta.dirname, }, }, plugins: { '@typescript-eslint': tseslint.plugin, }, rules: { '@typescript-eslint/no-explicit-any': 'warn', '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], '@typescript-eslint/ban-ts-comment': [ 'error', { 'ts-expect-error': 'allow-with-description' }, ], 'comma-dangle': ['error', 'always-multiline'], }, }, + prettierConfig, )Also applies to: 12-18
backend/src/api/wikibase/entities.ts (1)
15-21: Whitespace-only changes — no functional impactLooks good. One small note: CORS is also applied at the app level (backend/src/index.ts). Consider removing duplicate CORS usage here to avoid double headers.
backend/src/index.ts (1)
41-43: Route mounting order and duplicate CORSOrder is fine (entities under /api/wikibase/entities, instances under /api/wikibase). Since both routers also apply CORS internally, consider removing one level (prefer app-level) to prevent duplicate headers.
backend/tests/api/wikibase/constraints.test.ts (1)
5-6: Use named import for Elysia for consistencyElsewhere in the repo, Elysia is imported as a named export. Consider aligning to reduce interop edge cases.
Apply:
-import Elysia from 'elysia' +import { Elysia } from 'elysia'backend/src/api/wikibase/instances.ts (2)
121-172: language query param is unusedlanguage is declared and documented but not used in getProperty. Either plumb it through (if supported) or remove it from the contract to avoid misleading clients.
- query: { includeConstraints = false }, + query: { includeConstraints = false, language = 'en' }, ... - // (if unsupported) remove language from schema: - query: t.Object({ - includeConstraints: t.Optional( - t.Boolean({ description: 'Include property constraints in response', default: false }), - ), - language: t.Optional( - t.String({ description: 'Language code for labels and descriptions', default: 'en' }), - ), - }), + // or (preferred) use it when requesting labels if your service supports language
204-233: Property validation — array coercionWrapping single value into an array matches the service contract. Consider documenting that the service expects an array to keep callers aligned.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.kiro/specs/mwn-wikibase-integration/tasks.md(1 hunks)backend/eslint.config.ts(1 hunks)backend/src/api/wikibase/entities.ts(4 hunks)backend/src/api/wikibase/index.ts(1 hunks)backend/src/api/wikibase/instance-routes.ts(0 hunks)backend/src/api/wikibase/instances.ts(1 hunks)backend/src/index.ts(2 hunks)backend/src/services/wikibase-config.service.ts(1 hunks)backend/tests/api/wikibase/constraints.test.ts(4 hunks)backend/tests/api/wikibase/instances.test.ts(0 hunks)backend/tests/services/nodemw-wikibase.service.test.ts(1 hunks)backend/tests/services/wikibase-api.service.test.ts(3 hunks)backend/tests/services/wikibase-config.service.test.ts(8 hunks)eslint.config.ts(1 hunks)frontend/eslint.config.ts(2 hunks)tsconfig.base.json(0 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (3)
- tsconfig.base.json
- backend/src/api/wikibase/instance-routes.ts
- backend/tests/api/wikibase/instances.test.ts
🧰 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:
backend/tests/services/nodemw-wikibase.service.test.tsbackend/src/api/wikibase/entities.tsbackend/src/services/wikibase-config.service.tsbackend/tests/services/wikibase-api.service.test.tsfrontend/eslint.config.tsbackend/src/api/wikibase/index.tsbackend/tests/api/wikibase/constraints.test.tsbackend/src/index.tseslint.config.tsbackend/src/api/wikibase/instances.tsbackend/tests/services/wikibase-config.service.test.tsbackend/eslint.config.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:
backend/tests/services/nodemw-wikibase.service.test.tsbackend/tests/services/wikibase-api.service.test.tsbackend/tests/api/wikibase/constraints.test.tsbackend/tests/services/wikibase-config.service.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:
backend/tests/services/nodemw-wikibase.service.test.tsbackend/src/api/wikibase/entities.tsbackend/src/services/wikibase-config.service.tsbackend/tests/services/wikibase-api.service.test.tsfrontend/eslint.config.tsbackend/src/api/wikibase/index.tsbackend/tests/api/wikibase/constraints.test.tsbackend/src/index.tseslint.config.tsbackend/src/api/wikibase/instances.tsbackend/tests/services/wikibase-config.service.test.tsbackend/eslint.config.ts
🔇 Additional comments (16)
backend/tests/services/wikibase-api.service.test.ts (1)
311-314: I’ve requested the relevant context around both the test and its implementation to verify whether parsing errors should reject or resolve. Once we have those snippets, I’ll confirm the intended behavior and update the review accordingly.frontend/eslint.config.ts (1)
2-4: Verify package versions support Flat Config + projectService.defineConfig (eslint@9) and tseslint projectService require recent versions. Ensure eslint, typescript-eslint, and eslint-config-prettier are compatible.
#!/bin/bash # Check installed versions jq -r '.devDependencies.eslint, .devDependencies["typescript-eslint"], .devDependencies["eslint-config-prettier"]' package.json npm ls --depth=0 eslint typescript-eslint eslint-config-prettier 2>/dev/null || truebackend/tests/services/nodemw-wikibase.service.test.ts (1)
198-206: Expectation fixed to 'datatype' — aligns with mocked shapeSwitching from dataType to datatype matches the mocked response and the service shape. LGTM.
backend/src/services/wikibase-config.service.ts (1)
49-58: Predefined Wikidata config bumped to v1 + updated UAThe baseUrl (…/wikibase/v1), userAgent, and metadata.version updates look correct and consistent with tests.
backend/src/index.ts (1)
5-9: Barrel import consolidation looks goodCentralizing wikibase* APIs via @backend/api/wikibase simplifies wiring and avoids path churn.
backend/tests/services/wikibase-config.service.test.ts (2)
26-41: Promise-based assertions improve readabilitySwitch to resolves/rejects across tests is clean and consistent with Bun’s test style.
164-166: Consistent resolves usage after state changesUsing resolves for post-ops checks (empty lists, equality of retrieved configs) is correct and avoids unnecessary awaits.
Also applies to: 374-389, 405-406, 427-428
backend/src/api/wikibase/index.ts (1)
1-5: Barrel re-exports are tidy and discoverableHandlers/schemas and API surfaces are cleanly exposed. 👍
backend/tests/api/wikibase/constraints.test.ts (3)
1-1: Import from barrel is correctUsing wikibaseConstraintsApi from the barrel matches the new structure.
34-36: Lightweight test API factory — niceMounting only the constraints plugin under test keeps scope minimal and fast.
48-65: Response-shape assertions match the treaty patternAsserting { data, status, error } is correct with treaty. Tests are clear and deterministic.
Also applies to: 68-88, 91-113, 116-130
backend/src/api/wikibase/instances.ts (5)
174-202: Constraints endpoint — ID validation and response shape look goodStraightforward and consistent with details route validation.
235-254: Schema validation — OKContract and response wrapping match other endpoints.
256-298: Item search — mirrors entities API correctlyValidation and response wrapper are consistent.
300-328: Item details — solid validation and 404 pathLooks good.
29-85: ```shell
#!/bin/bash
set -eLocate the properties search endpoint file
file=$(rg -l "properties/search" -g "backend/src/api/wikibase/instances.ts")
echo "Instances file: $file"Show the .get definition for the properties/search endpoint
rg -nP ".get(\s*'/:instanceId/properties/search'" -C5 "$file"
Inspect the query schema definition in that route
rg -nP "query:\s*t.Object" -C5 "$file"
Check how searchProperties is invoked
rg -n "searchProperties" -C3 "$file"
Locate the entities search endpoint to compare param naming
entityFile=$(rg -l "searchEntities" -g "backend/src/api/wikibase/.ts")
echo "Entities file: $entityFile"
rg -nP ".get(\s'/:instanceId/entities/search'" -C5 "$entityFile"
rg -n "searchEntities" -C3 "$entityFile"</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| expect(service.searchProperties('test-instance', 'test', searchOptions)).resolves.toThrow() | ||
| }) |
There was a problem hiding this comment.
Fix matcher: use rejects.toThrow() for async errors.
.resolves.toThrow() expects the promise to resolve to a function that throws, which is not the case here. Keep promise-rejection semantics consistent with the rest of this file.
- expect(service.searchProperties('test-instance', 'test', searchOptions)).resolves.toThrow()
+ expect(service.searchProperties('test-instance', 'test', searchOptions)).rejects.toThrow()To confirm intended behavior, run this repo-only scan to see whether these methods throw (reject) or return Error (resolve):
#!/bin/bash
# Inspect WikibaseApiService method implementations and error handling
fd -a 'wikibase-api.service.ts' backend | while read -r f; do
echo "== $f =="
rg -nC2 -P '(class\s+WikibaseApiService\b|^\s*(async\s+)?(searchProperties|getProperty|getItem|getPropertyConstraints|getPropertyDataTypes)\s*\()' "$f"
echo "--- throws in methods ---"
rg -n -P 'throw\s+new\s+Error' "$f"
echo
done🤖 Prompt for AI Agents
In backend/tests/services/wikibase-api.service.test.ts around lines 259-260, the
test uses expect(service.searchProperties(...)).resolves.toThrow() which is
incorrect for an async function that rejects; change the matcher to
expect(service.searchProperties('test-instance','test',searchOptions)).rejects.toThrow()
(or await expect(...).rejects.toThrow()) so the test asserts promise rejection
correctly and matches the rest of the file's async error semantics.
| expect(service.getProperty('test-instance', 'P999999' as PropertyId)).resolves.toThrow() | ||
| }) |
There was a problem hiding this comment.
Invalid property ID test should reject, not resolve.
A promise that fails should be asserted with .rejects.toThrow().
- expect(service.getProperty('test-instance', 'P999999' as PropertyId)).resolves.toThrow()
+ expect(service.getProperty('test-instance', 'P999999' as PropertyId)).rejects.toThrow()📝 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(service.getProperty('test-instance', 'P999999' as PropertyId)).resolves.toThrow() | |
| }) | |
| expect(service.getProperty('test-instance', 'P999999' as PropertyId)).rejects.toThrow() | |
| }) |
🤖 Prompt for AI Agents
In backend/tests/services/wikibase-api.service.test.ts around lines 273 to 274,
the test for an invalid property ID is asserting a failing promise with
.resolves.toThrow(), which is incorrect; change the assertion to use
.rejects.toThrow() so the test expects the promise to reject, i.e., replace
.resolves.toThrow() with .rejects.toThrow() on the call to
service.getProperty('test-instance', 'P999999' as PropertyId).
| expect(service.getItem('test-instance', 'Q999999' as ItemId)).resolves.toThrow() | ||
| }) |
There was a problem hiding this comment.
Invalid item ID test should reject, not resolve.
- expect(service.getItem('test-instance', 'Q999999' as ItemId)).resolves.toThrow()
+ expect(service.getItem('test-instance', 'Q999999' as ItemId)).rejects.toThrow()📝 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(service.getItem('test-instance', 'Q999999' as ItemId)).resolves.toThrow() | |
| }) | |
| expect(service.getItem('test-instance', 'Q999999' as ItemId)).rejects.toThrow() | |
| }) |
🤖 Prompt for AI Agents
In backend/tests/services/wikibase-api.service.test.ts around lines 277-278, the
test for an invalid item ID currently uses
expect(service.getItem('test-instance', 'Q999999' as ItemId)).resolves.toThrow()
which is wrong; change it to expect(service.getItem('test-instance', 'Q999999'
as ItemId)).rejects.toThrow() (or await expect(...).rejects.toThrow()) so the
promise rejection is asserted rather than a resolved value.
| expect(service.getPropertyConstraints('test-instance', 'P1' as PropertyId)).resolves.toThrow() | ||
| }) |
There was a problem hiding this comment.
Test description and assertion disagree; expect empty array, not a thrown error.
The description says “return empty constraints array,” but the matcher expects a thrown error. Align the assertion with the description.
- expect(service.getPropertyConstraints('test-instance', 'P1' as PropertyId)).resolves.toThrow()
+ expect(
+ service.getPropertyConstraints('test-instance', 'P1' as PropertyId)
+ ).resolves.toEqual([])📝 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(service.getPropertyConstraints('test-instance', 'P1' as PropertyId)).resolves.toThrow() | |
| }) | |
| expect( | |
| service.getPropertyConstraints('test-instance', 'P1' as PropertyId) | |
| ).resolves.toEqual([]) | |
| }) |
🤖 Prompt for AI Agents
In backend/tests/services/wikibase-api.service.test.ts around lines 307 to 308,
the test description says the method should "return empty constraints array" but
the assertion expects a thrown error; change the assertion to match the
description by expecting a resolved empty array (e.g., use await or .resolves
and assert toEqual([]) or toStrictEqual([])) so the test verifies the method
returns an empty array instead of throwing.
1 participant
No description provided.