Skip to content

4.4.3-3 docs - #10

Merged
brianmacy merged 1 commit into
mainfrom
sync-4.4.3-3
Nov 2, 2026
Merged

brianmacy merged 1 commit into
mainfrom
sync-4.4.3-3

Conversation

@brianmacy

Copy link
Copy Markdown
Contributor

Public docs and changelog for release 4.4.3-3: install preflight, source via tables, resolver sizing derived from Postgres, license fields in SZ_HEALTH, and the breaking JSON_DOC → RECORD_JSON mapping-view rename.

@brianmacy
brianmacy requested a review from a team as a code owner November 2, 2026 10:47
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

PR Code Review

This PR is docs and changelog only (CHANGELOG.md, docs/*.md). The install script and SQL that implement these changes aren't in the diff, so I could only check the docs for consistency with each other. I did not review the code behind them.

Code Quality

  • ✅ Style conventions: no code changes. The SQL example in docs/use.md is consistently formatted.
  • ✅ No commented-out code.
  • ✅ Meaningful names: RECORD_JSON, LICENSE_DAYS_LEFT and similar are clear.
  • ✅ DRY: the LICENSE_* explanations repeat across CHANGELOG, troubleshooting and use.md, but each doc has its own audience.
  • ❌ Defects and doc accuracy:
    • Unexplained changelog restructure (CHANGELOG.md:69-83). The diff adds a ## [4.4.3-2] - 2026-10-31 heading. It sits above the older bullets, including the sz-snowflake-cord-data.tar.gz entry, so those now fall under 4.4.3-2. Confirm that is intended. If these items shipped in 4.4.3-2, the Unreleased section is right to start fresh. If the heading was added to carry them, the diff should say so.
    • Breaking change for existing views (CHANGELOG.md:51-55). JSON_DOC → RECORD_JSON with no alias means existing registered views stop resolving. The note says to rename the column before upgrading, but it doesn't say how an upgrade behaves if the user forgets. Please verify that SZ_UPGRADE fails loudly and doesn't leave a half-resolved state.
    • Stale numbers in docs/use.md: the connection-budget paragraph reads "4 resolvers x 11 + 11 + 4 = 59" and says "STANDARD_L allows 80". The same paragraph's older text says "a 4-thread resolver holds 9 sessions". Check that these figures are consistent with SZ_CONFIGURE's -20024 check, which allows PG_MAX_CONNECTIONS less 10.
    • "Idle … two of the cheapest nodes … 1.4 credits a day" in docs/use.md is unchanged. Check it is still true now that the pool maximum is derived.
    • Mixed via syntax (docs/use.md, docs/ai-assisted.md). The SQL form uses a map ('via': {...}). The install form is TABLE.KEY:TABLE.COL.RECORD_ID, which uses dots and breaks on schema-qualified names with ambiguous parsing. Verify the script splits db.schema.table.column correctly.
    • docs/ai-assisted.md placeholder: the --source template <db.schema.mapping_view>:<db.schema.table>[=<column>][,...][:<record id column>] is hard to read. The [,...] obscures that [=<column>] repeats per table. docs/use.md words it better.
  • ✅ Project .claude/CLAUDE.md: not part of this diff. The only copy is the untracked build-resources/.claude/CLAUDE.md, which is review tooling and not part of the PR.

Testing

  • ❌ Unit and integration tests: none are in the diff. The behaviors the docs describe have no tests here:

    • the new SZ_HEALTH license codes;
    • via capture;
    • SZ_REGISTER_SOURCE upper-casing;
    • the preflight checks;
    • the resolve wait: line;
    • the derived RESOLVER_MAX_INSTANCES.

    If the tests live in the code PR, link it.

  • ❌ Edge cases and coverage: can't be assessed from a docs-only diff. Cases worth covering:

    • an unknown PG_COMPUTE_FAMILY falling back to 4;
    • an upgrade resetting the old defaults 7 and 9;
    • via pointing at a table that isn't one of the source's own tables.

Documentation

  • ✅ README and API docs: use.md, install.md, troubleshooting.md and ai-assisted.md are all updated.
  • ✅ CHANGELOG updated under Unreleased, with a Breaking marker.
  • ❌ Other docs:
    • Check that README.md and docs/install.md examples elsewhere don't still say JSON_DOC. A grep of the tracked .md files found it only in the CHANGELOG breaking note, which is fine.
    • docs/ai-assisted.md:35: the table row for "Your sources" is now far wider than the others. Prettier will reflow the whole table, so run prettier --write.
    • The first Added bullet in CHANGELOG.md:15 is one very long line. Every other bullet is wrapped at about 90 columns, so wrap it.
    • CHANGELOG.md has a blank line between some bullets and not others, and the via example's continuation lines are indented inconsistently ('key_column'… starts at column 0). Inside a list item, a line at column 0 is a lazy continuation, so it renders but isn't prettier-clean. Run prettier.
    • In docs/use.md, the long Settings table rows and the 2-line CORD paragraph exceed the usual width. Run prettier to confirm formatting.
    • Inline comments are not applicable to this diff.
  • ✅ Table via section in use.md is clear and includes a worked example.

Security

  • ✅ No hardcoded credentials in the diff.
  • ✅ No sensitive data in logs. The resolve wait: line carries counts and status only.
  • ✅ Input validation: the docs describe up-front validation, both of the preflight checks and of SZ_REGISTER_SOURCE.
  • ✅ Error handling: the docs describe the failed-poll retry and the timeout behavior.
  • ✅ SQL example: it binds nothing user-supplied. The AI prompt says to bind search text as a parameter, which is good.
  • ✅ No .lic files or AQAAAD strings in the diff or in tracked files. The AQAAAD matches are in untracked review-tooling files (prompt-header.md, prompt-temp.md). They are not part of the PR and should not be committed.

Summary

The docs are clear and consistent. Before merging:

  1. Confirm the [4.4.3-2] changelog heading placement.
  2. Run prettier on CHANGELOG.md and the docs/ files, which fixes the wide table and wrapping issues.
  3. Point to the tests for the described behavior.
  4. Check the --source-via dotted syntax against schema-qualified names.

Automated code review analyzing defects and coding standards

@brianmacy
brianmacy merged commit e4d86fe into main Nov 2, 2026
8 checks passed
@brianmacy
brianmacy deleted the sync-4.4.3-3 branch November 2, 2026 10:49
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.

1 participant