Skip to content

Change default mtree schema from "spock" to "pgedge_ace"#116

Merged
mason-sharp merged 2 commits intomainfrom
task/default-schema
Apr 22, 2026
Merged

Change default mtree schema from "spock" to "pgedge_ace"#116
mason-sharp merged 2 commits intomainfrom
task/default-schema

Conversation

@mason-sharp
Copy link
Copy Markdown
Member

@mason-sharp mason-sharp commented Apr 20, 2026

Update ace.yaml, default_config.yaml, configuration docs, and visualise.sh. Add -S flag to visualise.sh for schema override. CHANGELOG documents the breaking change and migration path.

Users of mtree functionality with previous versions will want to change their default schema back to "spock".

Also make tests less flakey

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Walkthrough

The default Merkle-tree CDC metadata schema changed from "spock" to "pgedge_ace" across configs, docs, and tooling. visualise.sh gained a -S option to specify the ACE schema. A breaking-change note and migration guidance were added to the changelog.

Changes

Cohort / File(s) Summary
Configuration Schema Update
ace.yaml, internal/cli/default_config.yaml
Changed mtree.cdc.schema default from "spock" to "pgedge_ace".
Documentation Updates
docs/CHANGELOG.md, docs/configuration.md
Added a ⚠️ breaking-change entry and migration instructions; updated configuration docs to show "pgedge_ace" as the default.
Script Enhancement
visualise.sh
Added -S <ace_schema> flag, updated getopts handling, defaulted ace_schema to pgedge_ace, and revised usage text/examples.

Poem

🐰 In my burrow I nibble a change,
From spock to pgedge_ace I range,
A new -S flag flutters like spring,
Schemas hop, the scripts now sing,
I twitch my nose—update with a bounce!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: updating the default mtree schema value from 'spock' to 'pgedge_ace'.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description clearly relates to the changeset, documenting the schema update, -S flag addition, CHANGELOG entry, and test improvements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/default-schema

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Apr 20, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes. Give us feedback

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@visualise.sh`:
- Around line 37-40: The ace_schema value parsed from getopts (flag -S) is
user-controlled and must be validated before being interpolated into SQL
identifiers; add a validation step in visualise.sh after getopts (or immediately
after assigning ace_schema) that rejects any value that does not match a safe
SQL identifier pattern (e.g., starts with a letter or underscore and contains
only letters, digits or underscores), print a clear error mentioning the -S
argument and ace_schema, and exit non‑zero; ensure the validation is applied to
ace_schema before any use in SQL-building code paths (references: ace_schema,
getopts handling for -S).
🪄 Autofix (Beta)

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

Run ID: 26d8201f-292b-4deb-bfba-396f166093f7

📥 Commits

Reviewing files that changed from the base of the PR and between 34f13e0 and e528c2c.

📒 Files selected for processing (5)
  • ace.yaml
  • docs/CHANGELOG.md
  • docs/configuration.md
  • internal/cli/default_config.yaml
  • visualise.sh

Comment thread visualise.sh
Update ace.yaml, default_config.yaml, configuration docs, and
visualise.sh. Add -S flag to visualise.sh for schema override.
CHANGELOG documents the breaking change and migration path.
@mason-sharp mason-sharp force-pushed the task/default-schema branch from e528c2c to 23b7e44 Compare April 21, 2026 21:16
Counts alone miss UPDATE-only drift: subtests that modify field values
without changing row counts leave the two-node table content temporarily
divergent while their cleanup repair replicates. The old gate returned
immediately in that window, letting the next subtest start against
drifted state.

Add a hashtext-sum row digest alongside the count check. The digest is
order-independent and PK-agnostic, so it generalises across tables and
catches UPDATE-induced drift that counts cannot see.
@ibrarahmad ibrarahmad self-requested a review April 22, 2026 05:30
@mason-sharp mason-sharp merged commit a1b3388 into main Apr 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants