Conversation
🦋 Changeset detectedLatest commit: 6672b0c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
- Add "AI" before "agent readiness" in changeset and docs for clarity - Replace <pre> block with fenced code block in score.md - Add security scheme coverage to metrics documentation - Remove resolveIfRef helper, replace with resolveNode that falls back to the original node when resolution fails - Refactor to use walkDocument visitor approach (matching stats command pattern) instead of manually iterating the document tree - Use resolveDocument + normalizeVisitors + walkDocument from openapi-core for proper $ref resolution and spec-format extensibility - Update index.test.ts to mock the new walk infrastructure Made-with: Cursor
adamaltman
force-pushed
the
aa/api-score
branch
from
79078d6 to
0e328aa
Compare
Co-authored-by: Jacek Łękawa <164185257+JLekawa@users.noreply.github.com>
tatomyr
left a comment
tatomyr
left a comment
There was a problem hiding this comment.
Left a couple of comments. I haven'd fully reviewed the scoring and collectors though as it takes time.
packages/cli/src/commands/score/__tests__/document-metrics.test.ts
Outdated
Show resolved
Hide resolved
tatomyr
left a comment
tatomyr
left a comment
There was a problem hiding this comment.
I found some dead code. Please check if it's needed and remove if not. Also, I'm not sure what tests to review as many appear to only test that dead code, so let's handle that first.
packages/cli/src/commands/score/__tests__/document-metrics.test.ts
Outdated
Show resolved
Hide resolved
- Inline collect-metrics.ts test helper into document-metrics.test.ts - Use parseYaml as yaml directly instead of wrapper function - Remove default parameter from getStylishOutput in formatter tests - Use getMajorSpecVersion + exitWithError for spec version check - Add explicit case 'stylish' before default in format switch - Remove unsupported 'markdown' from score command format choices - Add comment explaining depth=-1 initialization - Clarify anyOf penalty and dependency terminology in docs - Update non-oas3 rejection test for exitWithError (throws) Made-with: Cursor
- Add type cast for parseYaml (returns unknown) in document-metrics tests - Inline collectDocumentMetrics helper into example-coverage tests - Remove collect-metrics.js import from scoring tests, use direct metrics - Add missing debugLogs property to accumulator mock in index tests Made-with: Cursor
packages/cli/src/commands/score/__tests__/document-metrics.test.ts
Outdated
Show resolved
Hide resolved
Extract the metric-collection pipeline from handleScore into a standalone collect-metrics.ts module with two exports: - collectMetrics(): low-level function used by handleScore - collectDocumentMetrics(): high-level convenience used by tests This eliminates ~300 lines of duplicated walker setup across three test files (document-metrics, example-coverage, index) and ensures tests exercise the same code path as the production command. Add $ref-keyed memoization to walkSchema so repeated references to the same component schema return cached stats instead of re-walking. Stripe API: 37.6s → 11.3s (3.3× faster). Made-with: Cursor
Add median alongside averages for parameters, schema depth, polymorphism, and properties in the stylish output. Rename the misleading "Avg max schema depth" to "Schema depth". Made-with: Cursor
Rename workflowClarity to dependencyClarity, workflowDepths to dependencyDepths, and workflow-graph to dependency-graph to align code naming with the "Dependency Clarity" display label. Also adds discoverability subscore, recursive composition keyword stripping for accurate property counting, and updates e2e snapshots. Made-with: Cursor
- Use isPlainObject from openapi-core for schema cycle detection instead of raw typeof checks that match arrays - Remove formatter unit tests in favor of e2e snapshot coverage - Clarify makeScores() purpose in hotspot tests Made-with: Cursor
| metrics: makeDocumentMetrics(new Map([['listItems', makeTestMetrics()]])), | ||
| debugLogs: [], | ||
| }); | ||
| process.exitCode = undefined; |
There was a problem hiding this comment.
Seems redundant. At least, I don't see where we use exitCode in the tests.
- Add experimental admonition to score command docs - Remove --config from usage examples - Remove redundant mockClear and process.exitCode in tests - Simplify JSON formatter by using spread instead of manual field mapping - Use isRef from openapi-core for $ref detection in collect-metrics - Use isPlainObject and isNotEmptyObject from openapi-core in document-metrics - Replace != null with Array.isArray for discriminatorRefs check - Remove unnecessary non-null assertions on discriminatorRefs - Lower branch coverage threshold to 70% per reviewer guidance Made-with: Cursor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Discriminator flag lost with oneOf/anyOf polymorphism
- polyStats now sets hasDiscriminator when the parent schema has a discriminator (propertyName or non-empty mapping), merged with branch stats via OR.
- ✅ Fixed: Structured error count double-incremented per media type
- Added errorResponseStructuredCounted so structuredErrorResponseCount increases at most once per error response (description-only in Response.enter, or the first MediaType.enter that counts).
- ✅ Fixed: Independent max of correlated metrics across media types
- totalSchemaProperties, schemaPropertiesWithDescription, and constraintCount are now summed across media types instead of maxed independently.
- ✅ Fixed: Document parsed object mutated during metric collection
- collectMetrics now uses try/finally so restoreCompositionKeywords always runs after stripCompositionKeywords, even when inner work throws.
Or push these changes by commenting:
@cursor push b6568647da
Preview (b6568647da)
diff --git a/packages/cli/src/commands/score/collect-metrics.ts b/packages/cli/src/commands/score/collect-metrics.ts
--- a/packages/cli/src/commands/score/collect-metrics.ts
+++ b/packages/cli/src/commands/score/collect-metrics.ts
@@ -122,6 +122,30 @@
}: CollectMetricsOptions): CollectMetricsResult {
const removedComposition = stripCompositionKeywords(document.parsed);
+ try {
+ return collectMetricsInner({
+ document,
+ types,
+ resolvedRefMap,
+ ctx,
+ debugOperationId,
+ removedComposition,
+ });
+ } finally {
+ restoreCompositionKeywords(removedComposition);
+ }
+}
+
+function collectMetricsInner({
+ document,
+ types,
+ resolvedRefMap,
+ ctx,
+ debugOperationId,
+ removedComposition,
+}: CollectMetricsOptions & {
+ removedComposition: Map<object, StrippedComposition>;
+}): CollectMetricsResult {
const schemaWalkState = createSchemaWalkState();
const schemaVisitor = createSchemaMetricVisitor(schemaWalkState);
const normalizedSchemaVisitors = normalizeVisitors(
@@ -236,6 +260,10 @@
: null;
const hasDiscriminatorBranches =
Array.isArray(discriminatorRefs) && discriminatorRefs.length > 0;
+ const hasParentDiscriminator = !!(
+ disc?.propertyName ||
+ (isPlainObject(disc?.mapping) && Object.keys(disc.mapping).length > 0)
+ );
let result: SchemaStats;
@@ -258,6 +286,7 @@
...maxBranch,
polymorphismCount: maxBranch.polymorphismCount + polyBranches.length,
anyOfCount: maxBranch.anyOfCount + (polyKeyword === 'anyOf' ? polyBranches.length : 0),
+ hasDiscriminator: maxBranch.hasDiscriminator || hasParentDiscriminator,
};
}
@@ -323,8 +352,6 @@
ctx,
});
- restoreCompositionKeywords(removedComposition);
-
return {
metrics: getDocumentMetrics(accumulator),
debugLogs: accumulator.debugLogs,
diff --git a/packages/cli/src/commands/score/collectors/document-metrics.ts b/packages/cli/src/commands/score/collectors/document-metrics.ts
--- a/packages/cli/src/commands/score/collectors/document-metrics.ts
+++ b/packages/cli/src/commands/score/collectors/document-metrics.ts
@@ -212,6 +212,8 @@
inRequestBody: boolean;
inResponse: boolean;
currentResponseCode: string;
+ /** True once structured error counting ran for the current error response (Response or first MediaType). */
+ errorResponseStructuredCounted: boolean;
refsUsed: Set<string>;
}
@@ -278,6 +280,7 @@
inRequestBody: false,
inResponse: false,
currentResponseCode: '',
+ errorResponseStructuredCounted: false,
refsUsed: new Set(),
};
@@ -337,8 +340,10 @@
if (isErrorCode(code)) {
current.totalErrorResponses++;
+ current.errorResponseStructuredCounted = false;
if (!response.content && response.description) {
current.structuredErrorResponseCount++;
+ current.errorResponseStructuredCounted = true;
}
}
},
@@ -359,8 +364,13 @@
if (current.inResponse) current.responseExamplePresent = true;
}
- if (current.inResponse && isErrorCode(current.currentResponseCode)) {
+ if (
+ current.inResponse &&
+ isErrorCode(current.currentResponseCode) &&
+ !current.errorResponseStructuredCounted
+ ) {
current.structuredErrorResponseCount++;
+ current.errorResponseStructuredCounted = true;
}
if (mediaType.schema) {
@@ -372,15 +382,9 @@
const stats = accumulator.walkSchema(mediaType.schema, isDebugTarget);
current.propertyCount = Math.max(current.propertyCount, stats.propertyCount);
- current.totalSchemaProperties = Math.max(
- current.totalSchemaProperties,
- stats.totalSchemaProperties
- );
- current.schemaPropertiesWithDescription = Math.max(
- current.schemaPropertiesWithDescription,
- stats.schemaPropertiesWithDescription
- );
- current.constraintCount = Math.max(current.constraintCount, stats.constraintCount);
+ current.totalSchemaProperties += stats.totalSchemaProperties;
+ current.schemaPropertiesWithDescription += stats.schemaPropertiesWithDescription;
+ current.constraintCount += stats.constraintCount;
current.polymorphismCount = Math.max(current.polymorphismCount, stats.polymorphismCount);
current.anyOfCount = Math.max(current.anyOfCount, stats.anyOfCount);
if (stats.hasDiscriminator) current.hasDiscriminator = true;This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
- Fix duplicate import from @redocly/openapi-core (CI lint error) - Propagate hasDiscriminator flag for oneOf/anyOf + discriminator schemas - Prevent structured error response double-counting per media type - Keep totalSchemaProperties and schemaPropertiesWithDescription paired from the same schema to avoid misleading documentation quality ratios - Wrap composition stripping in try/finally to guarantee restoration - Export isMappingRef from openapi-core and use in collect-metrics - Rename bfsMaxDepth to computeLongestBfsPath for clarity Made-with: Cursor
- Add allOf member count to polymorphismCount for parity with oneOf/anyOf branch counting - Handle RFC 6901 JSON Pointer escaping (~0 → ~ and ~1 → /) in resolveJsonPointer - Remove unused scores parameter from getHotspotReasons Made-with: Cursor
tatomyr
left a comment
tatomyr
left a comment
There was a problem hiding this comment.
These metrics look like a duplication:
Is it intended?
| async function collect(doc: Record<string, unknown>) { | ||
| return (await collectDocumentMetrics(doc)).metrics; | ||
| } |
There was a problem hiding this comment.
This wrapper seems needless and doesn't add to the tests clarity.
| } | ||
|
|
||
| export function resetSchemaWalkState(s: SchemaWalkState): void { | ||
| s.depth = -1; // starts at -1 because the root Schema.enter increments to 0 |
There was a problem hiding this comment.
It seems that resetting the state is the same as createSchemaWalkState. Could it be reused to avoid creating several source of truth?
| const schemaWalkState = createSchemaWalkState(); | ||
| const schemaVisitor = createSchemaMetricVisitor(schemaWalkState); | ||
| const normalizedSchemaVisitors = normalizeVisitors( | ||
| [{ severity: 'warn', ruleId: 'score-schema', visitor: schemaVisitor as any }], |
There was a problem hiding this comment.
What is the type cast needed for? Is the visitor type wrong?
| return node; | ||
| } | ||
|
|
||
| interface SchemaStats { |
There was a problem hiding this comment.
This looks similar to SchemaStats in document-metrics.ts but with a wrong type import. Are they the same?
| if (!ref.startsWith('#/')) return undefined; | ||
| let node = root; | ||
| for (const segment of ref.slice(2).split('/')) { | ||
| const decoded = decodeURIComponent(segment).replace(/~1/g, '/').replace(/~0/g, '~'); |
There was a problem hiding this comment.
It's probably better to reuse unescapePointerFragment from ref-utils.ts.
| return merged; | ||
| } | ||
|
|
||
| function hasExample(mediaType: { example?: unknown; examples?: Record<string, unknown> }): boolean { |
There was a problem hiding this comment.
| function hasExample(mediaType: { example?: unknown; examples?: Record<string, unknown> }): boolean { | |
| function hasExample(mediaType: Oas3MediaType): boolean { |
| mediaType: { schema?: any; example?: unknown; examples?: Record<string, unknown> }, | ||
| _ctx: UserContext |
There was a problem hiding this comment.
| mediaType: { schema?: any; example?: unknown; examples?: Record<string, unknown> }, | |
| _ctx: UserContext | |
| mediaType |
There was a problem hiding this comment.
Looks like you're trying to put as much code as possible where you can perfectly get away with fewer 😄. It works correctly, but the codebase gets bloated and harder to maintain.
| maxDepth: schemaWalkState.maxDepth, | ||
| polymorphismCount: schemaWalkState.polymorphismCount, | ||
| anyOfCount: schemaWalkState.anyOfCount, | ||
| hasDiscriminator: schemaWalkState.hasDiscriminator, | ||
| propertyCount: schemaWalkState.propertyCount, | ||
| totalSchemaProperties: schemaWalkState.totalSchemaProperties, | ||
| schemaPropertiesWithDescription: schemaWalkState.schemaPropertiesWithDescription, | ||
| constraintCount: schemaWalkState.constraintCount, | ||
| hasPropertyExamples: schemaWalkState.hasPropertyExamples, | ||
| writableTopLevelFields: schemaWalkState.writableTopLevelFields, |
There was a problem hiding this comment.
| maxDepth: schemaWalkState.maxDepth, | |
| polymorphismCount: schemaWalkState.polymorphismCount, | |
| anyOfCount: schemaWalkState.anyOfCount, | |
| hasDiscriminator: schemaWalkState.hasDiscriminator, | |
| propertyCount: schemaWalkState.propertyCount, | |
| totalSchemaProperties: schemaWalkState.totalSchemaProperties, | |
| schemaPropertiesWithDescription: schemaWalkState.schemaPropertiesWithDescription, | |
| constraintCount: schemaWalkState.constraintCount, | |
| hasPropertyExamples: schemaWalkState.hasPropertyExamples, | |
| writableTopLevelFields: schemaWalkState.writableTopLevelFields, | |
| ...schemaWalkState |
Could it be simplified like so?
| }; | ||
| } | ||
|
|
||
| function buildOperationMetrics(ctx: CurrentOperationContext): OperationMetrics { |
There was a problem hiding this comment.
What is the difference between those two types? It looks like they are either the same or could be reused, so this function could be avoided or at least shortened.
| const seen = new WeakSet<object>(); | ||
|
|
||
| function walk(node: any): void { | ||
| if (!node || typeof node !== 'object' || seen.has(node)) return; |
There was a problem hiding this comment.
| if (!node || typeof node !== 'object' || seen.has(node)) return; | |
| if (!isPlainObject(node) || seen.has(node)) return; |
Was this the intention? Or you also want to traverse arrays?
Another question--could nodes be $refs? If yes, you have to resolve them. On the other hand, you already have walkSchemaRaw function which walks through schemas, maybe you can reuse it, but with a different visitor?
|
Yes, I think there is too much overlap between the two scores so I'm going to consolidate it into one. It will make docs easier, result understanding easier, etc... Addressing other comments too. |
… single score Merge the two overlapping composite scores into a single "Agent Readiness" score with all 9 subscores. This eliminates duplication in weights, scoring functions, and output. Also addresses code review feedback: use unescapePointerFragment, simplify resetSchemaWalkState via Object.assign, consolidate SchemaStats into types.ts, remove collect() test wrapper, simplify hasExample return, and use spread for walkSchemaRaw return. Made-with: Cursor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6672b0c. Configure here.
|
|
||
| walk(parsed); | ||
| return removed; | ||
| } |
There was a problem hiding this comment.
Nested oneOf/anyOf silently lost by aggressive stripping
Medium Severity
stripCompositionKeywords removes oneOf/anyOf from every schema in the document tree, but walkSchema only restores and handles composition for schemas it directly processes (top-level request/response schemas and their direct composition branches). Schemas nested within properties or items that originally had oneOf/anyOf are walked by walkSchemaRaw, which sees the stripped version — so their branches, property counts, depth, and polymorphism are silently invisible. This undercounts polymorphismCount, anyOfCount, propertyCount, and maxDepth for any API using nested polymorphism, producing artificially optimistic scores.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6672b0c. Configure here.
| maxParamsGood: 5, | ||
| maxDepthGood: 4, | ||
| maxPolymorphismGood: 2, | ||
| maxPropertiesGood: 20, |
There was a problem hiding this comment.
Threshold maxPropertiesGood defined but never used in scoring
Low Severity
maxPropertiesGood is declared in ScoringThresholds, set to 20 in DEFAULT_SCORING_CONSTANTS, but never referenced in any scoring computation in scoring.ts or elsewhere. This is dead code that may confuse future maintainers into thinking property count is factored into the score when it isn't.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6672b0c. Configure here.
| const sorted = [...values].sort((a, b) => a - b); | ||
| const mid = Math.floor(sorted.length / 2); | ||
| return sorted.length % 2 !== 0 ? sorted[mid] : (sorted[mid - 1] + sorted[mid]) / 2; | ||
| } |
There was a problem hiding this comment.
Duplicate median utility function across two files
Low Severity
An identical median function is defined as a private function in both scoring.ts and formatters/stylish.ts. This duplicated logic increases the risk of inconsistent bug fixes if the implementation ever needs to change.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6672b0c. Configure here.
4 participants
What/Why/How?
What: Adds a new score command to Redocly CLI that analyzes OpenAPI 3.x descriptions and produces two composite scores: Integration Simplicity (0-100, how easy is this API to integrate) and Agent Readiness (0-100, how usable is this API by AI agents/LLM tooling).
Why: API producers currently lack a quick, deterministic way to assess how developer-friendly or AI-agent-friendly their API descriptions are. The existing stats command counts structural elements but doesn't evaluate quality signals like documentation coverage, example presence, schema complexity, or error response structure. This command fills that gap with actionable, explainable scores.
How: The implementation follows the same pattern as the
statscommand (bundle+analyze), with a clean separation between metric collection and score calculation:collectors/): Walks the bundled document, resolving internal $refs, to gather per-operation raw metrics (parameter counts, schema depth, polymorphism, description/constraint/example coverage, structured error responses, workflow dependency depth via shared schema refs).scoring.ts): Pure functions that normalize raw metrics into subscores and compute weighted composite scores. Thresholds and weights are configurable constants. anyOf is penalized more heavily than oneOf/allOf; discriminator presence improves the agent readiness polymorphism clarity subscore.hotspots.ts): Identifies the operations with the most issues, sorted by number of reasons, with human-readable explanations.--format=stylish(default, with color bar charts) and--format=json(machine-readable for CI/dashboards).Reference
Related to API governance and developer experience tooling. No existing issue -- this is a new feature.
Testing
$refresolution, polymorphism counting (oneOf/anyOf/allOf), constraint detection (including const), example coverage scoring,anyOfpenalty multiplier,discriminatorimpact on agent readiness, deterministic output, and score range validation.tsc --noEmit), all existing tests continue to pass.Screenshots (optional)
Stylish output for Redocly Cafe:
Check yourself
Security
Note
Medium Risk
Adds a new CLI command with non-trivial OpenAPI walking/scoring logic and extends the core public API (
isMappingRefexport), which could affect consumers and introduces new analysis paths that may need tuning for edge-case specs.Overview
Adds a new experimental
redocly scorecommand that evaluates OpenAPI 3.x descriptions and outputs an Agent Readiness score (0–100) with normalized subscores, per-operation raw metrics, dependency-depth signals from shared$refs, and a ranked list of hotspot operations with human-readable reasons.Implements metric collection via OpenAPI document walking (including schema depth/polymorphism/constraints/examples/error-structure and ambiguous parameter detection), scoring/aggregation and discoverability weighting, plus
stylishandjsonformatters and an optional--debug-operation-idschema breakdown.Updates CLI docs/navigation, adds unit + e2e snapshot coverage for the new command, publishes a changeset, and exports
isMappingReffrom@redocly/openapi-core(plus a small coverage threshold adjustment).Reviewed by Cursor Bugbot for commit 6672b0c. Bugbot is set up for automated code reviews on this repo. Configure here.