Skip to content

docs: apply comprehensive 2.0 copy review - #201

Open
NikolayS wants to merge 2 commits into
NikolayS:mainfrom
samo-agent:agent/copy-review-157
Open

docs: apply comprehensive 2.0 copy review#201
NikolayS wants to merge 2 commits into
NikolayS:mainfrom
samo-agent:agent/copy-review-157

Conversation

@NikolayS

Copy link
Copy Markdown
Owner

Summary

Applies the comprehensive user-facing copy inventory from #157 after
rechecking every finding against current main and the queued final stamp in
#200.

The history is deliberately split:

  1. docs: correct misleading 2.0 contracts applies every actively misleading
    finding, including the scheduler behavior needed to make documented
    cadences exact, the ROLLUP_DESIGN.md correction from 43,800 rows/year to
    8,760 rows/year (approximately 43,800 over five years), and the
    RAISE WARNING severity correction.
  2. docs: normalize historical and catalog wording contains the lower-severity
    terminology and precision cleanups. It can be dropped independently.

Because #179 froze the beta payload in sql/, behavioral/catalog corrections
are carried as the valid 2.0-beta1 development installer overlay at
devel/sql/ash-install.sql; the workflow and upgrade-chain helpers consume
that overlay.

Tail-aware deduplication

The branch is based on 87c22a3, which already includes #192, #179, #198, and
#199. The recheck retained their fixes and removed copy-review duplicates:

Coordination with draft #200

This PR should land before #200. Then #200 should be refreshed onto the
post-#157 main; it must not be merged first.

#200 wins on every identity surface it changes:

  • README: current main contains 2.0 SQL, v1.5 is the previous stable release,
    and the status example reports 2.0.
  • Release notes: final 2.0 heading, v2.0 tag/payload identity, beta-to-final
    upgrade wording, and final-release organization.
  • SQL/SECURITY: all final 2.0 identity stamps and support labels.

After this PR lands, #200 must promote
devel/sql/ash-install.sql into sql/ash-install.sql, preserve all four final
2.0 identity sites, and remove the development overlay. A plain rebase that
leaves a final released payload plus the beta overlay is invalid under #179's
SQL-chain checks.

The overlapping RELEASE_NOTES.md content requires these explicit resolutions:

#178 remains independent and is not merged or marked ready by this PR.

Verification

  • devel/scripts/release_gate.sh 17 all: PASS on all seven surfaces
    (fresh-install, upgrade-chain, features, degraded-no-cron,
    degraded-no-pgss, degraded-neither, and cron-path).
  • devel/scripts/test_ci_step_script.py: 7/7 PASS.
  • devel/scripts/test_release_stamp.py: 17/17 PASS.
  • git diff --check origin/main...HEAD: PASS.
  • Current release: prepare 2.0 stamp #200 head d1971fd was rechecked for overlap immediately before
    push; its identity wording remains authoritative as described above.

Refs #157.

Inventory:

@NikolayS NikolayS left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

VERIFIED — ready to land.

  • Hand rebase: clean/no-op onto current main 87c22a3; the reviewed head remains dac20cf.
  • Commit split: 97f22ac contains the actively misleading contract fixes and the complete development installer overlay; dac20cf contains the independently droppable terminology/catalog normalization. I checked the changes against the owner's #157 inventory and found no wording hunk to drop.
  • Red/green on PostgreSQL 17: current main accepted ash.start('59 minutes'), so the new uniform-cadence assertion failed with 59-minute calendar cadence was not rejected; this head rejects it, exposes sampling_config_enabled / sampling_evidence, removes the misleading legacy status labels, and uses WARNING severity as specified.
  • devel/scripts/release_gate.sh 17 all: all seven surfaces PASS (fresh-install, upgrade-chain, features, three degraded modes, and cron-path).
  • devel/scripts/test_ci_step_script.py: 7/7 PASS.
  • devel/scripts/test_release_stamp.py: 17/17 PASS.
  • git diff --check origin/main...HEAD: PASS.

No hunks dropped. (GitHub does not permit the repository owner to approve their own PR, so this is recorded as a verification review rather than an APPROVE event.)

@samo-agent
samo-agent force-pushed the agent/copy-review-157 branch from dac20cf to a437807 Compare July 28, 2026 03:48

@NikolayS NikolayS left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

VERIFIED on rewritten head a437807 — this supersedes my review on the pre-edit head.

The two-commit split is preserved (428a4a7 actively misleading contracts; a437807 lower-severity catalog/history normalization). I found and corrected five factual copy defects before landing:

  1. made the threshold pseudocode internally consistent (count(*) for both total and threshold, matching the shipped one-row-per-appearance CTE);
  2. made the one-bucket 1m example satisfy avg = peak = p99;
  3. qualified raw → rollup_1m → rollup_1h ordering to sources whose retention reaches the requested start;
  4. stated that samples() is raw-only without a source column and chart() identifies source only for a widening rollup_1h plan;
  5. corrected RAISE LOG routing to account for both server and client thresholds and disclosed the changed enable-message text in the 2.0 notes.

I also scoped sql/ash--1.0.sql as absent from the current 2.0 tree rather than nonexistent. No complete wording hunk was dropped.

Discriminating PostgreSQL 17 red/green reproduced on the rewritten head: current main accepts ash.start('59 minutes'), so the new assertion fails with 59-minute calendar cadence was not rejected; a437807 rejects it and passes the new status/severity contract.

Final verification:

  • devel/scripts/release_gate.sh 17 all: 7/7 surfaces PASS.
  • devel/scripts/test_ci_step_script.py: 7/7 PASS.
  • devel/scripts/test_release_stamp.py: 17/17 PASS.
  • git diff --check origin/main...HEAD: PASS.
  • Hand rebase onto 87c22a3: clean; PR reports MERGEABLE/CLEAN.

Ready for hosted CI and squash merge.

@NikolayS
NikolayS marked this pull request as ready for review July 28, 2026 03:49
@samo-agent
samo-agent force-pushed the agent/copy-review-157 branch 2 times, most recently from 4df93f2 to 77b196f Compare July 28, 2026 05:56

@samo-agent samo-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

APPROVE on rebased head 77b196f6620b8017f699df3f9fd353017d87d6cd.

Verification reproduced the discriminators rather than relying only on copy inspection: base lacks the uniform-cadence guards and fails the focused probe; this head enforces a 12-hour maximum, accepts 30m/12h, rejects 59m/5h without changing jobs/config, corrects hourly storage to 8,760 rows/year (about 43,800 over five years), and emits a real RAISE WARNING. The complete PostgreSQL 17 release gate passed all seven surfaces; CI-step tests are 7/7, release-stamp tests 17/17, and the diff is clean.

The two commits remain split between actively misleading contracts and cosmetic normalization. The rebased tree is byte-identical to the reviewed pre-rebase tree. No hunks were dropped.

@samo-agent

Copy link
Copy Markdown
Contributor

Reopening immediately to retrigger the missing pull_request check suite on the verified rebased head; no code or review state is changing.

@samo-agent
samo-agent force-pushed the agent/copy-review-157 branch from 77b196f to f7a7c8e Compare July 28, 2026 06:22
@samo-agent

Copy link
Copy Markdown
Contributor

Codex review (standing in for REV) — verification complete; merge blocked on hosted CI.

Reviewed exact head f7a7c8ef66db2a9339481cd678fe14a775b0228c after a clean hand rebase onto origin/main@87c22a3. git range-diff shows both commits patch-identical to the previously reviewed head and the final tree is byte-identical. Codex found no actionable regression; PostgreSQL 17 passed all seven release-gate surfaces, the CI-step tests passed 7/7, the release-stamp tests passed 17/17, and git diff --check passed. The two-commit misleading/cosmetic split remains intact; no wording hunk was dropped.

Hosted CI link: not available — GitHub allocated no workflow run or check suite for this fork head after the synchronize push. Per the landing rules, I will not merge until an upstream maintainer approves/runs the PR workflow and the exact head is green.

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