docs: apply comprehensive 2.0 copy review - #201
Conversation
NikolayS
left a comment
There was a problem hiding this comment.
VERIFIED — ready to land.
- Hand rebase: clean/no-op onto current main
87c22a3; the reviewed head remainsdac20cf. - Commit split:
97f22accontains the actively misleading contract fixes and the complete development installer overlay;dac20cfcontains 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 with59-minute calendar cadence was not rejected; this head rejects it, exposessampling_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, andcron-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.)
dac20cf to
a437807
Compare
NikolayS
left a comment
There was a problem hiding this comment.
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:
- made the threshold pseudocode internally consistent (
count(*)for both total and threshold, matching the shipped one-row-per-appearance CTE); - made the one-bucket 1m example satisfy avg = peak = p99;
- qualified raw → rollup_1m → rollup_1h ordering to sources whose retention reaches the requested start;
- stated that
samples()is raw-only without a source column andchart()identifies source only for a widening rollup_1h plan; - corrected
RAISE LOGrouting 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.
4df93f2 to
77b196f
Compare
samo-agent
left a comment
There was a problem hiding this comment.
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.
|
Reopening immediately to retrigger the missing pull_request check suite on the verified rebased head; no code or review state is changing. |
77b196f to
f7a7c8e
Compare
|
Codex review (standing in for REV) — verification complete; merge blocked on hosted CI. Reviewed exact head 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. |
Summary
Applies the comprehensive user-facing copy inventory from #157 after
rechecking every finding against current
mainand the queued final stamp in#200.
The history is deliberately split:
docs: correct misleading 2.0 contractsapplies every actively misleadingfinding, including the scheduler behavior needed to make documented
cadences exact, the
ROLLUP_DESIGN.mdcorrection from 43,800 rows/year to8,760 rows/year (approximately 43,800 over five years), and the
RAISE WARNINGseverity correction.docs: normalize historical and catalog wordingcontains the lower-severityterminology and precision cleanups. It can be dropped independently.
Because #179 froze the beta payload in
sql/, behavioral/catalog correctionsare carried as the valid
2.0-beta1development installer overlay atdevel/sql/ash-install.sql; the workflow and upgrade-chain helpers consumethat 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:
rollup_minute()catalog contract remains untouched.dropped: processed-grain returns, PostgreSQL 19beta2/release-gate coverage,
default-reader regrant behavior, hourly-grain disclosure, retention
geometry, logical/physical raw bounds, compacted attribution, and stale
rollup fallback.
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:
maincontains 2.0 SQL, v1.5 is the previous stable release,and the status example reports
2.0.2.0heading,v2.0tag/payload identity, beta-to-finalupgrade wording, and final-release organization.
2.0identity stamps and support labels.After this PR lands, #200 must promote
devel/sql/ash-install.sqlintosql/ash-install.sql, preserve all four final2.0identity sites, and remove the development overlay. A plain rebase thatleaves a final released payload plus the beta overlay is invalid under #179's
SQL-chain checks.
The overlapping
RELEASE_NOTES.mdcontent requires these explicit resolutions:credits, and release-engineering section.
whole-minute values must divide 60, whole-hour values must divide 24, and
the maximum is 12 hours. Do not retain release: prepare 2.0 stamp #200's current “through 23 hours”
wording.
sql/ash--1.0.sql.ash.timeline()catalog comment describes “no stored observation”; thewording does not create heartbeat storage.
#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, andcron-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.d1971fdwas rechecked for overlap immediately beforepush; its identity wording remains authoritative as described above.
Refs #157.
Inventory: