feat(wikibase-integration): implement instance config management#140
feat(wikibase-integration): implement instance config management#140
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a full Wikibase instance configuration service with validation, connectivity, and health checks; exposes CRUD and auxiliary endpoints via a new /wikibase/instances API; registers the router in the app; updates specs to mark tasks complete; and introduces comprehensive unit/integration tests for service and API. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant A as Elysia App
participant R as wikibaseInstancesApi
participant S as wikibaseConfigService
participant W as Remote Wikibase
C->>A: HTTP GET /wikibase/instances
A->>R: Route dispatch
R->>S: getInstances()
S-->>R: instances[]
R-->>A: 200 JSON
A-->>C: instances list
C->>A: HTTP POST /wikibase/instances (config)
A->>R: Route dispatch + validate body
R->>S: addInstance(config)
S->>S: validateInstance(config)
S-->>R: ok
R-->>A: 200 JSON
A-->>C: added
C->>A: HTTP POST /wikibase/instances/validate (config)
A->>R: Route dispatch + validate body
R->>S: validateInstanceWithConnectivity(config)
S->>W: test request (e.g., /openapi.json)
W-->>S: status/spec
S-->>R: validation + connectivity
R-->>A: 200 JSON
A-->>C: validation result
C->>A: HTTP GET /wikibase/instances/:id/health
A->>R: Route dispatch
R->>S: performHealthCheck(id)
S->>W: connectivity probe
W-->>S: response / error
S-->>R: health summary
R-->>A: 200 JSON
A-->>C: health result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (12)
.kiro/specs/wikibase-rest-api-integration/tasks.md (1)
45-59: Task 4 status updates look good; verify whether 5.3 should also be checked given new /wikibase/instances API is implementedThis PR adds the Wikibase Instances API router and endpoints. If those map to “5.3 Implement instance management endpoints,” consider marking 5.3 as completed (or add a note clarifying scope if it’s intentionally separate).
backend/tests/services/wikibase-config-connectivity.test.ts (2)
220-231: Stabilize timeout-sensitive test to reduce flakinessNetwork conditions vary in CI. Consider explicitly limiting this test’s runtime to avoid build hangs and reduce variance.
Example (if supported by bun:test in your setup):
- test('should handle network timeouts gracefully', async () => { + test('should handle network timeouts gracefully', { timeout: 15000 }, async () => { const timeoutConfig = { ...validConfig, baseUrl: 'https://httpbin.org/delay/2', // Shorter delay to avoid test timeout }If the object options form isn’t supported in your version, wrap with a manual AbortController in the service under test (see related suggestion in wikibase-config.service.ts).
260-270: Use a valid but closed port for “connection refused”Port 99999 is outside the valid range (0–65535); parsing may fail before a connection attempt, producing inconsistent error shapes. Prefer a valid but presumably closed port to consistently trigger ECONNREFUSED.
- baseUrl: 'http://localhost:99999', // Port that should be closed + baseUrl: 'http://localhost:65535', // Valid port, likely closed locally/CIbackend/src/services/wikibase-config.service.ts (2)
370-413: Optional: Drop ApiClient dependency and build requests directlyYou’re using ApiClient only for basePath and default headers, while all requests are done via fetch. Removing the dependency avoids the missing types problem altogether.
If desired, I can provide a concrete refactor to replace ApiClient with a small internal helper (buildHeaders + sanitized baseUrl).
495-534: Enforce fetch timeouts in makeTestRequest
Add a 10 s AbortController-based timeout around both HTTP calls to avoid hanging indefinitely and clear the timer on success.File:
backend/src/services/wikibase-config.service.ts(around lines 495–534)--- a/backend/src/services/wikibase-config.service.ts +++ b/backend/src/services/wikibase-config.service.ts @@ private async makeTestRequest(apiClient: ApiClient): Promise<{ - try { + try { + const controller = new AbortController() + const timer = setTimeout(() => controller.abort(), 10_000) // First, try to get the OpenAPI specification - const response = await fetch(`${apiClient.basePath}/openapi.json`, { + const response = await fetch( + `${apiClient.basePath.replace(/\/+$/, '')}/openapi.json`, + { + method: 'GET', + headers: apiClient.defaultHeaders, + signal: controller.signal, + } + ) @@ if (response.ok) { const openApiSpec = await response.json() as any + clearTimeout(timer) return { @@ // If OpenAPI spec is not available, try a simple GET request to the base path - const baseResponse = await fetch(apiClient.basePath, { + const baseResponse = await fetch(apiClient.basePath, { + method: 'GET', + headers: apiClient.defaultHeaders, + signal: controller.signal, + }) + clearTimeout(timer) return { status: baseResponse.status, apiVersion: 'unknown', features: [], }This guards both the OpenAPI and base-path checks against slow or unresponsive endpoints and ensures the timeout is cleaned up on success.
backend/tests/services/wikibase-config.service.test.ts (3)
25-49: Tests assume two predefined instances; ensure service provides bothThese assertions will fail until the service defines both wikidata and wikimedia-commons. The service currently only ships wikidata. See suggested fix in wikibase-config.service.ts to add wikimedia-commons to PREDEFINED_INSTANCES.
155-160: isPredefinedInstance for wikimedia-commons depends on service dataThis check will fail until wikimedia-commons is added as a predefined instance. After adding it to the service, this test should pass as-is.
352-354: Fix failing count assertion (expected 2, received 1) by syncing with serviceThis relies on service exposing exactly two predefined instances. After adding wikimedia-commons to PREDEFINED_INSTANCES, this should pass.
backend/src/api/wikibase/instances.ts (3)
200-211: Standardize error responses instead of throwing bare errors.A few routes
throw new Error(...), which bypasses your normalized error envelope used elsewhere. Return a consistent JSON error with appropriate status codes to improve DX and Swagger fidelity.Example for POST
/validate:- } catch (error) { - throw new Error(`Failed to validate instance configuration: ${error instanceof Error ? error.message : 'Unknown error'}`) - } + } catch (error) { + return { + success: false, + data: [], + errors: [ + { + code: 'INTERNAL', + message: error instanceof Error ? error.message : 'Failed to validate instance configuration', + }, + ], + } + }Do the same for GET
/and GET/default.Also applies to: 15-17, 315-316
116-131: Consider disallowingisDefaultupdates via PUT to avoid conflicting flows.You already provide a dedicated
POST /:instanceId/set-default. AllowingisDefaultmutations viaPUTencourages clients to set defaults two ways and increases the risk of multiple defaults. Recommend removingisDefaultfrom the update schema and delegating default management to the dedicated endpoint.
26-54: Add response schemas to tighten typings and improve Swagger.You’re defining input schemas (params/body), but not response schemas. Adding
response: t.Object({...})per route will:
- Improve type-safety
- Remove ambiguity for clients
- Help Swagger generate more precise docs
If desired, introduce a shared
ApiEnvelope<T>response type in a central module and reference it across routes.Also applies to: 56-94, 96-136, 138-166, 168-198, 267-296, 298-323
backend/tests/api/wikibase/instances.test.ts (1)
372-387: Singleton service state may leak across tests; consider resetting between tests.
wikibaseConfigServiceis a singleton and API calls mutate process-wide state (add/remove/set-default). To prevent test order dependency, reset state in abeforeEach/afterEachusing a public reset helper (service-level reset or API endpoint for tests). If the service hasreset()or similar, invoke it; otherwise, we can add a guarded test-only reset.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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 (7)
.kiro/specs/wikibase-rest-api-integration/tasks.md(1 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/instances.test.ts(1 hunks)backend/tests/services/wikibase-config-connectivity.test.ts(1 hunks)backend/tests/services/wikibase-config.service.test.ts(1 hunks)
🧰 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/src/index.tsbackend/tests/services/wikibase-config.service.test.tsbackend/src/api/wikibase/instances.tsbackend/tests/services/wikibase-config-connectivity.test.tsbackend/src/services/wikibase-config.service.tsbackend/tests/api/wikibase/instances.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/src/index.tsbackend/tests/services/wikibase-config.service.test.tsbackend/src/api/wikibase/instances.tsbackend/tests/services/wikibase-config-connectivity.test.tsbackend/src/services/wikibase-config.service.tsbackend/tests/api/wikibase/instances.test.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/wikibase-config.service.test.tsbackend/tests/services/wikibase-config-connectivity.test.tsbackend/tests/api/wikibase/instances.test.ts
🧬 Code Graph Analysis (5)
backend/src/index.ts (1)
backend/src/api/wikibase/instances.ts (1)
wikibaseInstancesApi(7-323)
backend/src/api/wikibase/instances.ts (1)
backend/src/services/wikibase-config.service.ts (1)
wikibaseConfigService(601-601)
backend/tests/services/wikibase-config-connectivity.test.ts (1)
backend/src/types/wikibase-api.ts (1)
WikibaseInstanceConfig(6-18)
backend/src/services/wikibase-config.service.ts (1)
backend/src/types/wikibase-api.ts (1)
WikibaseInstanceConfig(6-18)
backend/tests/api/wikibase/instances.test.ts (2)
backend/src/types/wikibase-api.ts (1)
WikibaseInstanceConfig(6-18)backend/src/index.ts (1)
elysiaApp(13-40)
🪛 GitHub Check: test
backend/tests/services/wikibase-config.service.test.ts
[failure] 353-353: error: expect(received).toBe(expected)
Expected: 2
Received: 1
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/services/wikibase-config.service.test.ts:353:32)
[failure] 158-158: error: expect(received).toBe(expected)
Expected: true
Received: false
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/services/wikibase-config.service.test.ts:158:65)
[failure] 57-57: error: expect(received).toBe(expected)
Expected: "https://www.wikidata.org/w/rest.php/wikibase/v0"
Received: "https://www.wikidata.org/w/rest.php/wikibase/v1"
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/services/wikibase-config.service.test.ts:57:32)
[failure] 45-45: error: expect(received).toBe(expected)
Expected: 2
Received: 1
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/services/wikibase-config.service.test.ts:45:42)
[failure] 31-31: error: expect(received).toBe(expected)
Expected: true
Received: false
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/services/wikibase-config.service.test.ts:31:65)
backend/tests/api/wikibase/instances.test.ts
[failure] 40-40: error: expect(received).toBe(expected)
Expected: true
Received: false
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/api/wikibase/instances.test.ts:40:86)
🪛 GitHub Actions: Run tests
backend/tests/services/wikibase-config.service.test.ts
[error] 1-1: Test assertion failed. Expected: true.
🪛 GitHub Actions: Typecheck code
backend/tests/services/wikibase-config.service.test.ts
[error] 71-71: TypeScript error TS2532: Object is possibly 'undefined'.
[error] 76-76: TypeScript error TS2532: Object is possibly 'undefined'.
[error] 90-90: TypeScript error TS2532: Object is possibly 'undefined'.
backend/tests/services/wikibase-config-connectivity.test.ts
[error] 196-196: TypeScript error TS6133: 'mockOpenApiSpec' is declared but its value is never read.
backend/src/services/wikibase-config.service.ts
[error] 2-2: TypeScript error TS7016: Could not find a declaration file for module '@wmde/wikibase-rest-api'. '/home/runner/work/dataforge/dataforge/node_modules/@wmde/wikibase-rest-api/dist/index.js' implicitly has an 'any' type.
backend/tests/api/wikibase/instances.test.ts
[error] 34-40: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 55-57: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 70-73: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 92-93: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 117-120: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 149-152: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 184-185: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 203-206: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 233-234: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 247-250: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 269-272: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 296-298: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 313-316: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 329-332: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 347-352: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 365-368: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 383-385: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 412-413: TypeScript error TS18046: 'data' is of type 'unknown'.
[error] 426-429: TypeScript error TS18046: 'data' is of type 'unknown'.
🔇 Additional comments (4)
backend/src/index.ts (1)
11-11: Router registration LGTMwikibaseInstancesApi is imported and mounted after wikibaseRoutes; order makes sense. No issues.
Also applies to: 37-37
backend/src/api/wikibase/instances.ts (1)
232-256: Health endpoint design looks solid.Returning 200 with a health payload (even when the instance doesn’t exist) is pragmatic for monitoring. The error information is conveyed in the body and won’t break scrapers that expect 200s.
backend/tests/api/wikibase/instances.test.ts (2)
254-300: LGTM: validation endpoints behavior is consistent with the service.‘Validate’ returns an explicit validation result (200 on both valid/invalid), which is a good UX for clients testing configs without persisting.
336-370: LGTM: health endpoint contract is pragmatic and assertions cover both paths.Asserting a 200 with structured payload for both existing and non-existent instances aligns with the API’s design.
| .get('/', async () => { | ||
| try { | ||
| const instances = await wikibaseConfigService.getInstances() | ||
| return { | ||
| success: true, | ||
| data: instances, | ||
| } | ||
| } catch (error) { | ||
| throw new Error(`Failed to get instances: ${error instanceof Error ? error.message : 'Unknown error'}`) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not expose authToken in API responses (redact or omit it).
The GET endpoints currently return the full instance object as provided by the service. If authToken is present on an instance, this leaks a credential to any API consumer. Redact or omit the token consistently in:
- GET /
- GET /:instanceId
- GET /default
A minimal redaction approach:
.get('/', async () => {
try {
- const instances = await wikibaseConfigService.getInstances()
+ const instances = await wikibaseConfigService.getInstances()
+ const redacted = instances.map(({ authToken, ...rest }) => rest)
return {
success: true,
- data: instances,
+ data: redacted,
}
} catch (error) {
throw new Error(`Failed to get instances: ${error instanceof Error ? error.message : 'Unknown error'}`)
}
}, {Similarly apply for GET /:instanceId and GET /default before returning the payload.
Also applies to: 26-33, 298-316
| body: t.Object({ | ||
| id: t.String({ description: 'Unique instance identifier' }), | ||
| name: t.String({ description: 'Human-readable instance name' }), | ||
| baseUrl: t.String({ description: 'Base URL for the Wikibase REST API' }), | ||
| userAgent: t.String({ description: 'User agent string for API requests' }), | ||
| authToken: t.Optional(t.String({ description: 'Authentication token (optional)' })), | ||
| isDefault: t.Optional(t.Boolean({ description: 'Whether this is the default instance' })), | ||
| metadata: t.Optional(t.Object({ | ||
| description: t.Optional(t.String()), | ||
| language: t.Optional(t.String()), | ||
| version: t.Optional(t.String()), | ||
| })), | ||
| }), | ||
| detail: { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen input validation (URL format and non-empty constraints).
The schema allows empty strings and arbitrary baseUrl. Enforce stronger constraints to catch issues early at the API boundary.
- Ensure required strings are non-empty:
minLength: 1 - Validate
baseUrlas a proper URL (and optionally constrain to HTTP/S) - Optionally restrict
idto a safe pattern (e.g.,^[a-z0-9-]+$)
Example:
- id: t.String({ description: 'Unique instance identifier' }),
- name: t.String({ description: 'Human-readable instance name' }),
- baseUrl: t.String({ description: 'Base URL for the Wikibase REST API' }),
- userAgent: t.String({ description: 'User agent string for API requests' }),
+ id: t.String({ description: 'Unique instance identifier', minLength: 1, pattern: '^[a-z0-9-]+$' }),
+ name: t.String({ description: 'Human-readable instance name', minLength: 1 }),
+ baseUrl: t.String({ description: 'Base URL for the Wikibase REST API', format: 'uri' }),
+ userAgent: t.String({ description: 'User agent string for API requests', minLength: 1 }),Apply similar constraints to the /validate body schema as well.
Also applies to: 212-224
🤖 Prompt for AI Agents
In backend/src/api/wikibase/instances.ts around lines 76 to 89 (and similarly
update the /validate body schema at lines ~212-224), the current t.String()
fields allow empty strings and baseUrl is not validated; update the schema to
require non-empty strings (add minLength: 1) for id, name, userAgent and
metadata description/language/version, validate baseUrl as a proper URL
(restrict to http/https) using a URL pattern or a dedicated URL type, and
tighten id with a safe regex like ^[a-z0-9-]+$ (or equivalent pattern) to
prevent invalid identifiers; apply the same constraints to the duplicate
/validate schema.
| @@ -0,0 +1,601 @@ | |||
| import type { WikibaseInstanceConfig } from '@backend/types/wikibase-api' | |||
| import { ApiClient } from '@wmde/wikibase-rest-api' | |||
There was a problem hiding this comment.
Fix missing type declarations for '@wmde/wikibase-rest-api' (TS7016)
Typecheck fails because the package lacks .d.ts. Add a minimal ambient declaration to unblock CI.
Create a new file: types/wmde__wikibase-rest-api.d.ts
declare module '@wmde/wikibase-rest-api' {
export class ApiClient {
basePath: string
defaultHeaders?: Record<string, string>
constructor(basePath: string)
}
}Then include "types" in tsconfig.json if not already:
{
"compilerOptions": {
"typeRoots": ["./types", "./node_modules/@types"]
}
}
I can generate the tsconfig and declaration file changes if you want.
🧰 Tools
🪛 GitHub Actions: Typecheck code
[error] 2-2: TypeScript error TS7016: Could not find a declaration file for module '@wmde/wikibase-rest-api'. '/home/runner/work/dataforge/dataforge/node_modules/@wmde/wikibase-rest-api/dist/index.js' implicitly has an 'any' type.
🤖 Prompt for AI Agents
In backend/src/services/wikibase-config.service.ts around line 2, the import
from '@wmde/wikibase-rest-api' fails typechecking because the package has no
TypeScript declarations; create a new ambient declaration file
types/wmde__wikibase-rest-api.d.ts containing a minimal module declaration that
exports the ApiClient class with basePath, optional defaultHeaders, and a
constructor accepting basePath, then update tsconfig.json to include
"typeRoots": ["./types", "./node_modules/@types"] (or add the new types folder
to the existing typeRoots) so the compiler picks up the declaration and CI
typechecks succeed.
| const PREDEFINED_INSTANCES: WikibaseInstanceConfig[] = [ | ||
| { | ||
| id: 'wikidata', | ||
| name: 'Wikidata', | ||
| baseUrl: 'https://www.wikidata.org/w/rest.php/wikibase/v1', | ||
| userAgent: 'DataForge/0.1 (https://github.com/dataforge/dataforge)', | ||
| isDefault: true, | ||
| metadata: { | ||
| description: 'The free knowledge base that anyone can edit', | ||
| language: 'en', | ||
| version: 'v0', | ||
| }, | ||
| }, | ||
| ] | ||
|
|
There was a problem hiding this comment.
Add missing predefined instance “wikimedia-commons” to satisfy tests and docs
Tests expect two predefined instances (wikidata and wikimedia-commons). Only wikidata is defined here, causing multiple test failures.
Apply this diff to add Wikimedia Commons:
const PREDEFINED_INSTANCES: WikibaseInstanceConfig[] = [
{
id: 'wikidata',
name: 'Wikidata',
- baseUrl: 'https://www.wikidata.org/w/rest.php/wikibase/v1',
+ baseUrl: 'https://www.wikidata.org/w/rest.php/wikibase/v1',
userAgent: 'DataForge/0.1 (https://github.com/dataforge/dataforge)',
isDefault: true,
metadata: {
description: 'The free knowledge base that anyone can edit',
language: 'en',
version: 'v0',
},
},
+ {
+ id: 'wikimedia-commons',
+ name: 'Wikimedia Commons',
+ baseUrl: 'https://commons.wikimedia.org/w/rest.php/wikibase/v1',
+ userAgent: 'DataForge/0.1 (https://github.com/dataforge/dataforge)',
+ isDefault: false,
+ metadata: {
+ description: 'Media repository of free-use images, sounds, and other media files',
+ language: 'en',
+ version: 'v0',
+ },
+ },
]📝 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.
| const PREDEFINED_INSTANCES: WikibaseInstanceConfig[] = [ | |
| { | |
| id: 'wikidata', | |
| name: 'Wikidata', | |
| baseUrl: 'https://www.wikidata.org/w/rest.php/wikibase/v1', | |
| userAgent: 'DataForge/0.1 (https://github.com/dataforge/dataforge)', | |
| isDefault: true, | |
| metadata: { | |
| description: 'The free knowledge base that anyone can edit', | |
| language: 'en', | |
| version: 'v0', | |
| }, | |
| }, | |
| ] | |
| const PREDEFINED_INSTANCES: WikibaseInstanceConfig[] = [ | |
| { | |
| id: 'wikidata', | |
| name: 'Wikidata', | |
| baseUrl: 'https://www.wikidata.org/w/rest.php/wikibase/v1', | |
| userAgent: 'DataForge/0.1 (https://github.com/dataforge/dataforge)', | |
| isDefault: true, | |
| metadata: { | |
| description: 'The free knowledge base that anyone can edit', | |
| language: 'en', | |
| version: 'v0', | |
| }, | |
| }, | |
| { | |
| id: 'wikimedia-commons', | |
| name: 'Wikimedia Commons', | |
| baseUrl: 'https://commons.wikimedia.org/w/rest.php/wikibase/v1', | |
| userAgent: 'DataForge/0.1 (https://github.com/dataforge/dataforge)', | |
| isDefault: false, | |
| metadata: { | |
| description: 'Media repository of free-use images, sounds, and other media files', | |
| language: 'en', | |
| version: 'v0', | |
| }, | |
| }, | |
| ] |
🤖 Prompt for AI Agents
In backend/src/services/wikibase-config.service.ts around lines 47 to 61 the
PREDEFINED_INSTANCES array only contains the 'wikidata' entry but tests and docs
expect a second predefined instance for 'wikimedia-commons'; add a second
WikibaseInstanceConfig object with id 'wikimedia-commons', name 'Wikimedia
Commons', baseUrl 'https://commons.wikimedia.org/w/rest.php/wikibase/v1', the
same userAgent string as wikidata, isDefault false, and metadata including a
brief description, language 'en', and version 'v0' so the array contains both
predefined instances.
| const data = await response.json() | ||
| expect(data.success).toBe(true) | ||
| expect(Array.isArray(data.data)).toBe(true) | ||
| expect(data.data.length).toBeGreaterThan(0) | ||
|
|
||
| // Should include pre-defined instances | ||
| expect(data.data.some((instance: any) => instance.id === 'wikidata')).toBe(true) | ||
| expect(data.data.some((instance: any) => instance.id === 'wikimedia-commons')).toBe(true) | ||
| }) |
There was a problem hiding this comment.
Fix TS18046: Response.json() returns unknown in Bun – cast or add a typed helper.
Bun’s type for Response.json() is unknown, so direct property access on data leads to TS18046 errors. Introduce a typed API envelope and cast JSON responses accordingly.
Apply this diff (adds types + helper and updates one callsite; repeat at all others similarly):
@@
-import { describe, test, expect, beforeEach } from 'bun:test'
+import { describe, test, expect, beforeEach } from 'bun:test'
+type ApiError = { code: string; message: string; details?: unknown[] }
+type ApiResponse<T> = { success: boolean; data: T; message?: string; errors?: ApiError[] }
+const parseJson = async <T>(res: Response): Promise<ApiResponse<T>> =>
+ res.json() as Promise<ApiResponse<T>>
@@
- const data = await response.json()
+ const data = await parseJson<any>(response)
expect(data.success).toBe(true)
expect(Array.isArray(data.data)).toBe(true)Then replace every const data = await response.json() with const data = await parseJson<any>(response) (or a stronger T where known). This will fix all TS18046 occurrences flagged in the pipeline.
Also applies to: 54-58, 69-74, 91-94, 116-121, 149-153, 184-186, 203-207, 233-235, 247-251, 269-273, 296-299, 313-317, 329-333, 347-353, 365-368, 383-385, 412-413, 426-429
🧰 Tools
🪛 GitHub Check: test
[failure] 40-40: error: expect(received).toBe(expected)
Expected: true
Received: false
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/api/wikibase/instances.test.ts:40:86)
🪛 GitHub Actions: Typecheck code
[error] 34-40: TypeScript error TS18046: 'data' is of type 'unknown'.
🤖 Prompt for AI Agents
In backend/tests/api/wikibase/instances.test.ts around lines 33 to 41, the test
calls response.json() which Bun types as unknown causing TS18046; add a typed
helper (e.g., parseJson<T>(response): Promise<T>) and a shared API envelope/type
for the response shape, then replace const data = await response.json() with
const data = await parseJson<any>(response) (or a stronger generic T where
known) and update imports; repeat the same replacement for the other listed call
sites so all Response.json() usages are cast via the typed helper to satisfy
TS18046.
| // Should include pre-defined instances | ||
| expect(data.data.some((instance: any) => instance.id === 'wikidata')).toBe(true) | ||
| expect(data.data.some((instance: any) => instance.id === 'wikimedia-commons')).toBe(true) | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Failing expectation: predefined 'wikimedia-commons' not found.
The test assumes a predefined instance with id wikimedia-commons, but CI shows it’s absent. Either:
- Add
wikimedia-commonsto the service’s predefined set, or - Relax the test to assert only
wikidata(or to assert that at least one predefined instance exists, and specificallywikidata).
If the canonical intent is to include wikimedia-commons, update the service; otherwise, update the test accordingly.
Proposed test relaxation:
- expect(data.data.some((instance: any) => instance.id === 'wikimedia-commons')).toBe(true)
+ // Optional: include wikimedia-commons if provided by the service
+ // Keeping this non-blocking to avoid false negatives if not bundled
+ // expect(data.data.some((instance: any) => instance.id === 'wikimedia-commons')).toBe(true)[suggest_optional_refactor]
To confirm what the service actually ships, run:
🏁 Script executed:
#!/bin/bash
# Verify whether 'wikimedia-commons' is defined among predefined instances
rg -n -C2 -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' -P "wikimedia-commons|wikidata" backend/src/servicesLength of output: 742
Update test to remove expectation for ‘wikimedia-commons’
The service only ships the wikidata instance, so the test should no longer assert on wikimedia-commons, which is causing the CI failure.
• File: backend/tests/api/wikibase/instances.test.ts
• Remove or comment out the second expectation
Proposed change:
- expect(data.data.some((instance: any) => instance.id === 'wikimedia-commons')).toBe(true)
+ // Removed: service does not currently include a predefined 'wikimedia-commons' instance
+ // expect(data.data.some((instance: any) => instance.id === 'wikimedia-commons')).toBe(true)📝 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.
| // Should include pre-defined instances | |
| expect(data.data.some((instance: any) => instance.id === 'wikidata')).toBe(true) | |
| expect(data.data.some((instance: any) => instance.id === 'wikimedia-commons')).toBe(true) | |
| }) | |
| // Should include pre-defined instances | |
| expect(data.data.some((instance: any) => instance.id === 'wikidata')).toBe(true) | |
| // Removed: service does not currently include a predefined 'wikimedia-commons' instance | |
| // expect(data.data.some((instance: any) => instance.id === 'wikimedia-commons')).toBe(true) | |
| }) |
🧰 Tools
🪛 GitHub Check: test
[failure] 40-40: error: expect(received).toBe(expected)
Expected: true
Received: false
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/api/wikibase/instances.test.ts:40:86)
🤖 Prompt for AI Agents
In backend/tests/api/wikibase/instances.test.ts around lines 38 to 41, the test
asserts that both 'wikidata' and 'wikimedia-commons' are present but the service
only ships 'wikidata'; remove (or comment out) the second expectation that
checks for 'wikimedia-commons' so the test only asserts the presence of
'wikidata' and update any test description if needed to reflect the reduced set
of pre-defined instances.
| const mockOpenApiSpec = { | ||
| paths: { | ||
| '/entities/items': {}, | ||
| '/entities/properties': {}, | ||
| '/search/entities': {}, | ||
| '/statements': {}, | ||
| '/labels': {}, | ||
| '/descriptions': {}, | ||
| '/aliases': {}, | ||
| }, | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix unused variable causing TS6133 error
mockOpenApiSpec is declared but unused, tripping the typecheck. Remove it (or actually use it via injection/mocking if intended).
Apply this diff to remove it:
- const mockOpenApiSpec = {
- paths: {
- '/entities/items': {},
- '/entities/properties': {},
- '/search/entities': {},
- '/statements': {},
- '/labels': {},
- '/descriptions': {},
- '/aliases': {},
- },
- }📝 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.
| const mockOpenApiSpec = { | |
| paths: { | |
| '/entities/items': {}, | |
| '/entities/properties': {}, | |
| '/search/entities': {}, | |
| '/statements': {}, | |
| '/labels': {}, | |
| '/descriptions': {}, | |
| '/aliases': {}, | |
| }, | |
| } |
🧰 Tools
🪛 GitHub Actions: Typecheck code
[error] 196-196: TypeScript error TS6133: 'mockOpenApiSpec' is declared but its value is never read.
🤖 Prompt for AI Agents
In backend/tests/services/wikibase-config-connectivity.test.ts around lines 196
to 207, the local const mockOpenApiSpec is declared but never used which causes
a TS6133 unused variable error; either remove the entire declaration block or
use it by injecting/mocking the OpenAPI spec where the test expects it (e.g.,
pass it into the function under test or into the mocked HTTP/OpenAPI client),
and then run typecheck to ensure the error is resolved.
| expect(instance.id).toBe('wikidata') | ||
| expect(instance.name).toBe('Wikidata') | ||
| expect(instance.baseUrl).toBe('https://www.wikidata.org/w/rest.php/wikibase/v0') | ||
| }) |
There was a problem hiding this comment.
Align expected Wikidata baseUrl with service (v1 vs v0)
The service uses /wikibase/v1. The test expects /v0, causing failure. Align the assertion to v1.
- expect(instance.baseUrl).toBe('https://www.wikidata.org/w/rest.php/wikibase/v0')
+ expect(instance.baseUrl).toBe('https://www.wikidata.org/w/rest.php/wikibase/v1')📝 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(instance.id).toBe('wikidata') | |
| expect(instance.name).toBe('Wikidata') | |
| expect(instance.baseUrl).toBe('https://www.wikidata.org/w/rest.php/wikibase/v0') | |
| }) | |
| expect(instance.id).toBe('wikidata') | |
| expect(instance.name).toBe('Wikidata') | |
| expect(instance.baseUrl).toBe('https://www.wikidata.org/w/rest.php/wikibase/v1') | |
| }) |
🧰 Tools
🪛 GitHub Check: test
[failure] 57-57: error: expect(received).toBe(expected)
Expected: "https://www.wikidata.org/w/rest.php/wikibase/v0"
Received: "https://www.wikidata.org/w/rest.php/wikibase/v1"
at <anonymous> (/home/runner/work/dataforge/dataforge/backend/tests/services/wikibase-config.service.test.ts:57:32)
🤖 Prompt for AI Agents
In backend/tests/services/wikibase-config.service.test.ts around lines 55 to 58,
the test asserts the Wikidata baseUrl with /wikibase/v0 but the service uses
/wikibase/v1; update the expectation to match the service by changing the
asserted baseUrl to use /wikibase/v1 so the test aligns with the implementation.
| test('should return instances sorted by default status and name', async () => { | ||
| await service.addInstance(mockCustomConfig) | ||
| const instances = await service.getInstances() | ||
|
|
||
| // Default instance should be first | ||
| expect(instances[0].isDefault).toBe(true) | ||
|
|
||
| // Non-default instances should be sorted by name | ||
| const nonDefaultInstances = instances.filter(i => !i.isDefault) | ||
| for (let i = 1; i < nonDefaultInstances.length; i++) { | ||
| expect(nonDefaultInstances[i].name >= nonDefaultInstances[i - 1].name).toBe(true) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent TS2532 (“possibly undefined”) and make ordering assertion explicit
Indexing instances[0] without a guard triggers TS2532. Add a length check and use a non-null assertion for clarity.
await service.addInstance(mockCustomConfig)
const instances = await service.getInstances()
// Default instance should be first
- expect(instances[0].isDefault).toBe(true)
+ expect(instances.length).toBeGreaterThan(0)
+ const defaultInstance = instances[0]!
+ expect(defaultInstance.isDefault).toBe(true)📝 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.
| test('should return instances sorted by default status and name', async () => { | |
| await service.addInstance(mockCustomConfig) | |
| const instances = await service.getInstances() | |
| // Default instance should be first | |
| expect(instances[0].isDefault).toBe(true) | |
| // Non-default instances should be sorted by name | |
| const nonDefaultInstances = instances.filter(i => !i.isDefault) | |
| for (let i = 1; i < nonDefaultInstances.length; i++) { | |
| expect(nonDefaultInstances[i].name >= nonDefaultInstances[i - 1].name).toBe(true) | |
| } | |
| }) | |
| test('should return instances sorted by default status and name', async () => { | |
| await service.addInstance(mockCustomConfig) | |
| const instances = await service.getInstances() | |
| // Default instance should be first | |
| expect(instances.length).toBeGreaterThan(0) | |
| const defaultInstance = instances[0]! | |
| expect(defaultInstance.isDefault).toBe(true) | |
| // Non-default instances should be sorted by name | |
| const nonDefaultInstances = instances.filter(i => !i.isDefault) | |
| for (let i = 1; i < nonDefaultInstances.length; i++) { | |
| expect(nonDefaultInstances[i].name >= nonDefaultInstances[i - 1].name).toBe(true) | |
| } | |
| }) |
🧰 Tools
🪛 GitHub Actions: Typecheck code
[error] 71-71: TypeScript error TS2532: Object is possibly 'undefined'.
[error] 76-76: TypeScript error TS2532: Object is possibly 'undefined'.
🤖 Prompt for AI Agents
In backend/tests/services/wikibase-config.service.test.ts around lines 66 to 78,
the test indexes instances[0] which can trigger TS2532; add an explicit guard
that instances.length > 0 (or fail the test early with an assertion) before
asserting the default, and when indexing use a non-null assertion for TypeScript
(e.g. instances[0]!) to satisfy the compiler; also make the ordering assertion
explicit by asserting the nonDefaultInstances array has the expected length and
comparing names with a clear comparison (e.g. use localeCompare or an explicit
>= check) inside the loop.
| const customInstances = await service.getCustomInstances() | ||
| expect(customInstances.length).toBe(1) | ||
| expect(customInstances[0].id).toBe('custom-test') | ||
| }) |
There was a problem hiding this comment.
Avoid TS2532 on array indexing by destructuring with non-null assertion
TypeScript doesn’t narrow after expect(). Use destructuring and a non-null assertion.
- const customInstances = await service.getCustomInstances()
- expect(customInstances.length).toBe(1)
- expect(customInstances[0].id).toBe('custom-test')
+ const customInstances = await service.getCustomInstances()
+ expect(customInstances.length).toBe(1)
+ const [first] = customInstances
+ expect(first!.id).toBe('custom-test')📝 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.
| const customInstances = await service.getCustomInstances() | |
| expect(customInstances.length).toBe(1) | |
| expect(customInstances[0].id).toBe('custom-test') | |
| }) | |
| const customInstances = await service.getCustomInstances() | |
| expect(customInstances.length).toBe(1) | |
| const [first] = customInstances | |
| expect(first!.id).toBe('custom-test') |
🧰 Tools
🪛 GitHub Actions: Typecheck code
[error] 90-90: TypeScript error TS2532: Object is possibly 'undefined'.
🤖 Prompt for AI Agents
In backend/tests/services/wikibase-config.service.test.ts around lines 88 to 91,
the test indexes customInstances[0] after an expect which can trigger TS2532
because TypeScript won’t narrow after expect(); destructure the array and use a
non-null assertion on the element when checking its id: replace direct indexing
with const [first] = customInstances; then assert
expect(first!).toBeDefined()/or rely on the length check and use
expect(first!.id).toBe('custom-test') to avoid the TS2532 error.
1 participant
Addresses Task 4 of #137