From 936f552496f494199c6ae5ad54920cf7cda9f162 Mon Sep 17 00:00:00 2001 From: Lukas Klingsbo Date: Fri, 7 Aug 2026 11:16:14 +0200 Subject: [PATCH 1/2] feat(compliance): split symbol evidence from symbol coverage The `symbols` list was serving two checks that want opposite things. The drift check reads it as evidence a capability exists, so it wants a short list of entry points. The new-symbol check reads it as an exhaustive account of the public surface, so it wants every option type, result type, schema model and exception to appear somewhere. One list cannot do both. When a capability's supporting types outnumber its methods, authors are pushed into padding `symbols` with types that do not implement anything, or into repeating one shared list across several features so each has something to point at. Both inflate what the matrix claims is implemented, fan drift warnings out across features that do not own the symbol, and leave symbol-to-feature attribution arbitrary, since the index silently last-wins on collision. Adds an optional `supporting_symbols` list, per feature and top level for types shared across features. It counts for new-symbol coverage and is never drift-verified, so `symbols` can stay precise. The field is optional and the drift check already reads only `symbols`, so existing compliance files are unaffected. --- docs/capability-matrix.md | 34 +++- scripts/capability-matrix/src/api-check.ts | 18 ++- scripts/capability-matrix/src/compliance.ts | 74 +++++++-- scripts/capability-matrix/src/types.ts | 7 + .../capability-matrix/test/compliance.test.ts | 145 +++++++++++++++++- 5 files changed, 259 insertions(+), 19 deletions(-) diff --git a/docs/capability-matrix.md b/docs/capability-matrix.md index f43d3d5..a48e09c 100644 --- a/docs/capability-matrix.md +++ b/docs/capability-matrix.md @@ -24,7 +24,35 @@ features: Valid status values: `implemented`, `partially_implemented`, `not_implemented`, `not_applicable`. -The optional `symbols` field lists the public API symbol(s) that implement a feature. CI uses it two ways: +### Registering symbols -- **New-symbol check** (blocking): a PR that adds a new public symbol not listed under any feature's `symbols` fails, prompting the author to register it. -- **Drift check** (non-blocking warning): every feature marked `implemented` with a `symbols` list is periodically re-verified against the SDK's actual public API. If a registered symbol can no longer be found (renamed, removed) — or if an `implemented` feature has no `symbols` registered at all to verify against — CI posts a warning so the entry can be corrected. +CI asks two different questions about an SDK's public API, and each one reads a different field. + +- **Is this capability really implemented?** The **drift check** (non-blocking warning) re-verifies every feature marked `implemented` against the SDK's actual public API. If a registered symbol can no longer be found (renamed, removed), or if an `implemented` feature has no symbols registered at all to verify against, CI posts a warning so the entry can be corrected. This reads `symbols`. +- **Is every public symbol accounted for?** The **new-symbol check** (blocking) fails a PR that adds a public symbol the compliance file does not mention anywhere, prompting the author to register it. This reads `symbols` and `supporting_symbols` alike. + +`symbols` is therefore for **entry points**: the methods a user calls to exercise the capability. Keep the list short and precise, because every name in it is a claim that the feature exists. + +`supporting_symbols` is for the rest of the public surface that hangs off those entry points: option types, result types, schema models, exceptions. These need to be accounted for so the new-symbol check stays meaningful, but they do not evidence a capability and are never drift-verified. + +```yaml +storage.analytics.create_table: + status: implemented + symbols: + - IcebergRestCatalog.createTable + supporting_symbols: + - CreateTableRequest + - CreateTableRequest.schema +``` + +Types shared across several features can be listed once in a top-level `supporting_symbols` list instead of being attributed to whichever feature happens to use them: + +```yaml +sdk: flutter +features: {} +supporting_symbols: + - IcebergException + - TableMetadata +``` + +Do not pad `symbols` with supporting types to satisfy the new-symbol check, and do not repeat one shared list across several features to give each of them something to point at. Both inflate what the matrix claims is implemented, make drift warnings fan out across features that do not own the symbol, and leave the symbol-to-feature attribution arbitrary. diff --git a/scripts/capability-matrix/src/api-check.ts b/scripts/capability-matrix/src/api-check.ts index 495941c..b995c1c 100644 --- a/scripts/capability-matrix/src/api-check.ts +++ b/scripts/capability-matrix/src/api-check.ts @@ -44,6 +44,7 @@ export function formatErrorMessage( lines.push(` defined at: ${location}`); } } + const example = uncoveredSymbols[0]?.name ?? "ClassName.methodName"; lines.push( "", "Register each symbol in sdk-compliance.yaml under the appropriate feature:", @@ -51,7 +52,22 @@ export function formatErrorMessage( " auth.my_feature:", " status: implemented", " symbols:", - ` - ${uncoveredSymbols[0]?.name ?? "ClassName.methodName"}`, + ` - ${example}`, + "", + "Use `symbols` only for the entry points that implement the capability, since", + "the drift check treats them as evidence that it exists. Option types, result", + "types, schema models and errors belong under `supporting_symbols`, which", + "counts for coverage without claiming to implement anything:", + "", + " auth.my_feature:", + " status: implemented", + " symbols:", + " - GoTrueClient.myFeature", + " supporting_symbols:", + ` - ${example}`, + "", + "Types shared across several features can go in the top-level", + "`supporting_symbols` list instead of being attributed to one of them.", "", "If the feature does not exist in the matrix yet, add it there first:", " https://github.com/supabase/sdk/blob/main/CONTRIBUTING.md", diff --git a/scripts/capability-matrix/src/compliance.ts b/scripts/capability-matrix/src/compliance.ts index 18a0bbf..da6b7b9 100644 --- a/scripts/capability-matrix/src/compliance.ts +++ b/scripts/capability-matrix/src/compliance.ts @@ -6,11 +6,33 @@ export interface ComplianceFinding { message: string; } -type RawValue = string | { status?: string; note?: string; symbols?: string[] }; +type RawValue = + | string + | { status?: string; note?: string; symbols?: string[]; supporting_symbols?: string[] }; export interface RawCompliance { sdk: string; features: Record; + supporting_symbols?: string[]; +} + +// Pseudo feature ID reported for symbols registered in the top-level +// supporting_symbols list, which belong to no single capability. +export const TOP_LEVEL_SUPPORTING = "supporting_symbols"; + +function checkSymbolList( + value: unknown, + label: string, + context: string, + findings: ComplianceFinding[] +): void { + if (value === undefined) return; + if (!Array.isArray(value) || value.some((s) => typeof s !== "string")) { + findings.push({ + level: "error", + message: `${context}: ${label} must be an array of strings`, + }); + } } export function validateCompliance( @@ -23,6 +45,8 @@ export function validateCompliance( findings.push({ level: "error", message: `unknown sdk "${raw.sdk}"` }); } + checkSymbolList(raw.supporting_symbols, "supporting_symbols", "top level", findings); + for (const [id, value] of Object.entries(raw.features ?? {})) { if (!knownIds.has(id)) { findings.push({ level: "error", message: `unknown feature id "${id}"` }); @@ -53,12 +77,9 @@ export function validateCompliance( findings.push({ level: "error", message: `"${id}": partially_implemented requires a note` }); } - if (typeof value === "object" && value !== null && "symbols" in value && value.symbols !== undefined) { - if (!Array.isArray(value.symbols)) { - findings.push({ level: "error", message: `"${id}": symbols must be an array of strings` }); - } else if (value.symbols.some((s) => typeof s !== "string")) { - findings.push({ level: "error", message: `"${id}": symbols must be an array of strings` }); - } + if (typeof value === "object" && value !== null) { + checkSymbolList(value.symbols, "symbols", `"${id}"`, findings); + checkSymbolList(value.supporting_symbols, "supporting_symbols", `"${id}"`, findings); } } @@ -75,6 +96,9 @@ export function normalizeCompliance(raw: RawCompliance): ComplianceMap { status: (value.status ?? "not_implemented") as Status, ...(value.note !== undefined ? { note: value.note } : {}), ...(value.symbols !== undefined ? { symbols: value.symbols } : {}), + ...(value.supporting_symbols !== undefined + ? { supporting_symbols: value.supporting_symbols } + : {}), }; } } @@ -96,16 +120,38 @@ export function findMissingFeatureIds(raw: RawCompliance, knownIds: Set) return [...knownIds].filter((id) => !declared.has(id)).sort(); } -// Returns a map from SDK symbol name → capability matrix feature ID. -// Built from the symbols arrays declared in sdk-compliance.yaml. +// Returns a map from SDK symbol name → capability matrix feature ID, covering +// every symbol the compliance file accounts for: entry points (`symbols`) and +// the supporting public API around them (`supporting_symbols`, per feature or +// top level). Used by the new-symbol check, which only asks whether a symbol is +// accounted for at all. Entry points are indexed last so that a symbol listed +// both ways is attributed to the capability it implements rather than to the +// supporting bucket. export function buildSymbolIndex(raw: RawCompliance): Map { const index = new Map(); - for (const [featureId, value] of Object.entries(raw.features ?? {})) { - if (typeof value === "object" && value !== null && Array.isArray(value.symbols)) { - for (const sym of value.symbols) { - if (typeof sym === "string") index.set(sym, featureId); - } + + for (const sym of raw.supporting_symbols ?? []) { + if (typeof sym === "string") index.set(sym, TOP_LEVEL_SUPPORTING); + } + + const entries = Object.entries(raw.features ?? {}).filter( + (entry): entry is [string, Exclude] => + typeof entry[1] === "object" && entry[1] !== null + ); + + for (const [featureId, value] of entries) { + if (!Array.isArray(value.supporting_symbols)) continue; + for (const sym of value.supporting_symbols) { + if (typeof sym === "string") index.set(sym, featureId); + } + } + + for (const [featureId, value] of entries) { + if (!Array.isArray(value.symbols)) continue; + for (const sym of value.symbols) { + if (typeof sym === "string") index.set(sym, featureId); } } + return index; } diff --git a/scripts/capability-matrix/src/types.ts b/scripts/capability-matrix/src/types.ts index 864e98f..67b5b0b 100644 --- a/scripts/capability-matrix/src/types.ts +++ b/scripts/capability-matrix/src/types.ts @@ -58,7 +58,14 @@ export interface Finding { export interface ComplianceEntry { status: Status; note?: string; + /** Entry points that implement the feature. Verified by the drift check. */ symbols?: string[]; + /** + * Public API that belongs to the feature's surface but does not by itself + * evidence the capability: option and result types, schema models, errors. + * Counts for new-symbol coverage only, never for drift verification. + */ + supporting_symbols?: string[]; } // Feature ID → ComplianceEntry (sparse; unlisted features default to not_implemented) diff --git a/scripts/capability-matrix/test/compliance.test.ts b/scripts/capability-matrix/test/compliance.test.ts index f0cbc59..2a8d8e6 100644 --- a/scripts/capability-matrix/test/compliance.test.ts +++ b/scripts/capability-matrix/test/compliance.test.ts @@ -1,5 +1,8 @@ import { describe, it, expect } from "vitest"; -import { validateCompliance, normalizeCompliance, collectFeatureIds, buildSymbolIndex, findMissingFeatureIds } from "../src/compliance"; +import { validateCompliance, normalizeCompliance, collectFeatureIds, buildSymbolIndex, findMissingFeatureIds, TOP_LEVEL_SUPPORTING } from "../src/compliance"; +import { checkDrift } from "../src/drift-check"; +import { checkNewSymbols } from "../src/api-check"; +import type { ParsedSymbol } from "../src/normalize-typedoc"; import type { LoadedArea } from "../src/types"; function areas(ids: string[]): LoadedArea[] { @@ -16,6 +19,10 @@ function areas(ids: string[]): LoadedArea[] { ]; } +function sym(name: string): ParsedSymbol { + return { name, kind: "method", file: "src/index.ts" }; +} + const knownIds = new Set(["auth.sign_up", "auth.sign_in_with_password", "auth.mfa_enroll"]); describe("validateCompliance", () => { @@ -145,6 +152,112 @@ describe("symbols field", () => { }); }); +describe("supporting_symbols field", () => { + it("accepts per-feature and top-level supporting_symbols", () => { + const raw = { + sdk: "javascript", + features: { + "auth.sign_up": { + status: "implemented", + symbols: ["GoTrueClient.signUp"], + supporting_symbols: ["SignUpOptions", "SignUpOptions.redirectTo"], + }, + }, + supporting_symbols: ["AuthException"], + }; + expect(validateCompliance(raw, knownIds)).toEqual([]); + }); + + it("errors when per-feature supporting_symbols is not an array of strings", () => { + const raw = { + sdk: "javascript", + // eslint-disable-next-line @typescript-eslint/no-explicit-any + features: { "auth.sign_up": { status: "implemented", supporting_symbols: [42] } as any }, + }; + const findings = validateCompliance(raw, knownIds); + expect( + findings.some((f) => f.message === '"auth.sign_up": supporting_symbols must be an array of strings') + ).toBe(true); + }); + + it("errors when top-level supporting_symbols is not an array of strings", () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const raw = { sdk: "javascript", features: {}, supporting_symbols: "AuthException" } as any; + const findings = validateCompliance(raw, knownIds); + expect( + findings.some((f) => f.message === "top level: supporting_symbols must be an array of strings") + ).toBe(true); + }); + + it("normalizeCompliance preserves supporting_symbols", () => { + const raw = { + sdk: "javascript", + features: { + "auth.sign_up": { status: "implemented", supporting_symbols: ["SignUpOptions"] }, + }, + }; + expect(normalizeCompliance(raw)["auth.sign_up"].supporting_symbols).toEqual(["SignUpOptions"]); + }); + + it("omits supporting_symbols from the normalized entry when absent", () => { + const raw = { + sdk: "javascript", + features: { "auth.sign_up": { status: "implemented", symbols: ["GoTrueClient.signUp"] } }, + }; + expect(normalizeCompliance(raw)["auth.sign_up"]).toEqual({ + status: "implemented", + symbols: ["GoTrueClient.signUp"], + }); + }); + + it("is not treated as implementation evidence by the drift check", () => { + const compliance = { + sdk: "javascript", + features: { + "auth.sign_up": { + status: "implemented", + symbols: ["GoTrueClient.signUp"], + supporting_symbols: ["SignUpOptions"], + }, + }, + }; + // Only the entry point exists in the parsed API; the supporting type is gone. + const findings = checkDrift([sym("GoTrueClient.signUp")], compliance); + expect(findings).toEqual([]); + }); + + it("does not let supporting_symbols alone satisfy the drift check", () => { + const compliance = { + sdk: "javascript", + features: { + "auth.sign_up": { status: "implemented", supporting_symbols: ["SignUpOptions"] }, + }, + }; + const findings = checkDrift([sym("SignUpOptions")], compliance); + expect(findings).toEqual([{ featureId: "auth.sign_up" }]); + }); + + it("counts toward new-symbol coverage", () => { + const compliance = { + sdk: "javascript", + features: { + "auth.sign_up": { + status: "implemented", + symbols: ["GoTrueClient.signUp"], + supporting_symbols: ["SignUpOptions"], + }, + }, + supporting_symbols: ["AuthException"], + }; + const result = checkNewSymbols( + [], + [sym("SignUpOptions"), sym("AuthException"), sym("Unregistered")], + compliance + ); + expect(result.uncoveredSymbols.map((s) => s.name)).toEqual(["Unregistered"]); + }); +}); + describe("buildSymbolIndex", () => { it("builds a symbol → feature-id map", () => { const raw = { @@ -167,6 +280,36 @@ describe("buildSymbolIndex", () => { const index = buildSymbolIndex(raw); expect(index.size).toBe(0); }); + + it("indexes supporting symbols against their feature", () => { + const raw = { + sdk: "javascript", + features: { + "auth.sign_up": { status: "implemented", supporting_symbols: ["SignUpOptions"] }, + }, + }; + expect(buildSymbolIndex(raw).get("SignUpOptions")).toBe("auth.sign_up"); + }); + + it("indexes top-level supporting symbols against the pseudo feature id", () => { + const raw = { sdk: "javascript", features: {}, supporting_symbols: ["AuthException"] }; + expect(buildSymbolIndex(raw).get("AuthException")).toBe(TOP_LEVEL_SUPPORTING); + }); + + it("attributes a symbol to the capability that implements it over any supporting list", () => { + const raw = { + sdk: "javascript", + features: { + "auth.sign_up": { status: "implemented", symbols: ["GoTrueClient.signUp"] }, + "auth.mfa_enroll": { + status: "implemented", + supporting_symbols: ["GoTrueClient.signUp"], + }, + }, + supporting_symbols: ["GoTrueClient.signUp"], + }; + expect(buildSymbolIndex(raw).get("GoTrueClient.signUp")).toBe("auth.sign_up"); + }); }); describe("findMissingFeatureIds", () => { From af2334650c609b3d6bc73681a4251aa3c3ea263d Mon Sep 17 00:00:00 2001 From: Lukas Klingsbo Date: Fri, 7 Aug 2026 13:20:09 +0200 Subject: [PATCH 2/2] fix(compliance): use distinct placeholders in the registration example The new-symbol failure message showed the same uncovered symbol under both `symbols` and `supporting_symbols`, which contradicts the distinction the message is trying to teach. The check cannot know which of the two a new symbol belongs in, so the example now uses neutral placeholders for each role and the offending names are left to the list above it. Also pins removal detection across supporting symbols with a test. The check asks whether the compliance file references API that no longer exists, which is equally true for a supporting type, and since the drift check reads only `symbols` nothing else would catch a stale entry. --- scripts/capability-matrix/src/api-check.ts | 24 +++++++----------- .../capability-matrix/test/api-check.test.ts | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/scripts/capability-matrix/src/api-check.ts b/scripts/capability-matrix/src/api-check.ts index b995c1c..630183c 100644 --- a/scripts/capability-matrix/src/api-check.ts +++ b/scripts/capability-matrix/src/api-check.ts @@ -44,30 +44,24 @@ export function formatErrorMessage( lines.push(` defined at: ${location}`); } } - const example = uncoveredSymbols[0]?.name ?? "ClassName.methodName"; lines.push( "", - "Register each symbol in sdk-compliance.yaml under the appropriate feature:", - "", - " auth.my_feature:", - " status: implemented", - " symbols:", - ` - ${example}`, - "", - "Use `symbols` only for the entry points that implement the capability, since", - "the drift check treats them as evidence that it exists. Option types, result", - "types, schema models and errors belong under `supporting_symbols`, which", - "counts for coverage without claiming to implement anything:", + "Register each symbol in sdk-compliance.yaml under the feature it belongs to:", "", " auth.my_feature:", " status: implemented", " symbols:", " - GoTrueClient.myFeature", " supporting_symbols:", - ` - ${example}`, + " - MyFeatureOptions", + " - MyFeatureResponse", "", - "Types shared across several features can go in the top-level", - "`supporting_symbols` list instead of being attributed to one of them.", + "`symbols` is for the entry points a caller invokes. Keep it precise: the", + "drift check treats every name in it as evidence the capability exists.", + "Option types, result types, schema models and errors belong under", + "`supporting_symbols`, which counts for coverage without claiming to", + "implement anything. Types shared across several features can go in the", + "top-level `supporting_symbols` list instead of being attributed to one.", "", "If the feature does not exist in the matrix yet, add it there first:", " https://github.com/supabase/sdk/blob/main/CONTRIBUTING.md", diff --git a/scripts/capability-matrix/test/api-check.test.ts b/scripts/capability-matrix/test/api-check.test.ts index 4f8aff1..1a9cd82 100644 --- a/scripts/capability-matrix/test/api-check.test.ts +++ b/scripts/capability-matrix/test/api-check.test.ts @@ -104,6 +104,31 @@ describe("checkNewSymbols — removed registered symbols", () => { expect(symbols).toContain("AuthClient.signIn"); expect(symbols).toContain("AuthClient.signUp"); }); + + // Removal detection deliberately spans supporting symbols as well as entry + // points. The check asks whether the compliance file references API that no + // longer exists, which is equally true either way, and the drift check reads + // only `symbols`, so nothing else would catch a stale supporting entry. + it("detects a removed supporting symbol", () => { + const withSupporting = { + sdk: "javascript", + features: { + "auth.sign_up": { + status: "implemented", + symbols: ["AuthClient.signUp"], + supporting_symbols: ["SignUpOptions"], + }, + }, + supporting_symbols: ["AuthException"], + }; + const base = [sym("AuthClient.signUp"), sym("SignUpOptions"), sym("AuthException")]; + const pr = [sym("AuthClient.signUp")]; + const result = checkNewSymbols(base, pr, withSupporting); + expect(result.removedRegisteredSymbols).toEqual([ + { symbol: "SignUpOptions", featureId: "auth.sign_up" }, + { symbol: "AuthException", featureId: "supporting_symbols" }, + ]); + }); }); describe("formatErrorMessage", () => {