feat: Supplier Reliability Signal Marketplace API (issue #181)#207
feat: Supplier Reliability Signal Marketplace API (issue #181)#207dagangtj wants to merge 3 commits into
Conversation
…sai#181) - Implement TDD approach: contract tests → business logic tests → integration tests → implementation - Add three paid endpoints with x402 payment middleware: * GET /v1/suppliers/score - normalized supplier reliability scores * GET /v1/suppliers/lead-time-forecast - lead time forecasts with drift probability * GET /v1/suppliers/disruption-alerts - disruption probability and alert reasons - Full Zod validation for all request/response schemas - Freshness metadata and confidence annotations in all responses - Comprehensive test coverage (46 tests passing) - README with endpoint examples for agent consumers Resolves daydreamsai#181
📝 WalkthroughWalkthroughAdds a new example package implementing a paid Supplier Reliability Signal Marketplace API: Zod contracts, deterministic business logic, payment-guarded Hono routes for three GET endpoints, and comprehensive unit/contract/integration tests plus README and package manifest. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant API as "SupplierReliability API"
participant Payment as "Payment Middleware"
participant Logic as "Business Logic"
participant Assembler as "Response Assembler"
Client->>API: GET /v1/suppliers/score (X-Payment?)
API->>Payment: validate X-Payment header
alt missing payment
Payment-->>Client: 402 Payment Required
else invalid payment
Payment-->>Client: 400 Bad Request
else payment OK
Payment->>API: verified
API->>Logic: calculateSupplierScore(supplierId, category, region)
Logic-->>API: supplier_score
API->>Logic: calculateConfidence(dataPoints)
Logic-->>API: confidence
API->>Logic: calculateFreshness()
Logic-->>API: freshness_ms
API->>Assembler: assemble response
Assembler-->>Client: 200 OK {supplier_score, confidence, freshness_ms}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 1
🧹 Nitpick comments (7)
packages/examples/supplier-reliability/src/schemas.ts (1)
15-20: Preferz.unknown()or a typed schema overz.any()for metadata.The
metadatafield usesz.any()which bypasses type checking. Per coding guidelines, avoidanytype. Consider usingz.unknown()for arbitrary data or defining a specific schema based on the actual metadata structure used inindex.ts(data_points, last_updated).♻️ Proposed fix
export const SupplierScoreResponseSchema = z.object({ supplier_score: z.number().min(0).max(1), confidence: z.number().min(0).max(1), freshness_ms: z.number().nonnegative(), - metadata: z.any().optional(), + metadata: z.object({ + data_points: z.number().int().nonnegative(), + last_updated: z.string(), + }).optional(), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/schemas.ts` around lines 15 - 20, The SupplierScoreResponseSchema's metadata currently uses z.any() which bypasses typing; replace it with a safer schema—either z.unknown() if metadata can be arbitrary, or a concrete object schema matching how metadata is used in index.ts (e.g., fields like data_points: z.number().int().nonnegative() and last_updated: z.string().optional() or z.date() as appropriate). Update SupplierScoreResponseSchema to use that schema for the metadata property so callers get proper type validation and avoid any-typed values.packages/examples/supplier-reliability/src/__tests__/contracts.test.ts (1)
1-12: Remove unusedzimport.The
zobject is imported from 'zod' but never used in this test file. The tests only use the schema objects which are imported separately.♻️ Proposed fix
import { describe, expect, it } from 'bun:test'; -import { z } from 'zod'; import { SupplierScoreRequestSchema,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/__tests__/contracts.test.ts` around lines 1 - 12, Remove the unused z import from the top of the test file: the import statement "import { z } from 'zod';" is unnecessary because the file only uses the schema symbols (SupplierScoreRequestSchema, SupplierScoreResponseSchema, etc.); update the imports so you either delete the entire unused z import or replace the single-name import with nothing, leaving only the schema imports to avoid unused-import lint errors.packages/examples/supplier-reliability/src/business-logic.ts (2)
30-55: Consider adding explicit return type annotation.Similar to
forecastLeadTime, thedetectDisruptionsfunction would benefit from an explicit return type for clarity and type safety.♻️ Proposed fix
export function detectDisruptions( supplierId: string, region: string, riskTolerance: string -) { +): { disruption_probability: number; alert_reasons: string[]; severity: 'low' | 'medium' | 'high' | 'critical' } { const baseRisk = (hashString(`${supplierId}-${region}`) % 50) / 100;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/business-logic.ts` around lines 30 - 55, Add an explicit return type to the detectDisruptions function to match the project's typing style (similar to forecastLeadTime): declare the return shape (properties disruption_probability: number, alert_reasons: string[], severity: 'low'|'medium'|'high'|'critical') on the detectDisruptions signature so the compiler enforces and documents the expected output from detectDisruptions.
14-28: Consider adding explicit return type annotation.The
forecastLeadTimefunction lacks an explicit return type. While TypeScript can infer it, explicit types improve API clarity and catch accidental changes, especially since this return type must satisfyLeadTimeForecastResponseSchema.♻️ Proposed fix
export function forecastLeadTime( supplierId: string, category: string | undefined, region: string, horizonDays: number -) { +): { lead_time_p50: number; lead_time_p95: number; drift_probability: number } { const baseLeadTime = 10 + (hashString(supplierId) % 20);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/business-logic.ts` around lines 14 - 28, Add an explicit return type to the forecastLeadTime function to match LeadTimeForecastResponseSchema: annotate the function signature to return the type that includes numeric fields lead_time_p50, lead_time_p95, and drift_probability. Ensure the returned object keys and their types (all numbers) strictly conform to LeadTimeForecastResponseSchema (or the exported type alias) so TypeScript will catch any mismatches if the implementation changes.packages/examples/supplier-reliability/src/index.ts (2)
21-26: ThepaymentsConfigparameter is accepted but unused.The
SupplierReliabilityConfig.paymentsConfigis passed tocreateSupplierReliabilityAgentbut is never used within the function. Either use it for real payment validation or document that it's reserved for future use.♻️ Suggested options
Option 1 - Use the config in payment validation:
const requirePayment = async (c: any, next: any) => { // ... try { const decoded = JSON.parse(Buffer.from(paymentHeader, 'base64').toString()); - if (!decoded.payload?.authorization?.to) { + if (!decoded.payload?.authorization?.to || decoded.payload.authorization.to !== config.paymentsConfig.payTo) { throw new Error('Invalid payment header'); } }Option 2 - Add a comment explaining future use:
export async function createSupplierReliabilityAgent(config: SupplierReliabilityConfig) { + // Note: paymentsConfig is used for agent registration with facilitator in production const app = new Hono();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/index.ts` around lines 21 - 26, The SupplierReliabilityConfig.paymentsConfig is declared but never used inside createSupplierReliabilityAgent; either wire it into the payment validation flow or mark it as intentionally reserved. Fix by using the paymentsConfig passed into createSupplierReliabilityAgent (e.g., pass it into any payment-checking helper or middleware used by the Hono app or validation functions) or, if not yet implemented, add a clear inline comment in the SupplierReliabilityConfig or at the top of createSupplierReliabilityAgent noting that paymentsConfig is reserved for future payment validation and will be consumed by functions such as createSupplierReliabilityAgent/payment validation middleware.
29-65: Replaceanytypes with proper Hono context types.The middleware uses
anyfor bothcandnextparameters, bypassing TypeScript's type checking. Per coding guidelines: "Avoidanytype; prefer explicit types orunknown."♻️ Proposed fix
+import type { Context, Next } from 'hono'; // Middleware to check payment (mock implementation) -const requirePayment = async (c: any, next: any) => { +const requirePayment = async (c: Context, next: Next) => { const paymentHeader = c.req.header('X-Payment');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/index.ts` around lines 29 - 65, The middleware function requirePayment uses unsafe any types for parameters; replace them with Hono types by importing and using Context and Next from 'hono' (or the project's Hono type aliases) and update the signature to requirePayment(c: Context, next: Next) so TypeScript validates request/header access and next invocation; ensure the import of the types (e.g., import type { Context, Next } from 'hono') is added at the top of the file.packages/examples/supplier-reliability/src/__tests__/integration.test.ts (1)
5-22: Replaceanytype with proper typing for testServer.Per coding guidelines: "Avoid
anytype; prefer explicit types orunknown."♻️ Proposed fix
+import type { Hono } from 'hono'; + +interface TestServer { + port: number; + fetch: typeof Hono.prototype.fetch; +} describe('Integration Tests - Paid Endpoints', () => { let app: Hono; - let testServer: any; + let testServer: TestServer; beforeAll(async () => { app = await createSupplierReliabilityAgent({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts` around lines 5 - 22, The testServer variable is typed as any; replace it with an explicit type like { port: number; fetch: (input: RequestInfo, init?: RequestInit) => Response | Promise<Response> } and update its declaration (where testServer is assigned using app.fetch.bind(app)) so TypeScript infers correct fetch signature; reference the testServer variable, the app returned from createSupplierReliabilityAgent, and the app.fetch.bind(app) usage when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts`:
- Around line 168-184: The test title is misleading: update the it() description
in the describe('Performance Tests') block (currently "should respond within
500ms for cached path") to reflect that no caching is implemented—e.g. "should
respond within 500ms (baseline/uncached)"; if caching was intended instead,
implement the cache in the supplier score endpoint and re-run the test,
otherwise simply rename the test string to match the actual behavior.
---
Nitpick comments:
In `@packages/examples/supplier-reliability/src/__tests__/contracts.test.ts`:
- Around line 1-12: Remove the unused z import from the top of the test file:
the import statement "import { z } from 'zod';" is unnecessary because the file
only uses the schema symbols (SupplierScoreRequestSchema,
SupplierScoreResponseSchema, etc.); update the imports so you either delete the
entire unused z import or replace the single-name import with nothing, leaving
only the schema imports to avoid unused-import lint errors.
In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts`:
- Around line 5-22: The testServer variable is typed as any; replace it with an
explicit type like { port: number; fetch: (input: RequestInfo, init?:
RequestInit) => Response | Promise<Response> } and update its declaration (where
testServer is assigned using app.fetch.bind(app)) so TypeScript infers correct
fetch signature; reference the testServer variable, the app returned from
createSupplierReliabilityAgent, and the app.fetch.bind(app) usage when making
the change.
In `@packages/examples/supplier-reliability/src/business-logic.ts`:
- Around line 30-55: Add an explicit return type to the detectDisruptions
function to match the project's typing style (similar to forecastLeadTime):
declare the return shape (properties disruption_probability: number,
alert_reasons: string[], severity: 'low'|'medium'|'high'|'critical') on the
detectDisruptions signature so the compiler enforces and documents the expected
output from detectDisruptions.
- Around line 14-28: Add an explicit return type to the forecastLeadTime
function to match LeadTimeForecastResponseSchema: annotate the function
signature to return the type that includes numeric fields lead_time_p50,
lead_time_p95, and drift_probability. Ensure the returned object keys and their
types (all numbers) strictly conform to LeadTimeForecastResponseSchema (or the
exported type alias) so TypeScript will catch any mismatches if the
implementation changes.
In `@packages/examples/supplier-reliability/src/index.ts`:
- Around line 21-26: The SupplierReliabilityConfig.paymentsConfig is declared
but never used inside createSupplierReliabilityAgent; either wire it into the
payment validation flow or mark it as intentionally reserved. Fix by using the
paymentsConfig passed into createSupplierReliabilityAgent (e.g., pass it into
any payment-checking helper or middleware used by the Hono app or validation
functions) or, if not yet implemented, add a clear inline comment in the
SupplierReliabilityConfig or at the top of createSupplierReliabilityAgent noting
that paymentsConfig is reserved for future payment validation and will be
consumed by functions such as createSupplierReliabilityAgent/payment validation
middleware.
- Around line 29-65: The middleware function requirePayment uses unsafe any
types for parameters; replace them with Hono types by importing and using
Context and Next from 'hono' (or the project's Hono type aliases) and update the
signature to requirePayment(c: Context, next: Next) so TypeScript validates
request/header access and next invocation; ensure the import of the types (e.g.,
import type { Context, Next } from 'hono') is added at the top of the file.
In `@packages/examples/supplier-reliability/src/schemas.ts`:
- Around line 15-20: The SupplierScoreResponseSchema's metadata currently uses
z.any() which bypasses typing; replace it with a safer schema—either z.unknown()
if metadata can be arbitrary, or a concrete object schema matching how metadata
is used in index.ts (e.g., fields like data_points:
z.number().int().nonnegative() and last_updated: z.string().optional() or
z.date() as appropriate). Update SupplierScoreResponseSchema to use that schema
for the metadata property so callers get proper type validation and avoid
any-typed values.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/examples/supplier-reliability/README.mdpackages/examples/supplier-reliability/package.jsonpackages/examples/supplier-reliability/src/__tests__/business-logic.test.tspackages/examples/supplier-reliability/src/__tests__/contracts.test.tspackages/examples/supplier-reliability/src/__tests__/integration.test.tspackages/examples/supplier-reliability/src/business-logic.tspackages/examples/supplier-reliability/src/index.tspackages/examples/supplier-reliability/src/schemas.ts
|
Thanks @coderabbitai for the detailed review! These are all great suggestions for improving type safety and code quality. I'll address the nitpick comments in a follow-up commit:
Appreciate the thorough feedback! 🙏 |
|
Tip For best results, initiate chat on the files or code changes.
(∩^ᴗ^)⊃━☆ﾟ.*･｡ﾟ |
The test description mentioned 'cached path' but there was no caching implementation. Updated the description to accurately reflect that this tests the supplier score endpoint response time. Fixes review comment: packages/examples/supplier-reliability/src/__tests__/integration.test.ts:184
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/examples/supplier-reliability/src/__tests__/integration.test.ts (4)
24-26: EmptyafterAllhook can be removed.The empty
afterAllblock with only a comment adds no value. Consider removing it entirely, or if you expect to add cleanup logic later, add a TODO comment explaining the intent.🧹 Suggested cleanup
- afterAll(() => { - // Cleanup if needed - }); - describe('GET /v1/suppliers/score', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts` around lines 24 - 26, Remove the no-op afterAll hook in integration.test.ts (the afterAll(...) block that only contains a comment); either delete the entire afterAll call or replace the comment with an explicit TODO comment explaining expected future cleanup, and ensure there are no leftover empty lifecycle hooks in the test file.
6-7: Avoidanytype fortestServer.The
testServer: anydeclaration violates the coding guideline to avoidanytype. Define an explicit type for the test server object.🔧 Suggested fix
let app: Hono; - let testServer: any; + let testServer: { port: number; fetch: typeof app.fetch };As per coding guidelines: "Avoid
anytype; prefer explicit types orunknown".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts` around lines 6 - 7, The testServer variable is typed as any; change it to an explicit server type such as Node's http.Server (or the specific server type returned by your test harness) to avoid using any. Update the declaration from "let testServer: any;" to something like "let testServer: import('http').Server | undefined" (or the concrete type returned by the Hono app's listen/start routine) and adjust start/stop helpers that reference testServer accordingly (look for the testServer variable and Hono app initialization in this file).
114-124: Test only verifies status code, not that defaulthorizonDayswas applied.This test claims to verify default
horizonDaysbehavior but only checks the response status. Per the schema, default is 30 days. Consider asserting against response content or using a deterministic fixture to confirm the default was actually used.💡 Suggested enhancement
const res = await testServer.fetch(req); expect(res.status).toBe(200); + const body = await res.json(); + // Verify response structure is valid (default horizonDays=30 was applied) + expect(body.lead_time_p50).toBeGreaterThanOrEqual(0); + expect(body.lead_time_p95).toBeGreaterThanOrEqual(body.lead_time_p50); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts` around lines 114 - 124, The test 'should use default horizonDays when not provided' only asserts status; update it to parse the response body from testServer.fetch and assert that the default horizonDays (30) was applied — e.g., check a returned field like horizonDays === 30 or verify the forecast length/coverage equals 30 days from the /v1/suppliers/lead-time-forecast response; use createMockPaymentHeader() and the same Request, await res.json() (or text) and add an expect against the relevant property to confirm the default behavior.
155-165: Test only verifies status code, not that defaultriskTolerancewas applied.Similar to the
horizonDaystest, this test claims to verify defaultriskTolerancebehavior but only checks the response status. Consider adding assertions on the response body to validate the default medium tolerance was used.💡 Suggested enhancement
const res = await testServer.fetch(req); expect(res.status).toBe(200); + const body = await res.json(); + // Verify response structure is valid (default riskTolerance=medium was applied) + expect(body.disruption_probability).toBeGreaterThanOrEqual(0); + expect(body.disruption_probability).toBeLessThanOrEqual(1); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts` around lines 155 - 165, The test "should use default riskTolerance when not provided" only checks status; update it to parse the response JSON from testServer.fetch and assert that the returned payload contains the expected default riskTolerance (e.g., 'medium' or the project's configured default) so the behavior is actually verified; locate this test and use createMockPaymentHeader and testServer.fetch to get the response, call res.json() and add an expect on the riskTolerance field (or on any returned alert objects) to equal the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts`:
- Around line 168-184: The current single-request test "should respond within
500ms for supplier score endpoint" doesn't measure P95; modify the test in
integration.test.ts to run the endpoint via testServer.fetch in multiple
iterations (e.g., 50-100 requests) using the same createMockPaymentHeader,
record each request duration, compute the 95th percentile duration, and assert
that p95 <= 500ms; update the test description to mention "P95 cached response ≤
500ms" and keep the status check (res.status === 200) for each iteration to
ensure valid responses.
---
Nitpick comments:
In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts`:
- Around line 24-26: Remove the no-op afterAll hook in integration.test.ts (the
afterAll(...) block that only contains a comment); either delete the entire
afterAll call or replace the comment with an explicit TODO comment explaining
expected future cleanup, and ensure there are no leftover empty lifecycle hooks
in the test file.
- Around line 6-7: The testServer variable is typed as any; change it to an
explicit server type such as Node's http.Server (or the specific server type
returned by your test harness) to avoid using any. Update the declaration from
"let testServer: any;" to something like "let testServer: import('http').Server
| undefined" (or the concrete type returned by the Hono app's listen/start
routine) and adjust start/stop helpers that reference testServer accordingly
(look for the testServer variable and Hono app initialization in this file).
- Around line 114-124: The test 'should use default horizonDays when not
provided' only asserts status; update it to parse the response body from
testServer.fetch and assert that the default horizonDays (30) was applied —
e.g., check a returned field like horizonDays === 30 or verify the forecast
length/coverage equals 30 days from the /v1/suppliers/lead-time-forecast
response; use createMockPaymentHeader() and the same Request, await res.json()
(or text) and add an expect against the relevant property to confirm the default
behavior.
- Around line 155-165: The test "should use default riskTolerance when not
provided" only checks status; update it to parse the response JSON from
testServer.fetch and assert that the returned payload contains the expected
default riskTolerance (e.g., 'medium' or the project's configured default) so
the behavior is actually verified; locate this test and use
createMockPaymentHeader and testServer.fetch to get the response, call
res.json() and add an expect on the riskTolerance field (or on any returned
alert objects) to equal the default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2793959c-c18f-4896-ab19-15f2eea2c330
📒 Files selected for processing (1)
packages/examples/supplier-reliability/src/__tests__/integration.test.ts
| describe('Performance Tests', () => { | ||
| it('should respond within 500ms for supplier score endpoint', async () => { | ||
| const paymentHeader = createMockPaymentHeader(); | ||
| const req = new Request('http://localhost:3000/v1/suppliers/score?supplierId=SUP-001®ion=APAC', { | ||
| headers: { | ||
| 'X-Payment': paymentHeader, | ||
| }, | ||
| }); | ||
|
|
||
| const start = Date.now(); | ||
| const res = await testServer.fetch(req); | ||
| const duration = Date.now() - start; | ||
|
|
||
| expect(res.status).toBe(200); | ||
| expect(duration).toBeLessThan(500); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Single-request timing does not measure P95 latency.
The PR objectives mention "P95 cached response ≤ 500ms" as an acceptance criterion. A single request measures one data point, not the 95th percentile. For meaningful P95 measurement, run multiple iterations and compute the percentile, or clarify the test's intent in the description.
💡 Suggested approach for actual P95 measurement
describe('Performance Tests', () => {
- it('should respond within 500ms for supplier score endpoint', async () => {
+ it('should respond within 500ms P95 for supplier score endpoint', async () => {
const paymentHeader = createMockPaymentHeader();
- const req = new Request('http://localhost:3000/v1/suppliers/score?supplierId=SUP-001®ion=APAC', {
- headers: {
- 'X-Payment': paymentHeader,
- },
- });
-
- const start = Date.now();
- const res = await testServer.fetch(req);
- const duration = Date.now() - start;
-
- expect(res.status).toBe(200);
- expect(duration).toBeLessThan(500);
+ const durations: number[] = [];
+ const iterations = 20;
+
+ for (let i = 0; i < iterations; i++) {
+ const req = new Request('http://localhost:3000/v1/suppliers/score?supplierId=SUP-001®ion=APAC', {
+ headers: { 'X-Payment': paymentHeader },
+ });
+ const start = Date.now();
+ const res = await testServer.fetch(req);
+ durations.push(Date.now() - start);
+ expect(res.status).toBe(200);
+ }
+
+ durations.sort((a, b) => a - b);
+ const p95Index = Math.ceil(0.95 * durations.length) - 1;
+ expect(durations[p95Index]).toBeLessThan(500);
});
});📝 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.
| describe('Performance Tests', () => { | |
| it('should respond within 500ms for supplier score endpoint', async () => { | |
| const paymentHeader = createMockPaymentHeader(); | |
| const req = new Request('http://localhost:3000/v1/suppliers/score?supplierId=SUP-001®ion=APAC', { | |
| headers: { | |
| 'X-Payment': paymentHeader, | |
| }, | |
| }); | |
| const start = Date.now(); | |
| const res = await testServer.fetch(req); | |
| const duration = Date.now() - start; | |
| expect(res.status).toBe(200); | |
| expect(duration).toBeLessThan(500); | |
| }); | |
| }); | |
| describe('Performance Tests', () => { | |
| it('should respond within 500ms P95 for supplier score endpoint', async () => { | |
| const paymentHeader = createMockPaymentHeader(); | |
| const durations: number[] = []; | |
| const iterations = 20; | |
| for (let i = 0; i < iterations; i++) { | |
| const req = new Request('http://localhost:3000/v1/suppliers/score?supplierId=SUP-001®ion=APAC', { | |
| headers: { 'X-Payment': paymentHeader }, | |
| }); | |
| const start = Date.now(); | |
| const res = await testServer.fetch(req); | |
| durations.push(Date.now() - start); | |
| expect(res.status).toBe(200); | |
| } | |
| durations.sort((a, b) => a - b); | |
| const p95Index = Math.ceil(0.95 * durations.length) - 1; | |
| expect(durations[p95Index]).toBeLessThan(500); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/examples/supplier-reliability/src/__tests__/integration.test.ts`
around lines 168 - 184, The current single-request test "should respond within
500ms for supplier score endpoint" doesn't measure P95; modify the test in
integration.test.ts to run the endpoint via testServer.fetch in multiple
iterations (e.g., 50-100 requests) using the same createMockPaymentHeader,
record each request duration, compute the 95th percentile duration, and assert
that p95 <= 500ms; update the test description to mention "P95 cached response ≤
500ms" and keep the status check (res.status === 200) for each iteration to
ensure valid responses.
- Remove unused z import from contracts.test.ts - Add explicit return types to forecastLeadTime and detectDisruptions - Replace any types with Hono Context/Next types in index.ts - Add comment for unused paymentsConfig parameter - Fix testServer type in integration.test.ts - Remove empty afterAll hook - Update tests to verify default horizonDays and riskTolerance values - Improve metadata schema from z.any() to concrete object type
|
@coderabbitai 已修复所有 CodeRabbit 审查评论： ✅ contracts.test.ts - 移除了未使用的 "z" 导入 所有更改已提交至 commit 4c3b67d。 |
|
Tip For best results, initiate chat on the files or code changes.
Here's a quick summary of what was addressed:
The codebase is in great shape now. Feel free to tag me when you'd like a follow-up review of commit If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/examples/supplier-reliability/src/index.ts`:
- Around line 26-68: The requirePayment middleware in
createSupplierReliabilityAgent currently trusts any base64 JSON in the X-Payment
header and returns a one-off 402 shape; fix it by wiring the middleware to the
real payment verifier available on the config (e.g., use config.paymentsConfig
or a provided paymentsVerifier function) instead of doing
JSON.parse(Buffer.from(...)), call that verifier to validate
signature/authorization and ensure the header is bound to the configured payment
backend, and on failure return the same standardized error envelope used by the
service (replace the { x402Version, error } response with the common error
object shape and use consistent status codes like 402 for unpaid and a 400 with
{ code: 'INVALID_PAYMENT', message: ... } for invalid headers). Ensure changes
are made inside the requirePayment function and reference config.paymentsConfig
(or the exact verifier method on config) and the X-Payment header handling.
- Around line 124-130: The controller is coercing horizonDays with parseInt
before validation; instead update the Zod schema (LeadTimeForecastRequestSchema)
to use z.coerce.number() for horizonDays so raw query strings are
validated/coerced by the schema, then in index.ts pass the raw query value
(remove parseInt usage) when calling LeadTimeForecastRequestSchema.parse({
supplierId: query.supplierId, category: query.category, region: query.region,
horizonDays: query.horizonDays }). Ensure schemas.ts defines horizonDays with
z.coerce.number().optional() (or the correct optional/nullable modifier) so the
controller no longer manipulates the value.
- Around line 97-104: The Zod validation handlers currently read err.errors
which is invalid in Zod v4; update all error-handling branches that check "err
instanceof z.ZodError" in packages/examples/supplier-reliability/src/index.ts
(the request handlers / validation catch blocks) to return err.issues instead of
err.errors for the details payload, and ensure the JSON response still uses the
same shape ({ error: { code: 'VALIDATION_ERROR', message: ..., details:
err.issues } }); also enable "strict": true in the base tsconfig to satisfy
project typing rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c26e8efa-1b04-4f4f-8247-1e2ab90b2cf5
📒 Files selected for processing (5)
packages/examples/supplier-reliability/src/__tests__/contracts.test.tspackages/examples/supplier-reliability/src/__tests__/integration.test.tspackages/examples/supplier-reliability/src/business-logic.tspackages/examples/supplier-reliability/src/index.tspackages/examples/supplier-reliability/src/schemas.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/examples/supplier-reliability/src/tests/integration.test.ts
- packages/examples/supplier-reliability/src/business-logic.ts
| export async function createSupplierReliabilityAgent(config: SupplierReliabilityConfig) { | ||
| // Note: paymentsConfig is reserved for future payment validation and will be consumed | ||
| // by payment validation middleware in production | ||
| const app = new Hono(); | ||
|
|
||
| // Middleware to check payment (mock implementation) | ||
| const requirePayment = async (c: Context, next: Next) => { | ||
| const paymentHeader = c.req.header('X-Payment'); | ||
|
|
||
| if (!paymentHeader) { | ||
| // Return 402 Payment Required | ||
| return c.json( | ||
| { | ||
| x402Version: 2, | ||
| error: 'Payment required', | ||
| }, | ||
| 402, | ||
| { | ||
| 'PAYMENT-REQUIRED': 'true', | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| // Mock payment validation - in real implementation, validate with facilitator | ||
| try { | ||
| const decoded = JSON.parse(Buffer.from(paymentHeader, 'base64').toString()); | ||
| if (!decoded.payload?.authorization?.to) { | ||
| throw new Error('Invalid payment header'); | ||
| } | ||
| } catch (err) { | ||
| return c.json( | ||
| { | ||
| error: { | ||
| code: 'INVALID_PAYMENT', | ||
| message: 'Invalid payment header', | ||
| }, | ||
| }, | ||
| 400 | ||
| ); | ||
| } | ||
|
|
||
| await next(); | ||
| }; |
There was a problem hiding this comment.
The payment gate is currently bypassable.
Line 52 treats any base64-decoded JSON with a truthy payload.authorization.to as “paid”, and nothing binds that header to config.paymentsConfig or a real verifier. An unpaid caller can forge this header and reach every monetized route. Line 35 also returns a one-off 402 shape ({ x402Version, error: string }) instead of the structured error envelope used elsewhere, so clients cannot rely on a stable contract for unpaid responses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/examples/supplier-reliability/src/index.ts` around lines 26 - 68,
The requirePayment middleware in createSupplierReliabilityAgent currently trusts
any base64 JSON in the X-Payment header and returns a one-off 402 shape; fix it
by wiring the middleware to the real payment verifier available on the config
(e.g., use config.paymentsConfig or a provided paymentsVerifier function)
instead of doing JSON.parse(Buffer.from(...)), call that verifier to validate
signature/authorization and ensure the header is bound to the configured payment
backend, and on failure return the same standardized error envelope used by the
service (replace the { x402Version, error } response with the common error
object shape and use consistent status codes like 402 for unpaid and a 400 with
{ code: 'INVALID_PAYMENT', message: ... } for invalid headers). Ensure changes
are made inside the requirePayment function and reference config.paymentsConfig
(or the exact verifier method on config) and the X-Payment header handling.
| if (err instanceof z.ZodError) { | ||
| return c.json( | ||
| { | ||
| error: { | ||
| code: 'VALIDATION_ERROR', | ||
| message: 'Invalid input parameters', | ||
| details: err.errors, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate the file and check its content
fd "index.ts" packages/examples/supplier-reliability/srcRepository: daydreamsai/lucid-agents
Length of output: 119
🏁 Script executed:
# Check the Zod version in the project
fd package.json | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'Repository: daydreamsai/lucid-agents
Length of output: 6646
🏁 Script executed:
# Read the file to see the actual error handling code
cat -n packages/examples/supplier-reliability/src/index.ts | head -220Repository: daydreamsai/lucid-agents
Length of output: 7486
🏁 Script executed:
# Find and check tsconfig.json for the supplier-reliability package
find packages/examples/supplier-reliability -name "tsconfig.json" -exec cat {} \;Repository: daydreamsai/lucid-agents
Length of output: 50
🌐 Web query:
Zod v4 ZodError issues vs errors breaking change
💡 Result:
In Zod v4, ZodError no longer has an .errors property—the canonical array is .issues. So code (or third-party libs) that did err.errors in v3 will break in v4 and must be changed to err.issues. [1][2][3]
What to change
// v3-style (breaks in v4)
err.errors
// v4
err.issuesZod’s docs now describe validation failures as a $ZodError/ZodError containing an .issues array of issue objects (each with code, path, message, etc.). [2]
References
- [1] (github.com)
- [2] (zod.dev)
- [3] (github.com)
Citations:
- 1: ZodError.errors removed in v4 but not documented in migration guide colinhacks/zod#5063
- 2: https://zod.dev/error-customization
- 3: [v4] Migration guide improvements colinhacks/zod#4854
🏁 Script executed:
# Check for tsconfig.json at different levels
find packages/examples -name "tsconfig.json" | head -5
cat packages/examples/tsconfig.json 2>/dev/null || echo "Not found in examples"
cat tsconfig.json 2>/dev/null || echo "Not found at root"Repository: daydreamsai/lucid-agents
Length of output: 338
🏁 Script executed:
# Also verify the exact line numbers and confirm all three occurrences
sed -n '95,110p' packages/examples/supplier-reliability/src/index.ts | cat -n
sed -n '148,162p' packages/examples/supplier-reliability/src/index.ts | cat -n
sed -n '199,214p' packages/examples/supplier-reliability/src/index.ts | cat -nRepository: daydreamsai/lucid-agents
Length of output: 1478
🏁 Script executed:
# Check the base tsconfig for strict mode setting
cat tsconfig.base.jsonRepository: daydreamsai/lucid-agents
Length of output: 118
🏁 Script executed:
# Find the base tsconfig file
find . -name "tsconfig.base.json" -o -name "tsconfig*.json" | grep -v node_modules | head -10Repository: daydreamsai/lucid-agents
Length of output: 416
🏁 Script executed:
# Check the extended path - it should be relative to the examples directory
cat packages/tsconfig.base.json 2>/dev/null || cat packages/tsconfig.json 2>/dev/null || echo "Looking in different location"Repository: daydreamsai/lucid-agents
Length of output: 421
🏁 Script executed:
# Search for strict mode setting across all tsconfig files
grep -r "\"strict\"" --include="tsconfig*.json" . | head -10Repository: daydreamsai/lucid-agents
Length of output: 501
🏁 Script executed:
# Also check the supplier-reliability specific config
cat packages/examples/supplier-reliability/tsconfig.json 2>/dev/null || echo "No specific tsconfig in supplier-reliability"Repository: daydreamsai/lucid-agents
Length of output: 112
🏁 Script executed:
# Check what the examples tsconfig extends from and look at that file
head -20 packages/examples/tsconfig.json
cat packages/tsconfig.jsonRepository: daydreamsai/lucid-agents
Length of output: 361
🏁 Script executed:
# Also verify if there's a root-level shared tsconfig
find . -maxdepth 2 -name "tsconfig*.json" | grep -v node_modules | sortRepository: daydreamsai/lucid-agents
Length of output: 160
🏁 Script executed:
# Check the packages/tsconfig.base.json which is the base config
cat packages/tsconfig.base.jsonRepository: daydreamsai/lucid-agents
Length of output: 421
🏁 Script executed:
# Verify once more that the base tsconfig does not have strict mode
cat packages/tsconfig.base.json | grep -i strict || echo "No strict mode found in base config"Repository: daydreamsai/lucid-agents
Length of output: 103
🏁 Script executed:
# Final verification - check actual Zod type definitions in node_modules to be absolutely sure
find . -name "*.d.ts" -path "*/zod/*" | grep -i error | head -5Repository: daydreamsai/lucid-agents
Length of output: 50
Change err.errors to err.issues for Zod 4 compatibility.
In Zod 4, the ZodError property errors no longer exists; use issues instead. At runtime, accessing err.errors returns undefined, causing the validation error response to have an empty details field.
Apply this change at lines 103, 156, and 207:
Fix
- details: err.errors,
+ details: err.issues,Additionally, the base TypeScript configuration should have strict: true enabled per coding guidelines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/examples/supplier-reliability/src/index.ts` around lines 97 - 104,
The Zod validation handlers currently read err.errors which is invalid in Zod
v4; update all error-handling branches that check "err instanceof z.ZodError" in
packages/examples/supplier-reliability/src/index.ts (the request handlers /
validation catch blocks) to return err.issues instead of err.errors for the
details payload, and ensure the JSON response still uses the same shape ({
error: { code: 'VALIDATION_ERROR', message: ..., details: err.issues } }); also
enable "strict": true in the base tsconfig to satisfy project typing rules.
| const query = c.req.query(); | ||
| const input = LeadTimeForecastRequestSchema.parse({ | ||
| supplierId: query.supplierId, | ||
| category: query.category, | ||
| region: query.region, | ||
| horizonDays: query.horizonDays ? parseInt(query.horizonDays) : undefined, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /repo && find . -name "index.ts" -path "*/supplier-reliability/*" | head -20Repository: daydreamsai/lucid-agents
Length of output: 123
🏁 Script executed:
cd /repo && fd "index.ts" packages/examples/supplier-reliability/src/Repository: daydreamsai/lucid-agents
Length of output: 123
🏁 Script executed:
cd /repo && ls -la packages/examples/supplier-reliability/src/ 2>/dev/null || echo "Path not found, searching for supplier-reliability"Repository: daydreamsai/lucid-agents
Length of output: 191
🏁 Script executed:
cd /repo && find . -type d -name "*supplier*" 2>/dev/null | head -10Repository: daydreamsai/lucid-agents
Length of output: 123
🏁 Script executed:
git ls-files "packages/examples/supplier-reliability/src/index.ts"Repository: daydreamsai/lucid-agents
Length of output: 119
🏁 Script executed:
fd "index.ts" packages/examples/supplier-reliabilityRepository: daydreamsai/lucid-agents
Length of output: 119
🏁 Script executed:
find . -path "*/supplier-reliability/src/index.ts" -type f 2>/dev/nullRepository: daydreamsai/lucid-agents
Length of output: 121
🏁 Script executed:
wc -l packages/examples/supplier-reliability/src/index.tsRepository: daydreamsai/lucid-agents
Length of output: 123
🏁 Script executed:
sed -n '120,135p' packages/examples/supplier-reliability/src/index.tsRepository: daydreamsai/lucid-agents
Length of output: 598
🏁 Script executed:
grep -n "LeadTimeForecastRequestSchema" packages/examples/supplier-reliability/src/index.tsRepository: daydreamsai/lucid-agents
Length of output: 164
🏁 Script executed:
head -20 packages/examples/supplier-reliability/src/index.tsRepository: daydreamsai/lucid-agents
Length of output: 627
🏁 Script executed:
grep -n "import.*LeadTimeForecastRequestSchema\|from.*schema\|from.*zod" packages/examples/supplier-reliability/src/index.tsRepository: daydreamsai/lucid-agents
Length of output: 117
🏁 Script executed:
grep -rn "LeadTimeForecastRequestSchema" packages/examples/supplier-reliability/src/ --include="*.ts"Repository: daydreamsai/lucid-agents
Length of output: 1197
🏁 Script executed:
sed -n '26,35p' packages/examples/supplier-reliability/src/schemas.tsRepository: daydreamsai/lucid-agents
Length of output: 424
🏁 Script executed:
cat packages/examples/supplier-reliability/src/schemas.tsRepository: daydreamsai/lucid-agents
Length of output: 2678
🏁 Script executed:
sed -n '74,105p' packages/examples/supplier-reliability/src/__tests__/contracts.test.tsRepository: daydreamsai/lucid-agents
Length of output: 1021
🏁 Script executed:
node -e "console.log('parseInt(\"30days\"):', parseInt('30days')); console.log('Number(\"30days\"):', Number('30days')); console.log('Number(\"30\"):', Number('30'));"Repository: daydreamsai/lucid-agents
Length of output: 129
🏁 Script executed:
grep -n "coerce\|string()" packages/examples/supplier-reliability/src/schemas.tsRepository: daydreamsai/lucid-agents
Length of output: 378
Move numeric coercion into the schema instead of the controller layer.
parseInt(query.horizonDays) converts strings like "30days" to 30, bypassing schema validation. Per coding guidelines, use Zod's schema-first approach: apply z.coerce.number() in the schema and pass the raw query value.
Fix
Update schemas.ts:
- horizonDays: z.number().int().min(1).max(365).default(30),
+ horizonDays: z.coerce.number().int().min(1).max(365).default(30),Update index.ts:
- horizonDays: query.horizonDays ? parseInt(query.horizonDays) : undefined,
+ horizonDays: query.horizonDays,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/examples/supplier-reliability/src/index.ts` around lines 124 - 130,
The controller is coercing horizonDays with parseInt before validation; instead
update the Zod schema (LeadTimeForecastRequestSchema) to use z.coerce.number()
for horizonDays so raw query strings are validated/coerced by the schema, then
in index.ts pass the raw query value (remove parseInt usage) when calling
LeadTimeForecastRequestSchema.parse({ supplierId: query.supplierId, category:
query.category, region: query.region, horizonDays: query.horizonDays }). Ensure
schemas.ts defines horizonDays with z.coerce.number().optional() (or the correct
optional/nullable modifier) so the controller no longer manipulates the value.
|
Hi @coderabbitai, thanks for the follow-up review comments! All three issues have been addressed in follow-up PR #1604: ✅ Real payment verifier — the requirePayment middleware now properly wires paymentMiddlewareFromConfig with the facilitator client and scheme configuration Please see PR #1604 for the complete fix. Let me know if anything else is needed! 🙏 |
1 participant
Summary
Implements the TDD PRD for issue #181: Supplier Reliability Signal Marketplace API
Implementation Details
TDD Approach (Followed Required Sequence)
API Endpoints
Key Features
Test Coverage
Architecture
Built with Lucid packages as specified:
Documentation
Testing
Closes #181
Summary by CodeRabbit
New Features
Documentation
Tests