feat: cost schema the production snapshot doesn't cover via the synthesizer#184
Open
veksen wants to merge 2 commits into
Open
feat: cost schema the production snapshot doesn't cover via the synthesizer#184veksen wants to merge 2 commits into
veksen wants to merge 2 commits into
Conversation
…esizer Pass the current schema into Statistics in QueryOptimizer.setStatistics, so a table the exported production snapshot doesn't cover — added on this branch, or since the snapshot was captured — is sized by core's synthesizer (foreign-key graph, shape class, column inference) instead of the flat default. Both the connected and CI paths already thread the schema through onSuccessfulSync -> optimizer.start, so this one change covers both. Expose the synthesized-table set (QueryOptimizer.syntheticTables) and plumb it onto ReportContext.modeledTables for the run report. Rendering it in the PR comment (a 'modeled, not verified' banner) is the follow-up, done once core publishes so the template and snapshot tests can be validated. Requires @query-doctor/core >= 0.13.0 (the currentSchema parameter and syntheticTables); the dep is bumped but the lockfile can't reconcile until that version is published. CI will fail on install until then — expected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Integration test (testcontainers, mirroring query-optimizer.test.ts): a table absent from the exported snapshot but present in the schema — foreign-keyed to a covered table sized at 1M — is reported in syntheticTables and costed from the snapshot, not its single branch row. Proves setStatistics forwards the schema so core's synthesizer engages. Can't run until @query-doctor/core@0.13.0 publishes and deps install; staged to validate when the branch is closed out post-publish, alongside the deferred comment-render. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
The analyzer side of Production Statistics (Site
feat-sync-production-stats, ADR 0004). Makes the core synthesizer actually run on the connected and CI costing paths, so a table the production snapshot has never seen is costed with a real row count and column selectivity instead of the flat 10M default.What
Before:
QueryOptimizer.setStatisticsbuiltStatisticswithout the current schema, so an uncovered table fell to the default. After: the schema is passed in, so uncovered tables are sized by the synthesizer (FK graph → shape class → DB-median) with type/constraint-inferred column stats. Both connected and CI paths already thread the schema throughonSuccessfulSync → optimizer.start, so the single wiring change covers both.How
query-optimizer.ts: passschemaas the 5thStatisticsarg (the onlynew Statisticsin the repo); add asyntheticTablesgetter mirroringcomputedStats/ownMetadata.reporter.ts/runner.ts: plumb the synthesized-table set ontoReportContext.modeledTables.package.json: bump@query-doctor/coreto^0.13.0(the minor that addscurrentSchema+syntheticTables).Deferred (follow-up, same branch)
Rendering the "modeled, not verified" banner in the PR comment (template +
buildViewModel+ snapshot regen). Left until core publishes so the template and snapshot tests can be validated rather than written blind.Sequencing
feat-sync-production-stats→ staging → core publishes0.13.0.npm installreconciles the lockfile, CI goes green, then the comment-render follow-up.