Skip to content

release: reject inverted reader windows and clarify diagnostics - #180

Merged
NikolayS merged 4 commits into
mainfrom
agent/w2-b5
Jul 27, 2026
Merged

release: reject inverted reader windows and clarify diagnostics#180
NikolayS merged 4 commits into
mainfrom
agent/w2-b5

Conversation

@NikolayS

@NikolayS NikolayS commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

Rebase resolution

Hand-rebased onto main c28fce3. The resolution preserves #181's dynamic reader/helper grants, #182's honest rollup_1h_flat planning, and #189's logical/physical raw-retention boundary and active-slot exclusion. No new reader helper was introduced.

Validation

Keep #180's until-relative compare defaults exactly once. The #182 reader expansion carried an older duplicate declaration block whose now-relative defaults would both fail function compilation and discard the B5 contract.

Use #189's _raw_retention_start() and _active_slots() helpers for #180's alternate-coverage NOTICE instead of reintroducing the pre-B3 retention formula. This keeps the corrected logical/physical raw boundary split and excluded-slot rule intact.

Adapt the sparse-hour C1 gate to #182's honest rollup_1h_flat plan: aggregate readers disclose one widened hourly datum, while report still returns silent NULL because exact per-minute coverage is not derivable.
When cron.database_name is unset or names the current database, explain how to configure pg_cron or use the external scheduler. Recommend installing pg_ash in another database only when the setting names a real, different target.

Document ash.report's rollup_1m-only contract in the README and guard both disclosures in CI. Exercise mutable cron.database_name placeholders only on the cron-off axis because preloaded pg_cron registers the GUC as postmaster-only. This closes the Codex pre-merge review findings without changing report behavior.

@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.

Pre-merge review — PASS

Codex review is standing in for REV for this merge.

Audit verdict verified: B5 requires an error for since > until, an until-only default of until - interval '1 hour', and preservation of the #63 overflow/degenerate clamp after validation. N3–N7 and C1 Option A are documentation/diagnostic contracts, not report source changes.

RED / GREEN

  • RED on current main c28fce3: aas, timeline, top, report, chart, samples, summary, and both compare windows accepted explicit inversions; until-only calls used the wrong now-relative window.
  • GREEN on 56b67dd: all exact function-specific errors and until-relative bounds pass, including both compare windows and the #63 edge clamps.
  • ash.report() still returns SQL NULL when rollup_1m is absent. Raw-only and valid hourly-only windows produce exactly one source-naming NOTICE; no-source, excluded-slot-only, and invalid sparse-hour cases remain silent.

Hand-rebase checks

  • Kept #181's dynamic reader/helper grant bundle and _admin_funcs narrowing; #180 adds no helper, and all reader-visible dependencies remain covered.
  • Kept #182's numeric bucket rounding and honest rollup_1h_flat plan for incomplete minute_counts.
  • Replaced the pre-B3 raw formula with #189's _raw_retention_start() and _active_slots(), preserving the logical/physical boundary and excluded-slot rule.
  • Removed the duplicate pre-rebase ash.compare declaration block.

Review findings resolved

  1. README lacked the explicit rollup_1m-only report contract. Added the shipped disclosure and CI guards.
  2. The pg_cron hint could recommend install pg_ash in (not set) or recommend another database when the configured target was the current database. Split unset/current/alternate guidance and behaviorally tested each branch.
  3. The first test fix tried to mutate preloaded pg_cron's postmaster-only cron.database_name GUC. Restricted mutable placeholder probes to the cron-off axis and verified a real pg_cron-preloaded PG17 server after DROP EXTENSION does not attempt mutation.

Final Codex re-review: no actionable regressions found. Isolated PG17 runtime gates, fresh install, YAML parse, documentation guards, and git diff --check all pass. Scratch resources were removed.

@NikolayS
NikolayS marked this pull request as ready for review July 27, 2026 21:45
@NikolayS
NikolayS merged commit 28136cf into main Jul 27, 2026
11 checks passed
@NikolayS
NikolayS deleted the agent/w2-b5 branch July 27, 2026 21:53
samo-agent added a commit to samo-agent/pg_ash that referenced this pull request Jul 27, 2026
Document that retained AAS is weighted by the current sampler interval, successful idle ticks are not persisted, and intervals above one minute can overstate minute extrema. Mark the affected user stories partial and expose the limitation in the README, release notes, blueprints, and sample_interval catalog comment.

Hand-resolve against current main by retaining NikolayS#182's final grain/provenance contracts, NikolayS#180's until-only window and report disclosures, NikolayS#189's raw-boundary wording, and NikolayS#187's retired-benchmark caveat.
NikolayS added a commit that referenced this pull request Jul 27, 2026
Document that retained AAS is weighted by the current sampler interval, successful idle ticks are not persisted, and intervals above one minute can overstate minute extrema. Mark the affected user stories partial and expose the limitation in the README, release notes, blueprints, and sample_interval catalog comment.

Hand-resolve against current main by retaining #182's final grain/provenance contracts, #180's until-only window and report disclosures, #189's raw-boundary wording, and #187's retired-benchmark caveat.

Co-authored-by: samo-agent <280144521+samo-agent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment