feat(compliance): split symbol evidence from symbol coverage - #75
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe capability matrix now supports 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/capability-matrix/src/api-check.ts`:
- Around line 47-67: Update the registration examples around the `example`
placeholder and `lines.push` so `symbols` and `supporting_symbols` use distinct,
role-appropriate placeholders. Do not reuse the first `uncoveredSymbols` entry
for both fields; use neutral placeholders or select each value according to the
symbol kind, preserving the documented distinction between implementation entry
points and supporting types.
In `@scripts/capability-matrix/src/compliance.ts`:
- Around line 133-154: The symbol index populated in compliance processing
currently mixes coverage-only supporting symbols with implementation symbols
used by checkNewSymbols to compute removedRegisteredSymbols. Introduce a
separate implementation-symbol index containing only feature symbols, use it for
removedRegisteredSymbols, and retain the existing index for new-symbol coverage.
Add a test covering a supporting symbol present in the base API but absent from
the PR API, ensuring it is not reported as removed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d7046c3-308a-4a6c-af05-85e0a7a1a470
📒 Files selected for processing (5)
docs/capability-matrix.mdscripts/capability-matrix/src/api-check.tsscripts/capability-matrix/src/compliance.tsscripts/capability-matrix/src/types.tsscripts/capability-matrix/test/compliance.test.ts
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.
…nd splits (#1667) ## Summary Reconciles `sdk-compliance.yaml` with three upstream changes that have now landed in `supabase/sdk`: - **supabase/sdk#74** renamed and split several canonical feature IDs. - **supabase/sdk#75** separated symbol *evidence* from symbol *coverage*, adding `supporting_symbols`. - **supabase/sdk#76** added the five Iceberg catalog capabilities that #74's split left without an ID. ### Renames and merges - `auth.sign_in.reset_password` → `auth.sign_in.send_password_reset_email` - `realtime.channel.send` → `realtime.channel.broadcast` - `storage.file_buckets.list_files_paginated` merged into `list_files` - `storage.analytics.iceberg_namespace` split into `create_namespace` / `list_namespaces` / `delete_namespace` - `storage.analytics.iceberg_table` split into `create_table` / `list_tables` / `load_table` / `update_table` / `rename_table` / `delete_table` ### New capabilities declared `load_namespace_metadata`, `namespace_exists`, `update_namespace_properties`, `register_table`, `table_exists`. ## Why the Iceberg entries look the way they do Splitting two bundled entries into fifteen raises the question of which symbols belong where. The Iceberg surface is 352 symbols, only 18 of which are catalog entry points; the rest are option types, result types, the schema and type model, and the exception hierarchy. `symbols` now holds **only** the methods a caller invokes, because the drift check treats every name in it as evidence the capability exists. Everything else sits under `supporting_symbols`, which counts for new-symbol coverage without claiming to implement anything. Owners for the supporting types are derived from the source rather than assigned by hand: build the type graph from `packages/storage_client/lib/src/iceberg/`, including subtype edges since a signature naming a sealed base reaches every variant a caller can pass, then ask which entry points reach each type. A type reachable from exactly one feature belongs to that feature. | | count | |---|---| | Sole natural owner | 37 of 67 | | Genuinely shared across several features | 20 | | Reachable from no entry point (thrown, not passed) | 10 | So all 28 `*Update` and `Assert*` classes land on `update_table`, `ListTablesOptions`/`ListTablesResult` on `list_tables`, `RegisterTableRequest` on `register_table`. The schema and type model and the exception hierarchy stay in the top-level `supporting_symbols` list, which is the honest answer rather than a coin flip. The alternative was to replicate the full 330-symbol list across all six table IDs and the 22-symbol list across all three namespace IDs (+1729 lines, as in the now-closed #1666). That inflates what the file claims is implemented, fans drift findings across features that do not own the symbol, and leaves attribution arbitrary, since `buildSymbolIndex` last-wins on collision. Under that shape only 2 of 9 split features resolved to their own entry point; here it is 9 of 9. ## Test plan Validated against current `supabase/sdk@main`, with #74, #75 and #76 all merged: - [x] `validate-compliance`: `OK — compliance file is valid.` Two features remain undeclared (`postgres_changes_multiple_filters`, `error_codes`); both are pre-existing and out of scope here. - [x] `check-drift` against symbols extracted with the real Dart extractor: `✅ No capability matrix drift detected.` - [x] `check-api-symbols`: all public API accounted for; 887 symbols covered, unchanged from before this PR. - [x] Verified no unintended edits: every feature outside the rename and split scope is byte-identical to `main` after re-serialization. - [x] CI re-run after the upstream merges: `Validate compliance file` and `Check public API against capability matrix` both green. No SDK code changes, only capability declarations. Context: [SDK-1439](https://linear.app/supabase/issue/SDK-1439) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Standardized capability names for password reset and realtime broadcast functionality. * Clarified storage file listing capabilities, including pagination and sorting support. * Added more granular capability definitions for Iceberg namespaces and tables. * Consolidated shared Iceberg models, errors, catalog access, and supporting symbols for more consistent capability descriptions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Problem
The
symbolslist insdk-compliance.yamlfeeds two checks that want opposite things:symbolsascheckDriftcheckNewSymbolsOne list cannot serve both. When a capability's supporting types outnumber its methods, authors get pushed into one of two workarounds:
symbolswith option types, result types and exceptions that do not implement anything, orBoth 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
buildSymbolIndexsilently last-wins on collision.This is not hypothetical. supabase/supabase-flutter#1666 hit it head-on: reconciling #74 required replicating a 330-symbol Iceberg list across 6 new feature IDs and a 22-symbol list across 3 more, for +1729 lines. Every one of those 330 symbols now resolves to
storage.analytics.delete_tablein the symbol index, purely because it sorts last.The existing escape hatches do not help.
@internaland.sdk-parse-ignoreremove symbols from the surface entirely, but types likeTableMetadataare genuinely public API that consumers construct. They just are not capabilities.Change
Adds an optional
supporting_symbolslist, per feature and top level, that counts for new-symbol coverage and is never drift-verified:compliance.ts: new field onRawValueandRawCompliance; validation extracted into a sharedcheckSymbolListhelper so both lists get identical treatment;normalizeCompliancepreserves it;buildSymbolIndexunions both. Entry points are indexed last, so a symbol listed both ways is attributed to the capability that implements it rather than to a supporting bucket. Top-level entries index against an exportedTOP_LEVEL_SUPPORTINGsentinel so removal messages stay readable.drift-check.ts: unchanged. It already read onlyvalue.symbols, so the separation falls out for free.api-check.ts: the failure message now teaches the distinction, since that message is exactly where an author hits this wall.types.ts,docs/capability-matrix.md, tests.Compatibility
The field is optional and the drift check already ignored anything outside
symbols, so existing compliance files are unaffected. Verified that supabase-flutter's currentsdk-compliance.yamlvalidates unchanged.Test plan
tsc --noEmitclean.npm run validatestill OK on the canonical registry.createTablemaps tocreate_table, notdelete_table), and 2046 symbol lines become 352.Follow-ups, deliberately not in this PR
renamed_fromaliases on canonical features. Separate concern and arguably higher value: today every ID rename here breaks all seven SDK repos at once, with no window in which both old and new IDs validate, because each repo pins the reusable workflow at@main.loadNamespaceMetadata,namespaceExists,updateNamespaceProperties,registerTableandtableExistsmap to no capability at all. This PR gives them an honest home rather than a false claim, but the underlying gap is worth deciding on separately.