feat(config): export diffProjectConfig from the public entrypoint - #6447
Conversation
Studio is now a second real consumer of the config-diff classification engine, so promote diffProjectConfig/ConfigChange* from @supabase/config/internal to the public `.` entrypoint per ADR 0022's deferred-promotion condition.
|
/ai-review |
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
The export promotion is code-consistent: the alleged remaining internal import does not exist, so the reported build failure is refuted. Three documentation issues are confirmed: the public export inventory is stale, the PR description understates pure-graph growth, and ADR 0022 omits its usual revision annotation. No confirmed runtime, build, or security defect was found.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | packages/config/README.md:108 |
documentation |
claude | The published package README claims to enumerate every public . export but omits diffProjectConfig and its five supporting types; its entrypoint summary is also stale. |
| ⚪ NIT | packages/config/src/entrypoint-purity.unit.test.ts:300 |
documentation |
claude | The PR description incorrectly says no new module enters the pure runtime graph; config-diff.ts itself is newly added to that graph, although its dependencies were already present and the addition is safe. |
| ⚪ NIT | docs/adr/0022-config-diff-classification-and-managed-surface.md:4 |
documentation |
claude | ADR 0022 rewrites its public-entrypoint decision without updating the revision annotation in its date header, despite annotating its two prior revisions there. |
Findings outside the diff
- 🟡 MINOR
packages/config/README.md:108— The published package README claims to enumerate every public.export but omitsdiffProjectConfigand its five supporting types; its entrypoint summary is also stale. - ⚪ NIT
docs/adr/0022-config-diff-classification-and-managed-surface.md:4— ADR 0022 rewrites its public-entrypoint decision without updating the revision annotation in its date header, despite annotating its two prior revisions there.
Refuted findings (kept for transparency, not posted as review comments)
packages/config/src/internal.ts:19(build-failure): Removing the config-diff exports from the internal entrypoint leavesdiff.live.test.tsimportingConfigChangeClassfrom it, causing a TypeScript missing-export error.
Refuted: The cited test does not importConfigChangeClassor@<!---->supabase/config/internal. A repository-wide search finds no remaining CLI import of that type; the actual affected formatter, formatter test, and handler imports were migrated.
Stats
Claude findings: 3 · Codex findings: 1 · Confirmed: 3 · Refuted: 1 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Address AI-review findings on PR #6447: the README's public `.` export inventory omitted diffProjectConfig/ConfigChange*, and ADR 0022's revision-date header didn't record this promotion alongside its two prior revisions.
There was a problem hiding this comment.
🤖 AI Review
The API promotion is consistent in the checked-out PR. Four nit-level findings are confirmed: one inaccurate PR-description claim, an undocumented ES2023 browser requirement, an import-style inconsistency, and a trusted import-convention violation. Two findings are refuted because the current PR already updates the ADR date and the alleged stale live-test import does not exist. No correctness, security, or build-breaking issue remains.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| ⚪ NIT | packages/config/src/entrypoint-purity.unit.test.ts:300 |
documentation |
claude | The PR description inaccurately says no new module enters the pure graph; config-diff.ts itself newly enters that graph, although its dependencies were already present. |
| ⚪ NIT | apps/cli/src/legacy/commands/config/diff/diff.format.ts:1 |
style |
claude | The import contains only types but uses inline type modifiers instead of the repository's prevalent import type statement form. |
| ⚪ NIT | apps/cli/src/legacy/commands/config/diff/diff.handler.ts:3 |
conventions |
claude | The handler imports public config symbols from the default entrypoint despite also requiring the Effect-native loadCliConfig API. |
| ⚪ NIT | packages/config/src/config-diff.ts:380 |
compatibility |
claude | Promoting config-diff.ts to the browser-facing public entrypoint introduces an Array.prototype.toSorted requirement without documenting a browser baseline. |
Findings outside the diff
- ⚪ NIT
packages/config/src/config-diff.ts:380— Promoting config-diff.ts to the browser-facing public entrypoint introduces an Array.prototype.toSorted requirement without documenting a browser baseline.
Refuted findings (kept for transparency, not posted as review comments)
docs/adr/0022-config-diff-classification-and-managed-surface.md:4(documentation): The ADR changes its Decision section without recording the public-entrypoint promotion in its Date header.
Refuted: The checked-out PR already records the promotion in the Date header at line 4, so the reported omission is not present.packages/config/src/internal.ts:19(build-breakage): Removing ConfigChangeClass from the internal entrypoint leaves a live CLI test importing it from @supabase/config/internal, breaking type-checking.
Refuted: The cited live test imports only Vitest and a local test helper; it neither imports nor references ConfigChangeClass. No stale CLI consumer exists in the checked-out code.
Stats
Claude findings: 5 · Codex findings: 1 · Confirmed: 4 · Refuted: 2 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@35ca79d62eef7d03a8ed35aba55fb2e7200dc1e9Preview package for commit |
Address remaining findings from PR #6447's second AI review round: drop the ES2023 Array.prototype.toSorted usage in config-diff.ts (a plain .sort() on the already-fresh filtered array is equivalent and keeps the pure entrypoint's browser baseline unchanged); align diff.format.ts's type-only import to the repo's `import type` form; and route diff.handler.ts's pure @supabase/config imports through @supabase/config/effect instead, matching every other consumer in the codebase that mixes @supabase/config/internal with Effect-native symbols. diff.live.test.ts (pulled in by the develop merge) is updated to import the now-public ConfigChangeClass from @supabase/config instead of @supabase/config/internal.
CI's fmt:check caught unformatted markdown table rows from the config-diff export-table addition.
…ublic-entrypoint # Conflicts: # apps/cli/src/legacy/commands/config/diff/diff.format.ts # apps/cli/src/legacy/commands/config/diff/diff.handler.ts # packages/config/src/entrypoint-purity.unit.test.ts # packages/config/src/internal.ts
Carries #6446 (branch-target detection and confirmation in config push) onto the diff-first handler: target resolution and the inferred-branch confirmation gate run before the cost-matrix read, the resolved target line replaces the bare ref line, and the machine payload gains the target fields. Adopts #6447's public diffProjectConfig entrypoint and re-applies the family hoists on develop's diff handler/formatter.
What changed
diffProjectConfigand its supporting types (ConfigChange,ConfigChangeClass,ConfigChangeCounts,ConfigChangeSet,DiffProjectConfigOptions) move from@supabase/config/internalto the package's public.entrypoint.Why
ADR 0022 deliberately deferred exporting the config-diff classification engine publicly until a second real consumer needed it, rather than exporting it speculatively. Studio is now that second consumer, so this promotes the (already pure/synchronous) engine to the public surface
apps/cliand Studio can both read.Notes for reviewers
config-diff.tsitself is newly added toindex.ts's pure runtime graph, but every module it transitively depends on (config-document.ts,project-config/project-config.ts,project-config/registry.ts,sparse.ts) was already reachable from that graph, so no new transitive module or bare specifier enters it.apps/cli'sconfig diffcommand (diff.handler.ts,diff.format.ts, and their test) are updated to importdiffProjectConfig/ConfigChange*from@supabase/configinstead of@supabase/config/internal; other internal-only imports in those files (loadCliConfig,projectConfigApiBlockKeys) are unaffected.packages/config/AGENTS.md,packages/config/README.md, and ADR 0022 are updated to reflect the new entrypoint.