diff --git a/.claude/skills/project-knowledge/overview.md b/.claude/skills/project-knowledge/overview.md index 5d12f24..8de646b 100644 --- a/.claude/skills/project-knowledge/overview.md +++ b/.claude/skills/project-knowledge/overview.md @@ -22,7 +22,7 @@ It reads PostgreSQL internal statistics views and presents them in a top-like in - `pg_replication_slots` (+ `pg_stat_replication_slots`) — replication slots screen (hotkey `o`; PG 14–18; multi-row, all slots; retained WAL + wal_status + spill/stream; TUI-only, not recordable in 0.11.0) - `pg_stat_io` — unified IO breakdown by backend_type × object × context (hotkey `j` toggles count↔time sub-screens, `J` opens the mode menu; PG 16+; multi-row; this is where `buffers_backend`/`buffers_backend_fsync` went on PG 17+ and WAL IO timings on PG 18; TUI-only, not recordable in 0.11.0) - `pg_stat_wal` — WAL generation stats (PG 14+; reduced schema in PG 18 — WAL IO timings moved to `pg_stat_io`) -- `pg_stat_statements` — top queries by various metrics (requires extension) +- `pg_stat_statements` — top queries by various metrics (requires extension); 7 sub-screens under the `X` menu / `x` cycle: timings, general, IO, temp files, local (temp tables), WAL, and **JIT** (compilation cost per query — generation/inlining/optimization/emission phase times + functions, `+deform` on PG 17+; PG 15+; rows filtered to `jit_functions > 0`; TUI-only, not recordable in 0.11.0) - System stats — CPU, memory, disk, network (read from /proc or via PL/Perl schema) ## Target Audience diff --git a/.claude/skills/project-knowledge/patterns.md b/.claude/skills/project-knowledge/patterns.md index 8b004db..f114b98 100644 --- a/.claude/skills/project-knowledge/patterns.md +++ b/.claude/skills/project-knowledge/patterns.md @@ -17,6 +17,13 @@ When a PG version changes columns in a stats view: - Call it in `view.Configure()` under the correct view name - Add version-specific test cases in `*_test.go` +When versions differ by **column count** (not just names), the selector must also carry the +layout: return `(string, int, [2]int)` (`+DiffIntvl`, see `io.go`) or `(string, int, [2]int, int)` +(`+UniqueKey`, see the `statements_jit` selector in `internal/query/statements.go` — UniqueKey +points at the trailing md5 `queryid`, whose index shifts with ncols) and patch all returned +fields in `Configure()`. When only names change but the count is constant (e.g. `statements_timings`), +`Configure()` swaps `QueryTmpl` alone and the static `Ncols`/`DiffIntvl`/`UniqueKey` stay valid. + Reference implementations of the single-row version-aware view: `internal/query/wal.go` and `internal/query/bgwriter.go`. The bgwriter screen is notable for placing absolute event-counter columns (`ckpt_*`, `rstpt_*`) **outside** the contiguous `DiffIntvl` range so they render cumulative, while the work/time/buffer columns inside the range render as per-interval deltas. For a **multi-row hybrid view** that LEFT JOINs two stats views, see `internal/query/replication_slots.go` (the `replslots` screen). Two patterns it establishes: @@ -28,9 +35,11 @@ When the row identity is **composite** (more than one column), emit a synthetic ## Adding a New View — test counts that must be updated -Registering a view in `view.New()` couples to two count-based tests that fail in CI (not always locally) if missed: -- `internal/view/view_test.go: TestNew` pins the total view count (and `TestView_VersionOK` pins per-version availability). -- `record/record_test.go: Test_filterViews` pins, per version, how many views `filterViews` drops vs keeps. A `NotRecordable: true` view is always dropped, so every `wantN` row increases by the number of new `NotRecordable` views (feature 006 added 2 → `+2` each row; `wantV` unchanged). This test runs without Postgres, so a stale count is a real failure even though the rest of the `record` package skips/fails on a missing PG fixture — do not assume a red `record` package is only the connection-refused tests. +Registering a view in `view.New()` couples to count-based tests that fail in CI (not always locally) if missed: +- `internal/view/view_test.go: TestNew` pins the total view count. `TestView_VersionOK` pins per-version availability — its row at a version **≥ the new view's `MinRequiredVersion`** also increases by one (feature 007's PG15+ view bumped only the `160000` row, not the `≤140000` rows). +- `record/record_test.go: Test_filterViews` pins, per version, how many views `filterViews` drops vs keeps. A `NotRecordable: true` view is always dropped, so every `wantN` row increases by the number of new `NotRecordable` views (feature 006 added 2 → `+2` each row; feature 007 added 1 → `+1`; `wantV` unchanged). This test runs without Postgres, so a stale count is a real failure even though the rest of the `record` package skips/fails on a missing PG fixture — do not assume a red `record` package is only the connection-refused tests. + +Adding a `pg_stat_statements` **sub-screen** (or any `menuPgss`/cycle entry) additionally breaks `top` tests — `Test_selectMenuStyle` (pins each menu's item count), `Test_statementsNextView`, and `Test_switchViewTo` (pin the `x`-cycle transitions). These `top` tests DO run locally without Postgres, so they catch the miss in `make test` — but feature 007's code-research overlooked them (the task wrongly assumed the TUI layer had no tests). When touching `top/menu.go` or `top/config_view.go`, grep `top/*_test.go` for the function you changed before assuming it is untested. ## Error Wrapping diff --git a/docs/decisions-log.md b/docs/decisions-log.md index 5b674d3..65a3c70 100644 --- a/docs/decisions-log.md +++ b/docs/decisions-log.md @@ -391,3 +391,35 @@ Used by tech-spec planning and code research to avoid repeating mistakes and re- **Rationale:** Follows the [004] per-version-branch ADR (not NULL-padded). Identical headers keep the DBA's table stable across versions. `coalesce` is mandatory — `pg_stat_io` returns NULL pervasively (e.g. `fsyncs` for `temp relation`), and a NULL inside `DiffIntvl` aborts the whole sample ([005] lesson). **Alternatives considered:** Single NULL-padded query across versions (rejected by [004]); a separate PG 17 branch (unnecessary — 16==17). + +## [007-feat-pg-stat-statements-jit] JIT query selector returns a 4-tuple (query, Ncols, DiffIntvl, UniqueKey) + +**Date:** 2026-06-22 +**Feature:** 007-feat-pg-stat-statements-jit +**Status:** Accepted + +**Context:** The JIT sub-screen's column COUNT differs across versions (PG 15/16 → 13 columns; PG 17+ → 15, adding `deform_total`/`deform,ms`). Because `UniqueKey` points at the trailing synthetic md5 `queryid`, its index shifts with the column count, and `DiffIntvl` shifts too. The existing `statements_timings` selector returns only a query string (its count is constant at 13 across versions), and the `pg_stat_io` selector returns `(string, int, [2]int)` (its `UniqueKey` is a fixed col 0). + +**Decision:** `SelectStatStatementsJITQuery(version) (string, int, [2]int, int)` returns query + `Ncols` + `DiffIntvl` + `UniqueKey`; `view.Configure()` patches all four. Two-way branch: `>= PostgresV17` → (Default, 15, {7,12}, 13); else → (PG15, 13, {6,10}, 11). Gate `MinRequiredVersion: query.PostgresV15`. + +**Rationale:** A trailing-key layout whose index moves with version forces the selector to carry the full layout — returning only the query (timings model) would leave stale `Ncols`/`DiffIntvl`/`UniqueKey`; returning a 3-tuple (io model) would leave a stale `UniqueKey`. Extending to a 4-tuple keeps the layout invariant explicit and unit-testable, and column hiding remains unavailable (`internal/align` floors width at 8, ADR [006]). + +**Alternatives considered:** Compute `UniqueKey = Ncols-2` in `Configure()` (works, but hides a layout invariant in arithmetic). Hide `deform` columns on PG 15 (impossible per ADR [006]). Both rejected. + +--- + +## [007-feat-pg-stat-statements-jit] JIT column layout: timings-style total+interval, drop the *_count phase counters + +**Date:** 2026-06-22 +**Feature:** 007-feat-pg-stat-statements-jit +**Status:** Accepted + +**Context:** `pg_stat_statements` exposes 8 JIT metrics on PG 15/16 (4 counts + 4 times) and 10 on PG 17+. Mirroring `statements_io`'s total+interval doubling across all of them would be ~22–26 columns; pgcenter has no horizontal column scroll (ADR [006]). + +**Decision:** Model the screen on `statements_timings`: cumulative `*_total` text durations + per-interval `*,ms` columns for the four (PG 17+: five) phase TIMES, plus a single `functions` counter. Drop the `inlining_count`/`optimization_count`/`emission_count` (and `deform_count`) metrics. Filter rows to `WHERE jit_functions > 0` (pg_stat_io zero-row precedent), surfacing the `jit=off` empty state via the view `Msg`. + +**Rationale:** The per-phase TIME is the cost signal a DBA acts on; the phase counts are secondary and `jit_functions` already represents "how much JIT work". Dropping the counts keeps the screen at 13/15 columns — within terminal width. The `Msg`-as-hint reuses the existing `track_io_timing` note mechanism (no new GUC-detection code). + +**Alternatives considered:** All counts+times with total+interval doubling (too wide). Splitting into count/time sub-screens like `pg_stat_io` (out of scope — JIT is the low-risk release closer, and the time-only set fits one screen). Both rejected. + +--- diff --git a/docs/features-catalog.md b/docs/features-catalog.md index 77a8acf..fa3ba6d 100644 --- a/docs/features-catalog.md +++ b/docs/features-catalog.md @@ -122,3 +122,23 @@ Used by spec-writer to understand existing functionality and avoid duplication o - `Q` does not reset `pg_stat_io` (it is shared/cluster-wide stats) — noted in the help screen. **Touches:** Shares the multi-row view model with `replslots`/`tables`; third + fourth `NotRecordable` views. Closes the visibility gaps left by [004-feat-bgwriter-checkpointer] (`buffers_backend` on PG 17+) and the `pg_stat_wal` screen (WAL IO timings on PG 18). + +### [007-feat-pg-stat-statements-jit] pg_stat_statements JIT Screen + +**What it does:** Adds a 7th `pg_stat_statements` sub-screen — **JIT** — under the `X` menu (and the `x` cycle), showing per-statement JIT compilation cost. Lets a DBA find which normalized queries pay heavy JIT generation/inlining/optimization/emission time (the classic cause of "mysterious" latency on short queries when `jit=on`) without dropping to `psql`. + +**Key scenarios:** +- Press `X` → choose `pg_stat_statements JIT compilation` (or press `x` to cycle `… wal → jit → timings …`). The screen opens sorted by `gen_total` (cumulative generation time) descending — the heaviest JIT compilers on top. +- Columns: `user`, `database`, cumulative phase totals `gen_total`/`inline_total`/`opt_total`/`emit_total` (`+deform_total` on PG 17+), per-interval `gen,ms`/`inline,ms`/`opt,ms`/`emit,ms` (`+deform,ms` on PG 17+), `functions` (JIT-compiled functions this interval), `queryid`, `query`. +- Decide whether JIT pays off: a query with large phase times but few rows is a candidate for raising `jit_above_cost`/`jit_optimize_above_cost` or turning JIT off for the workload. +- Re-sort (arrows — `*_total` text durations sort numerically) and filter (`/`) like the other pgss sub-screens. + +**Limitations:** +- TUI-only in 0.11.0 — the view is `NotRecordable`, so `pgcenter record`/`report` skip it. record/report is the planned next phase. +- PG 15+ only (JIT columns appeared in PG 15; `deform_*` in PG 17). On PG < 15 the sub-screen reports "not supported" via the runtime version guard. +- Rows with no JIT activity (`jit_functions = 0`) are hidden in SQL; under `jit=off` the screen is empty and the command line shows a hint. +- The `*_count` phase counters (inlining/optimization/emission) are omitted to fit the screen width (no horizontal scroll) — only `functions` is shown; the value is in the phase times. + +**Touches:** Shares the pgss sub-screen model (synthetic md5 `queryid` `UniqueKey`, total+interval columns) with `statements_timings`/`statements_io`; fifth `NotRecordable` view added in 0.11.0 (after bgwriter, replslots, stat_io ×2). Closes release 0.11.0. + +--- diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-adequacy-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-adequacy-review.json new file mode 100644 index 0000000..2f3cab7 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-adequacy-review.json @@ -0,0 +1,21 @@ +{ + "status": "approved", + "findings": [ + { + "category": "overengineering", + "severity": "major", + "issue": "The user-spec embeds tech-spec-level implementation detail: exact column-index tables with idx/DiffIntvl/Ncols/UniqueKey/OrderKey values (lines 82-104), the internal selector function name and signature SelectStatStatementsJITQuery(version) (lines 151, 185-187), specific source file paths (internal/stat/postgres.go, statements_io reference), and the precise test-count edits view_test.go::TestNew (26->27) and record/record_test.go::Test_filterViews +1 (lines 153, 197).", + "why_matters": "User-spec defines WHAT and WHY; HOW (column indices, function names, file paths, Ncols/DiffIntvl values, test-count deltas) belongs in tech-spec. Pinning indices in the user-spec couples the requirement to one implementation, and if the tech-spec chooses a slightly different layout the acceptance criteria become wrong/contradictory. It also blurs the review boundary — these numbers should be derived and validated during tech-spec, not pre-committed here.", + "fix": "Move the index/Ncols/DiffIntvl/UniqueKey tables, the selector name+signature, file paths, and the exact test-count edits into the tech-spec (they are already captured in code-research.md). In the user-spec keep behavioural acceptance criteria: which columns appear per PG version (by meaning, not index), default sort by generation time desc, jit_functions=0 rows hidden, PG<15 unavailable, NotRecordable, hint on empty. Restate AC lines 151/153 as behaviour, not as code edits." + }, + { + "category": "underengineering", + "severity": "minor", + "issue": "Edge cases are listed for the main flows (jit=off / empty set, PG<15 degradation, pg_stat_statements absent, sort/filter), but the duration-aware numeric sort of *_total text columns is mentioned only as an assumption (line 59-60) rather than as an explicit acceptance criterion, and there is no edge case for very large cumulative times (3+ digit hours / 'N days ...') beyond a parenthetical note.", + "why_matters": "The *_total columns are text durations; sorting them numerically (HH:MM:SS with 3+ digit hours and 'N days') is a known sharp edge in internal/stat/postgres.go. Without an explicit AC, a regression there would pass all listed criteria.", + "fix": "Add an acceptance criterion: sorting by a *_total column orders rows numerically by duration (correct for 100+ hour and multi-day values), not lexically. This is behaviour, so it belongs in the user-spec." + } + ], + "worst_category": "overengineering", + "summary": "The idea is sound, feasible, and correctly right-sized as S: a 7th pgss sub-screen built strictly by analogy with established, version-aware NotRecordable views (statements_timings / statements_io / stat_io / replslots) using only the existing stack (pgx/v5, gocui, version selectors). No new dependencies, infrastructure, or architectural conflicts; the chosen single-interval-block layout and WHERE jit_functions>0 filter are the simplest viable approach and match prior ADRs. Approved with no blocking concerns — main improvement is pulling implementation detail (column indices, selector signature, file paths, test-count edits) out of the user-spec into the tech-spec." +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-arch-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-arch-review.json new file mode 100644 index 0000000..6c56e93 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-arch-review.json @@ -0,0 +1,40 @@ +{ + "status": "approved", + "checks": { + "architectural_fit": "pass", + "scalability": "pass", + "database": "n/a", + "coupling": "pass", + "api_contracts": "n/a", + "backward_compat": "pass" + }, + "findings": [ + { + "check": "architectural_fit", + "severity": "minor", + "issue": "The tech-spec states 'OrderKey: 2, gen_total, desc' for the initial sort, but col 2 is a cumulative text interval (date_trunc output, e.g. '00:01:23'), not a numeric or diffed column. The sort engine in internal/stat/postgres.go:sort() auto-detects by probing the first row's value: if parseDuration succeeds it uses duration sort, otherwise falls back to string sort. Under an empty screen (WHERE jit_functions > 0 with no JIT activity) there are zero rows, so sort is a no-op. On a live screen the value '00:00:00' does parse as a duration via parseDuration(), so sorting is numerically correct — but only because the format matches HH:MM:SS. If a statement accumulates more than 24 hours of JIT time the format becomes 'N days HH:MM:SS', which parseDuration also handles. This works correctly and is consistent with the existing statements_timings precedent (OrderKey 0, user, which falls through to string sort). The minor concern is that the tech-spec does not explicitly document that OrderKey 2 relies on the duration-sort branch of stat.sort(), making the reasoning opaque for future contributors.", + "location": "Tech-spec section 'Decisions / Decision 2: Column layout', acceptance criteria OrderKey:2", + "fix": "Add a comment in the view entry or tech-spec noting that gen_total is formatted as HH:MM:SS text, which triggers the duration-sort branch in internal/stat/postgres.go:sort(). No code change required." + }, + { + "check": "architectural_fit", + "severity": "minor", + "issue": "The code-research file (section 5c) flags a risk: 'verify the top view-availability filter removes statements_jit on PG<15 so that viewSwitchHandler on PG14 does not yield a zero-value View{}'. The tech-spec names 'existing VersionOK runtime guard (stat.go:199)' as the mitigation, but does not explicitly resolve this concern. Looking at the codebase: view.New() registers statements_jit unconditionally; config.views is built from view.New() in top/top.go. The VersionOK check lives in the collector path (internal/stat), not in viewSwitchHandler or statementsNextView. This means pressing 'x' on PG14 would call viewSwitchHandler(config, 'statements_jit'), which reads config.views['statements_jit'] — a valid View entry (MinRequiredVersion is set but the view IS in the map). The collector then calls v.VersionOK(version) and returns the 'not supported' message. This is the same behavior as statements_wal on PG12: the view is in the map, but the collector guards it. So the existing pattern handles it correctly. However the tech-spec does not confirm this analysis explicitly; it should note that this is the same graceful-degradation path as statements_wal on PG<13.", + "location": "Tech-spec section 'How it works', step 3 — VersionOK guard", + "fix": "Add one sentence to the tech-spec confirming that the PG<15 degradation path is identical to statements_wal on PG<13: the view is in the map, viewSwitchHandler succeeds, and the collector's VersionOK guard returns the 'not supported' message. No code change required." + }, + { + "check": "backward_compat", + "severity": "minor", + "issue": "The tech-spec marks the statementsNextView cycle change as purely additive ('wal → jit → timings') and calls backward compatibility N/A. This is correct for the record/report path. However, the x-key cycle order is a user-observable behavior change for anyone who relies on the existing 'wal → timings' cycle. The spec describes this as intended, and the change is safe because no external API contract is affected. The mention of it here is informational only — the cycle insertion is the right approach.", + "location": "Tech-spec section 'Backward Compatibility'", + "fix": "Optionally note in the Backward Compatibility section that the x-cycle order changes from 'wal → timings' to 'wal → jit → timings', which is a deliberate UX change, not a breakage." + } + ], + "risk_assessment": { + "performance": "low", + "complexity": "low", + "tech_debt": "low" + }, + "summary": "The spec is architecturally sound and fits the existing codebase precisely. The selector-returns-(query,Ncols,DiffIntvl,UniqueKey) design correctly follows the SelectStatIOQuery model (not the timings model) because Ncols changes across PG versions; the NotRecordable+MinRequiredVersion+synthetic-md5-UniqueKey combination matches the stat_io/stat_io_time precedent exactly; and the column index arithmetic for both the PG15 (13-col, DiffIntvl{6,10}, UniqueKey 11) and PG17 (15-col, DiffIntvl{7,12}, UniqueKey 13) layouts is internally consistent and correct. Three minor findings — all documentation gaps rather than correctness defects — do not block implementation." +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-code-research.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-code-research.md new file mode 100644 index 0000000..65afd48 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-code-research.md @@ -0,0 +1,509 @@ +# Code Research — Feature 007: pg_stat_statements JIT sub-screen + +New 7th pg_stat_statements sub-screen (`statements_jit`, under the `X` menu, hotkey `x` +cycle) showing JIT compilation metrics per (user, database, queryid) row. PG15+ feature; +PG17 adds `jit_deform_count` / `jit_deform_time`. TUI-only, `NotRecordable`. Built by +analogy with the existing `statements_io` sub-screen. + +Research date: 2026-06-21. Model: opus-4-8[1m]. + +--- + +## 1. pg_stat_statements JIT column schema (exact, by PG version) + +Verified against official PostgreSQL docs: +- PG17: https://www.postgresql.org/docs/17/pgstatstatements.html +- PG15: https://www.postgresql.org/docs/15/pgstatstatements.html +- Context7 `/websites/postgresql_17` release notes (E.11.3.11.1): PG17 added the JIT + `deform_counter`, plus `local_blk_read_time`/`local_blk_write_time`, + `stats_since`/`minmax_stats_since`. + +### PG15 / PG16 base set (8 columns) + +| Column | SQL type | Diffable? | Kind | +|--------------------------|--------------------|-----------|----------| +| `jit_functions` | `bigint` | yes | count | +| `jit_generation_time` | `double precision` | yes (ms) | time, ms | +| `jit_inlining_count` | `bigint` | yes | count | +| `jit_inlining_time` | `double precision` | yes (ms) | time, ms | +| `jit_optimization_count` | `bigint` | yes | count | +| `jit_optimization_time` | `double precision` | yes (ms) | time, ms | +| `jit_emission_count` | `bigint` | yes | count | +| `jit_emission_time` | `double precision` | yes (ms) | time, ms | + +### PG17+ additions (2 columns) + +| Column | SQL type | Diffable? | Kind | +|--------------------|--------------------|-----------|----------| +| `jit_deform_count` | `bigint` | yes | count | +| `jit_deform_time` | `double precision` | yes (ms) | time, ms | + +`jit_deform_count` / `jit_deform_time` do NOT exist in PG15/PG16 — confirmed by the PG15 +docs (Table F.20 lacks them). All counts are cumulative `bigint` (safe to diff); all times +are cumulative `double precision` milliseconds. The existing pgss timing queries round such +ms values with `round(p.)` and present them as both a cumulative `text` interval and an +interval-diffable `,ms` column (see `PgStatStatementsTimingPG13`, statements.go:8). + +There is no PG18 JIT column change relative to PG17 in the release notes consulted — PG17 and +PG18 share the same JIT column set. So a two-branch selector (PG15/16 base; PG17+ adds +deform) covers PG15-18. + +--- + +## 2. Reference sub-screen pattern (`internal/query/statements.go`) + +File: `/home/lesovsky/Git/github.com/lesovsky/pgcenter/internal/query/statements.go` + +### Const naming convention + +`PgStatStatements` where suffix is `Default` (covers all / latest), +`PG13`, or `PG12`. Examples in file: +- `PgStatStatementsTimingPG12` (statements.go:21) — PG ≤12 +- `PgStatStatementsTimingPG13` (statements.go:8) — PG 13–16 +- `PgStatStatementsTimingDefault` (statements.go:34) — PG 17+ +- `PgStatStatementsGeneralDefault`, `PgStatStatementsIoDefault`, `PgStatStatementsTempDefault`, + `PgStatStatementsLocalDefault`, `PgStatStatementsWalDefault` — single-version modes. + +Plan: add `PgStatStatementsJITDefault` (PG15/16 base set) and +`PgStatStatementsJITPG17` (adds deform columns), or invert (`...PG15` base + `...Default` +for PG17+). Match the timing precedent which uses `Default` for the newest shape: +`PgStatStatementsJITPG15` (base) + `PgStatStatementsJITDefault` (PG17+). + +### SELECT shape (canonical pgss row layout) + +Every pgss "diffable" mode query follows this exact column order (see `PgStatStatementsIoDefault`, +statements.go:56, the closest analog): + +``` +pg_get_userbyid(p.userid) AS user, -- col 0 (UniqueKey component, displayed) +d.datname AS database, -- col 1 + AS *_total / "*_total,unit", -- cumulative snapshot (shown, not diffed) + AS * / "*,unit", -- DIFFED columns (DiffIntvl range) +p.calls AS calls, -- col after interval block +left(md5(p.userid::text || p.dbid::text || p.queryid::text), 10) AS queryid, -- synthetic key +regexp_replace({{.PgSSQueryLenFn}}, E'\\s+', ' ', 'g') AS query -- last col +FROM {{.PGSSSchema}}.pg_stat_statements p JOIN pg_database d ON d.oid=p.dbid +``` + +`statements_io` column count breakdown (Ncols=13): user(0), database(1), 4 totals(2-5), +4 intervals(6-9), calls(10), queryid(11), query(12). `DiffIntvl: [2]int{6,10}` (view.go:221) +— note the diff range upper bound is `calls` index (10), i.e. it spans the 4 interval +columns 6-9 plus `calls` at 10. (`statements_wal` uses `DiffIntvl {3,6}` with only 2 interval +cols + records/fpi + calls; layout differs per mode.) + +### Templating tokens + +- `{{.PGSSSchema}}` — schema where pg_stat_statements is installed (`p.ExtPGSSSchema`). +- `{{.PgSSQueryLenFn}}` — expands to a query-text expression honoring the configured query + length (e.g. `left(p.query, N)` or `p.query`). Resolved by `query.Format(tmpl, opts)`. +- Both are resolved in `view.Views.Configure()` → `query.Format()` (view.go:395-402). + +The `Default` (PG17+) templates use a raw backtick string with `'\s+'` (single backslash); +the older concatenated `+ "..."` style uses `E'\\s+'`. Either works — match neighbours. + +### Version-branch selector functions + +Two selectors live at the bottom of statements.go: +- `SelectStatStatementsTimingQuery(version int) string` (statements.go:306) — branches: + `version < 130000` → PG12; `version >= 170000` → Default; else → PG13. +- `SelectQueryReportQuery(version int) string` (statements.go:318) — same 3-way branch. + +Note: `statements_general`, `statements_io`, `statements_temp`, `statements_local`, +`statements_wal` have NO selector — they use a single `*Default` const wired directly as +`QueryTmpl` in view.go and are never re-selected in `Configure()`. Only `statements_timings` +goes through a selector (view.go:373-375). + +### Plan for `SelectStatStatementsJITQuery` + +```go +// SelectStatStatementsJITQuery returns proper statements_jit query depending on Postgres version. +func SelectStatStatementsJITQuery(version int) string { + switch { + case version >= 170000: + return PgStatStatementsJITDefault // base 8 + jit_deform_count/time + default: + return PgStatStatementsJITPG15 // base 8 columns (PG15/16) + } +} +``` + +The view's `MinRequiredVersion: query.PostgresV15` gate (see §3) means this selector is only +ever called with version ≥ 150000, so the two-way branch is sufficient — no PG<15 case +needed. `PostgresV15 = 150000` is confirmed present (query.go:18); also `PostgresV16`, +`PostgresV17`, `PostgresV18` all exist (query.go:19-21). + +Wire it in `view.Views.Configure()` next to the timings case (view.go:373): +```go +case "statements_jit": + view.QueryTmpl, view.Ncols, view.DiffIntvl = query.SelectStatStatementsJITQuery(opts.Version) + v[k] = view +``` +Because Ncols/DiffIntvl differ between PG15/16 (8 JIT metrics) and PG17+ (10 JIT metrics), +the selector should ALSO return Ncols and DiffIntvl (like `SelectStatWALQuery`, +`SelectStatIOQuery` which return `(string, int, [2]int)`), not just the string like the +timing selector. This is the cleanest fit — see §3 for the UniqueKey constraint that forces +two different column SETs rather than hiding columns. + +--- + +## 3. View registration (`internal/view/view.go`) + +File: `/home/lesovsky/Git/github.com/lesovsky/pgcenter/internal/view/view.go` + +### `View` struct fields (view.go:9-31) + +`Name, MinRequiredVersion, QueryTmpl, Query, DiffIntvl [2]int, Cols []string, Ncols int, +OrderKey int, OrderDesc bool, UniqueKey int, ColsWidth map[int]int, Aligned bool, Msg string, +Filters map[int]*regexp.Regexp, Refresh, ShowExtra, CollectExtra, IOAvailable, +DelayAcctAvailable, NotRecordable bool`. + +### `statements_io` entry (view.go:218-229) — primary template to copy + +```go +"statements_io": { + Name: "statements_io", + QueryTmpl: query.PgStatStatementsIoDefault, + DiffIntvl: [2]int{6, 10}, + Ncols: 13, + OrderKey: 0, + OrderDesc: true, + UniqueKey: 11, + ColsWidth: map[int]int{}, + Msg: "Show statements IO statistics", + Filters: map[int]*regexp.Regexp{}, +}, +``` + +### `statements_wal` entry (view.go:254-266) — has MinRequiredVersion + NotRecordable precedent + +```go +"statements_wal": { + Name: "statements_wal", + MinRequiredVersion: query.PostgresV13, + QueryTmpl: query.PgStatStatementsWalDefault, + DiffIntvl: [2]int{3, 6}, + Ncols: 9, + OrderKey: 0, + OrderDesc: true, + UniqueKey: 7, + ColsWidth: map[int]int{}, + Msg: "Show statements WAL statistics", + Filters: map[int]*regexp.Regexp{}, +}, +``` + +(`statements_wal` is NOT NotRecordable — it predates the TUI-first principle. Our new view +MUST set `NotRecordable: true`; the closest NotRecordable+MinRequiredVersion+UniqueKey +precedent is `stat_io` view.go:166-179.) + +### Field meanings + +- **`DiffIntvl [2]int`** — `[start, end]` inclusive column-index range that the diff engine + treats as deltas (per-interval values). Picks the "interval" block of the SELECT (and + often includes `calls`). `statements_io` `{6,10}` covers interval cols 6-9 + calls 10. +- **`UniqueKey int`** — index of the synthetic `md5(...) queryid` column used to match rows + across diff snapshots. For pgss it is the `queryid` column (`statements_io` → 11). The + `user`/`database` text columns are NOT the unique key — the md5 is, because the same query + appears once per (user,db) and the md5 folds all three. +- **`Ncols`** — total columns returned; right border for `OrderKey` wrap (config_view.go:37). +- **`OrderKey` / `OrderDesc`** — initial sort column / descending. pgss screens use + `OrderKey: 0` (user) DESC. (stat_io uses `OrderKey: 4`, the first diffed counter.) +- **`ColsWidth: map[int]int{}`**, **`Filters: map[int]*regexp.Regexp{}`** — initialized empty. +- **`Cols`** — populated at runtime from the result header (top/stat.go:314), not set here. + +### Synthetic md5 queryid → columns cannot be hidden per version + +The `UniqueKey` points at the md5 `queryid` column whose index is fixed by the SELECT column +count. The diff/align machinery enforces a minimum column width of 8 on the md5 (10-char +hash) and treats the layout positionally. Therefore you cannot hide a column for an older PG +version while keeping the same query — the indices (and hence UniqueKey/DiffIntvl/Ncols) +would shift. The established pattern is to ship a DIFFERENT query (different column SET) per +version and have the selector return matching `Ncols`/`DiffIntvl`/`UniqueKey`. + +How other pgss screens differ across versions: only `statements_timings` differs across +versions (PG12 vs PG13 vs PG17+), and it does so by swapping the WHOLE query +(`SelectStatStatementsTimingQuery`) — NOT by hiding columns. Its column COUNT happens to stay +constant (13) across those variants, so the static `Ncols/DiffIntvl/UniqueKey` in view.go +(view.go:197-201) remain valid and `Configure()` only swaps `QueryTmpl`. For JIT the column +count DOES change (8 vs 10 JIT metrics → ~Ncols differs), so the selector must also return +`Ncols`/`DiffIntvl` (and UniqueKey must be recomputed). This is exactly the `stat_io` model +(`SelectStatIOQuery` returns `(string, int, [2]int)`, view.go:386) — follow it, and set the +view's static `UniqueKey: 0`-style placeholder while the Configure path patches the rest. + +Recommended JIT layouts (proposal, with `user`=0, `database`=1): + +PG15/16 (8 JIT metrics, counts+times). Suggested compact set: show the 4 count metrics + +4 time metrics as interval columns. Mirroring statements_io's total+interval doubling would +make 8 totals + 8 intervals = very wide; given no horizontal scroll (see §7), prefer a +single interval block (like the timings `,ms` columns) rather than total+interval doubling. +Final column set is a tech-spec decision — but keep UniqueKey aligned to the md5 column index +and DiffIntvl spanning the count/time block + calls. + +--- + +## 4. The two count-tests that break when adding a view + +### (a) `internal/view/view_test.go::TestNew` (view_test.go:9-12) + +```go +func TestNew(t *testing.T) { + v := New() + assert.Equal(t, 26, len(v)) // 26 is the total number of views have to be returned +} +``` +**Fix:** bump `26` → `27` and update the comment. Adding `statements_jit` to `view.New()` +raises the map size by one. + +### (b) `record/record_test.go::Test_filterViews` (record_test.go:101-136) + +```go +testcases := []struct { + version int + pgssSchema string + wantN int // filtered-out count + wantV int // remaining count +}{ + {version: 140000, pgssSchema: "", wantN: 10, wantV: 16}, + {version: 140000, pgssSchema: "public", wantN: 4, wantV: 22}, + {version: 130000, pgssSchema: "public", wantN: 7, wantV: 19}, + {version: 120000, pgssSchema: "public", wantN: 10, wantV: 16}, + {version: 110000, pgssSchema: "public", wantN: 12, wantV: 14}, + {version: 100000, pgssSchema: "public", wantN: 12, wantV: 14}, +} +``` +The new view is `MinRequiredVersion: query.PostgresV15` (150000) AND `NotRecordable: true`. +In `filterViews` (record.go:200-233) the `NotRecordable` branch (record.go:208) fires BEFORE +the version gate — so on EVERY test row the new view counts as filtered-out: `wantN += 1`, +`wantV` unchanged (it never joins the remaining set), regardless of version or pgssSchema. + +**Fix — bump every row's `wantN` by exactly 1** (`wantV` stays the same on all 6 rows): + +| version | pgssSchema | wantN old → new | wantV | +|---------|-----------|-----------------|-------| +| 140000 | "" | 10 → 11 | 16 | +| 140000 | public | 4 → 5 | 22 | +| 130000 | public | 7 → 8 | 19 | +| 120000 | public | 10 → 11 | 16 | +| 110000 | public | 12 → 13 | 14 | +| 100000 | public | 12 → 13 | 14 | + +There is NO separate name-map that must gain an entry in `Test_filterViews`. The other +`record_test.go` tests (`TestFilterViews_NotRecordable`, `TestFilterViews_dropsExplicitNotRecordable`) +build their own ad-hoc `view.Views{}` literals and are NOT affected. + +Note: `Test_filterViews` cases stop at version 140000 (no 150000 row), but because the +`NotRecordable` drop is version-independent, the +1 applies uniformly anyway. (If a 150000+ +row were added later it would also be +1.) + +--- + +## 5. Menu + cycle wiring + +### (a) Uppercase menu `X` — `top/menu.go` + +File: `/home/lesovsky/Git/github.com/lesovsky/pgcenter/top/menu.go` + +`menuPgss` items list (`selectMenuStyle`, menu.go:53-60) currently has 6 entries (indices +0-5). **Add a 7th**, index 6: +```go +" pg_stat_statements WAL usage", +" pg_stat_statements JIT compilation", // NEW index 6 +``` + +`menuSelect` `case menuPgss` switch (menu.go:155-172) currently handles cy 0-5 + default. +**Add `case 6`** before `default`: +```go +case 5: + viewSwitchHandler(app.config, "statements_wal") +case 6: + viewSwitchHandler(app.config, "statements_jit") // NEW +default: + viewSwitchHandler(app.config, "statements_timings") +``` + +The `X` keybinding already opens this menu: `{"sysstat", 'X', menuOpen(menuPgss, app.config, +app.postgresProps.ExtPGSSSchema)}` (keybindings.go:45). The menu height auto-sizes to +`len(s.items)` (menu.go:115) — no manual height edit needed. + +### (b) Lowercase cycle `x` — `top/config_view.go` + +File: `/home/lesovsky/Git/github.com/lesovsky/pgcenter/top/config_view.go` + +`statementsNextView` (config_view.go:160-180) currently cycles +`...wal → timings`. **Insert `statements_jit`** between wal and timings: +```go +case "statements_wal": + next = "statements_jit" // CHANGED (was statements_timings) +case "statements_jit": // NEW + next = "statements_timings" +default: + next = "statements_timings" +``` + +The `x` keybinding: `{"sysstat", 'x', switchViewTo(app, "statements")}` (keybindings.go:40). +`switchViewTo` → `statementsNextView(app.config.view.Name)` (config_view.go:115). It already +guards `ExtPGSSSchema == ""` (config_view.go:105). No keybindings.go edit required for `x`. + +### (c) Version-aware availability note + +`statements_jit` has `MinRequiredVersion: query.PostgresV15`. On PG<15 the view is filtered +out of the recordable set, but in the live TUI the menu/cycle target it unconditionally. Check +how `viewSwitchHandler` (config_view.go:206-210) behaves when switching to a view absent from +`config.views` on PG<15 — confirm `view.New()` / the top-level view filter (top/top.go setup) +removes sub-PG15 views from `config.views`, otherwise switching to `statements_jit` on PG14 +yields a zero-value `View{}`. The tech-spec must verify the TUI's view-availability filter +(separate from record's `filterViews`) drops `statements_jit` on PG<15 and that the menu/cycle +degrade gracefully (skip it). Search target: where `top` builds `config.views` and applies +`VersionOK`. + +--- + +## 6. NotRecordable mechanism + +- Field: `NotRecordable bool` on `view.View` (view.go:30): "When true, + record/record.go:filterViews() skips this view." +- Enforcement: `record/record.go:filterViews()` (record.go:200), the `if v.NotRecordable` + branch at record.go:208 — `delete(views, k); filtered++; continue`. Fires BEFORE the + version gate (record.go:214) and the pgss-schema gate (record.go:221). +- Precedent setters (all `NotRecordable: true`): + - `bgwriter` (view.go:151) — feature 004. + - `replslots` (view.go:164) — feature 005. + - `stat_io` (view.go:178), `stat_io_time` (view.go:192) — feature 006. +- `statements_*` views are normally recordable (none set the flag) — our new + `statements_jit` MUST set `NotRecordable: true` per the 0.11.0 TUI-first principle + (`docs/roadmap-0.11.0.md:86-92`, ADR `[004-feat-bgwriter-checkpointer]` + `docs/decisions-log.md:223-233`). +- **report.go**: `doDescribe` (report/report.go:604) maps view-name → description text + (report.go:607+; includes `statements_io`, `statements_wal`, etc.). NotRecordable views + are not recorded, so there is no recorded data to report — `statements_jit` does NOT need a + description entry here. Confirm by precedent: bgwriter/replslots/stat_io (all NotRecordable) + have NO entry in this map. Skip the report.go description for the JIT view. + +--- + +## 7. jit=off / zero-JIT row filtering + +- **`statements_io` / `statements_wal` / all current pgss screens do NOT filter zero rows.** + Their SELECTs end at `FROM {{.PGSSSchema}}.pg_stat_statements p JOIN pg_database d ON + d.oid=p.dbid` with NO `WHERE`/`HAVING` (statements.go:67, 98). Every statement is shown. +- **`pg_stat_io` (feature 006) DOES filter all-zero rows** via a count-based `WHERE`: + `... WHERE coalesce(reads,0)+coalesce(writes,0)+...+coalesce(fsyncs,0) > 0` + (io.go:35, io.go:58, io.go:79). The comment (io.go:17-19) explains the count-based WHERE + keeps the screen compact and makes the count and time sub-screens share an identical + row-set. This was ADR-backed (decisions-log §[006...]). +- **Recommendation for JIT:** filter to rows with actual JIT activity. With `jit=on` (default) + the vast majority of normalized statements never trigger JIT (only large-cost plans do), so + WITHOUT a filter the screen would be dominated by all-zero rows. Add a count-based + `WHERE jit_functions > 0` (or + `WHERE coalesce(jit_functions,0)+coalesce(jit_inlining_count,0)+coalesce(jit_optimization_count,0)+coalesce(jit_emission_count,0) > 0`) + following the `pg_stat_io` precedent. This also gracefully handles `jit=off` (all-zero → + empty screen) — consider an optional cmdline hint "no JIT activity (jit=off?)" when empty, + matching the interview's "optional jit=off hint" open question. Using only `jit_functions + > 0` is simplest and sufficient: a statement with any JIT work always has + `jit_functions > 0`. This is a tech-spec decision but the strong recommendation is: FILTER. + +--- + +## 8. Exact files to touch (with current line anchors) + +| File | Change | Anchor | +|------|--------|--------| +| `internal/query/statements.go` | Add `PgStatStatementsJITPG15` + `PgStatStatementsJITDefault` consts; add `SelectStatStatementsJITQuery(version) (string, int, [2]int)` selector | consts block ends ~statements.go:303; selectors at statements.go:305-327 | +| `internal/view/view.go` | Add `"statements_jit"` View entry (MinRequiredVersion PostgresV15, NotRecordable true, UniqueKey=md5 idx); add `case "statements_jit":` in `Configure()` | view entries view.go:194-266; Configure switch view.go:373-391 | +| `top/menu.go` | Add 7th `menuPgss` item (index 6); add `case 6` in `menuSelect`/`case menuPgss` | items menu.go:53-60; switch menu.go:156-171 | +| `top/config_view.go` | Insert `statements_jit` into `statementsNextView` cycle (wal→jit→timings) | config_view.go:160-180 | +| `top/keybindings.go` | NO CHANGE (`x` at keybindings.go:40 and `X` at keybindings.go:45 already wired) | — | +| `report/report.go` | NO CHANGE (NotRecordable → no description entry; matches bgwriter/replslots/stat_io) | doDescribe map report.go:604-624 | + +### Test files to touch + +| File | Change | Anchor | +|------|--------|--------| +| `internal/view/view_test.go` | Bump `TestNew` count `26 → 27`; optionally add `TestNew_StatementsJITView` guard (mirror `TestNew_StatIOView` view_test.go:17) | view_test.go:9-12 | +| `record/record_test.go` | Bump `Test_filterViews` `wantN` +1 on all 6 rows (table in §4) | record_test.go:123-128 | +| `internal/query/statements_test.go` | Add `TestSelectStatStatementsJITQuery` (mirror `TestSelectStatStatementsTimingQuery` statements_test.go:10) + add JIT exec sub-test loop over versions 150000-180000 (mirror `Test_StatStatementsQueries` statements_test.go:34, gated PG15+ like the WAL PG13+ loop at statements_test.go:86) | statements_test.go:10-103 | + +### PG version constants (confirmed present, `internal/query/query.go`) + +`PostgresV13=130000` (query.go:16), `PostgresV14=140000` (17), `PostgresV15=150000` (18), +`PostgresV16=160000` (19), `PostgresV17=170000` (20), `PostgresV18=180000` (21). Use +`query.PostgresV15` for the gate. + +--- + +## 9. Integration points & runtime data flow + +- `view.Views.Configure(opts query.Options)` (view.go:357) runs the per-view selector switch + then `query.Format(view.QueryTmpl, opts)` for every view (view.go:395). `opts.Version` + drives JIT branch; `opts.ExtPGSSSchema` / query-len feed `{{.PGSSSchema}}` / + `{{.PgSSQueryLenFn}}`. +- `top/stat.go:alignViewToResult` (stat.go:303) populates `view.Cols` from the live result + header (stat.go:314) and computes `ColsWidth` — JIT columns are auto-aligned; the md5 + `queryid` min width is enforced there. +- Diff/ordering uses `DiffIntvl`, `UniqueKey`, `OrderKey`, `Ncols` — all must be internally + consistent with the returned column count (see §3). + +--- + +## 10. Potential problems / constraints + +1. **No horizontal column scroll.** Columns past terminal width are silently truncated + (decisions-log §[006...] context, decisions-log:355). With 10 JIT metrics on PG17+ plus + user/database/calls/queryid/query, a total+interval doubling (like statements_io) would be + far too wide. Prefer a SINGLE interval block (counts + times), like the timings `,ms` + columns. This is the main column-design constraint for the tech-spec. + +2. **Ncols changes across versions (8 vs 10 JIT metrics).** Because of the synthetic md5 + `UniqueKey` (§3), the JIT selector MUST return `(string, Ncols, DiffIntvl)` and the + Configure case must patch all three (follow `SelectStatIOQuery` model, view.go:386 — + NOT the timings model which keeps Ncols constant). Getting Ncols/UniqueKey/DiffIntvl out + of sync with the actual column count is the highest-risk bug; covered only by an exec test + against a real PG (CI matrix PG15-18) — local runs without PG skip these (`t.Skipf`, + statements_test.go:53). + +3. **Count-test breakage masked locally.** `TestNew` and `Test_filterViews` (§4) are the two + count assertions that will fail. They run without a PG instance, so `make test` catches + them locally — but they are easy to forget. Both are pure integer bumps. + +4. **PG<15 TUI view availability.** The record-side `filterViews` handles PG<15 via + `MinRequiredVersion`, but the live TUI menu/cycle (§5c) must also drop `statements_jit` on + PG<15. Verify the `top` view-availability filter (separate from record's `filterViews`) + removes sub-PG15 views from `config.views`; otherwise selecting JIT on PG14 yields a + zero-value `View{}`. This is the one item needing live verification in the tech-spec. + +5. **jit=off / no-activity empty screen.** With the recommended `WHERE jit_functions > 0` + filter, `jit=off` and low-activity systems show an empty screen. Decide whether to surface + an explanatory cmdline hint (interview open question). Not a blocker. + +### Tech debt / ADRs in scope + +- `docs/tech-debt.md` Active Debt: no items touch statements.go / view.go / menu.go / + config_view.go (grep found none) — no debt to flag. +- ADRs that constrain this feature (settled — do not re-litigate): + - `[004-feat-bgwriter-checkpointer]` (decisions-log:223) — TUI-first / `NotRecordable: true` + rationale. Apply directly: set `NotRecordable: true`, no record/report wiring. + - `[006-feat-pg-stat-io]` synthetic-md5-key + count-based zero-row WHERE (decisions-log:355, + 373) — direct precedent for the JIT `WHERE jit_functions > 0` filter and the + UniqueKey-on-md5 layout. + +--- + +## Summary + +- **JIT schema (confirmed vs PG official docs):** PG15/16 base = 8 columns — + `jit_functions`(bigint), `jit_generation_time`(double), `jit_inlining_count`(bigint), + `jit_inlining_time`(double), `jit_optimization_count`(bigint), `jit_optimization_time`(double), + `jit_emission_count`(bigint), `jit_emission_time`(double). PG17+ adds 2 — + `jit_deform_count`(bigint), `jit_deform_time`(double). All `*_count`/`jit_functions` are + cumulative bigint counts (diffable); all `*_time` are cumulative double-precision ms + (round + diff, like the timing screen). PG18 == PG17 column set. +- **Version-branch plan:** two query consts (PG15 base / PG17+ Default) + selector + `SelectStatStatementsJITQuery(version) (string, int, [2]int)` returning query + Ncols + + DiffIntvl (stat_io model, because Ncols differs); gate `MinRequiredVersion: + query.PostgresV15`; two-way branch (≥170000 → Default, else base) is sufficient. +- **Two count-test fixes:** `view_test.go::TestNew` `26 → 27`; `record_test.go::Test_filterViews` + `wantN +1` on all 6 rows (wantV unchanged), because NotRecordable drops it on every version. +- **Zero-row filtering:** RECOMMEND filtering — add `WHERE jit_functions > 0` (pg_stat_io + precedent). Current statements_io/wal do NOT filter, but JIT rows are overwhelmingly all-zero + under default `jit=on`, so an unfiltered JIT screen would be near-useless; filtering also + cleanly handles `jit=off`. diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-completeness.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-completeness.json new file mode 100644 index 0000000..181e925 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-completeness.json @@ -0,0 +1,29 @@ +{ + "status": "pass", + "sources": { + "user_spec": true, + "tech_spec": true, + "tasks": true + }, + "requirements_total": 14, + "requirements_covered": 14, + "requirements_partial": 0, + "requirements_missing": 0, + "findings": [ + { + "type": "underengineering", + "source": "tech-spec", + "requirement": "US-9: PG<15 sub-screen unavailable, app does not crash, menu/cycle degrade gracefully", + "detail": "User-spec acceptance criterion explicitly requires that menu/cycle do not return an empty View{} on PG<15 (Risk 3 + AC line 138). The tech-spec relies on the runtime VersionOK guard (stat.go:199, verified present) which returns 'not supported' instead of querying, and notes the view is always in the map so no zero-value View{} occurs. However, code-research §5c flags an OPEN verification item: the menu/cycle on PG<15 target statements_jit unconditionally, and it was never confirmed in the tech-spec that the TUI view-availability filter (separate from record's filterViews) actually drops the view from the menu/cycle, or whether the user simply gets the 'not supported' message after selecting it. The tech-spec resolves this as 'graceful degrade via VersionOK' but does not specify whether the menu still SHOWS the JIT item on PG14 (cosmetic, but the user-spec says menu/cycle should degrade). This is a minor gap between the user-spec's 'degrade gracefully (skip it)' wording and the tech-spec's 'show item, block at query time' resolution.", + "severity": "minor" + }, + { + "type": "underengineering", + "source": "tech-spec", + "requirement": "Ncols/DiffIntvl/UniqueKey consistency with real PG17 15-column branch", + "detail": "The highest-risk correctness concern (selector layout metadata out of sync with real columns, esp. the PG17 deform branch) is acknowledged in the Risks table and mitigated by the selector returning all four values explicitly + a per-version unit test + the gated exec test on the CI PG15-18 matrix. This is adequate coverage. Noting only that the exec test (the single place where layout-vs-real-columns is validated) is skipped locally without PG and runs only in CI; there is no local safeguard against an Ncols/DiffIntvl typo for the PG17 branch beyond manual PG17 testing. Acceptable for an S feature with the documented mitigation, but the dependency on CI for the core correctness check is the thinnest part of the test strategy.", + "severity": "minor" + } + ], + "summary": "PASS. 14/14 user-spec acceptance criteria map to tech-spec tasks (Task 1: version selector + WHERE jit_functions>0 + version branches; Task 2: view entry MinRequiredVersion PostgresV15, NotRecordable, OrderKey gen_total desc, Msg hint, Configure patch, count-test fixes; Task 3: 7th menu item + x-cycle wal→jit→timings; Task 4: QA). No gaps, no scope creep — every tech-spec decision (selector layout metadata, single interval block, jit_functions>0 filter, static Msg hint, NotRecordable) traces directly to a user-spec requirement or constraint, and all are explicitly framed as following established ADRs ([004], [006]). No overengineering: two-branch selector (not N-way), no abstractions, *_count metrics deliberately dropped to fit width (YAGNI-respecting), no record/report wiring (deferred per TUI-first principle). No critical underengineering: error handling is the existing VersionOK runtime guard (verified at stat.go:199), empty/jit=off state handled by SQL filter + Msg, no user input beyond existing keybindings, read-only consumer of pg_stat_statements so no write/concurrency concerns. Two minor findings: (1) PG<15 menu/cycle degradation resolved as 'show item, block at query time' vs user-spec 'skip it' wording; (2) core Ncols/DiffIntvl/UniqueKey-vs-real-columns check runs only in CI. Both mitigated and acceptable for an S feature. Tech-spec Decisions section is self-contained (5 decisions with rationale + alternatives); no decision-level content scattered in tasks (no structural_gap). Solution section has real substance (3-layer architecture, named files/functions, specific PG version branching) — not shallow." +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-decisions.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-decisions.md new file mode 100644 index 0000000..bd1b183 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-decisions.md @@ -0,0 +1,102 @@ +# Decisions Log: pg_stat_statements JIT screen + +Отчёты агентов о выполнении задач. Каждая запись создаётся агентом, выполнившим задачу. + +--- + +## Task 01: JIT query consts + version selector — internal/query/statements.go + +**Status:** Done +**Commit:** 79514ea +**Agent:** jit-query-dev (general-purpose) +**Summary:** Добавлены две SQL-константы по образцу `PgStatStatementsTimingPG13`/`PgStatStatementsIoDefault` — `PgStatStatementsJITPG15` (PG15/16, 13 колонок) и `PgStatStatementsJITDefault` (PG17+, 15 колонок, +`deform_total`/`deform,ms` через `jit_deform_time`); `*_total` через `date_trunc('seconds', round(p.jit_*_time)/1000 * '1 second'::interval)::text`, `*_ms` через `round(p.jit_*_time)`, `functions` = `p.jit_functions`, md5-`queryid`, оба заканчиваются `WHERE p.jit_functions > 0` (Decision 3). Добавлен селектор `SelectStatStatementsJITQuery(version int) (string, int, [2]int, int)` (4-tuple по модели `SelectStatIOQuery` + `UniqueKey`): `>= PostgresV17` → `(Default, 15, {7,12}, 13)`, иначе → `(PG15, 13, {6,10}, 11)`. Покрыто unit-тестом обеих веток + JIT exec-подтест gated PG15+ (`t.Skipf` без PG). +**Deviations:** Нет. Колоночные алиасы, индексы, токены и стиль строки точно соответствуют tech-spec (Decision 2) и образцовым timing/io-константам. +**Tech debt:** Нет. Двойная константа (отличаются только deform-колонками) — намеренная по позиционному align (ADR [006]), флаг code-reviewer'а только чтобы будущий рефактор случайно не слил их в одну строку. Live exec-пути PG15–18 локально не исполнялись (нет тестового PG-кластера) — гейтятся CI-матрицей; подтверждено `--- SKIP` на всех четырёх версиях. + +**Reviews:** + +*Round 1:* +- dev-code-reviewer: approved, 0 critical/major, 1 minor (намеренное дублирование консты — не менять) → [task-01-dev-code-reviewer-review.json] +- dev-security-auditor: approved, 0 findings (статические шаблоны над server-controlled токенами, нет injection-поверхности) → [task-01-dev-security-auditor-review.json] +- dev-test-reviewer: passed, 0 critical/major, 1 minor (нет проверки границы PG<15 — by design, ветка отсутствует) → [task-01-dev-test-reviewer-review.json] + +**Verification:** +- `go test ./internal/query/...` → ok (JIT exec-подтесты `t.Skipf` — PG-кластер недоступен локально; `TestSelectStatStatementsJITQuery` зелёный) +- `go build ./...` → clean; `gofmt -l` → clean; `go vet ./internal/query/...` → clean +- Commit 79514ea: 2 файла, +81 строка + +--- + +## Task 02: Register statements_jit view + Configure + count-test fixes — internal/view/view.go + +**Status:** Done +**Commit:** f7defde (view + Configure + count-tests), 304ee62 (review round 1 fix), 5028374 (review reports) +**Agent:** jit-view-dev (general-purpose) +**Summary:** В `view.New()` добавлена запись `statements_jit` по образцу `statements_io` с прецедентом `stat_io` для `MinRequiredVersion: query.PostgresV15` + `NotRecordable: true`: PG15-дефолты `QueryTmpl: query.PgStatStatementsJITPG15`, `Ncols: 13`, `DiffIntvl: {6,10}`, `UniqueKey: 11`, `OrderKey: 2` (gen_total), `OrderDesc: true`, `Msg` с подсказкой про `jit=off` (Decision 4), пустые `ColsWidth`/`Filters`. В `Configure()` добавлен `case "statements_jit":`, патчащий все 4 поля (`QueryTmpl`/`Ncols`/`DiffIntvl`/`UniqueKey`) из `query.SelectStatStatementsJITQuery(opts.Version)` — расширение модели `stat_io` (3 поля) до 4 с `UniqueKey`, т.к. md5-ключ JIT стоит в конце и сдвигается с `Ncols`. Поправлены три count-теста (TDD-якорь): `TestNew` 26→27, `TestView_VersionOK` строка 160000 26→27 (строки ≤140000 без изменений — `VersionOK(<150000)==false`), `Test_filterViews` `wantN +1` на всех 6 строках (`NotRecordable` отбрасывает view ДО версионного гейта, `wantV` неизменно). Добавлен guard-тест `TestNew_StatementsJITView` по образцу `TestNew_StatIOView`. `report/report.go` не тронут (NotRecordable → нет записи описания). +**Deviations:** Нет. Все Acceptance Criteria выполнены точно по task-02-спеку; `report.go` подтверждённо не изменён vs wave-1 база. +**Tech debt:** Нет. + +**Reviews:** + +*Round 1:* +- dev-code-reviewer: approved, 0 critical/major/minor (cross-file consistency: сигнатура 4-tuple селектора, PG15-дефолты, порядок NotRecordable→версионный гейт, три count-теста — всё подтверждено) → [task-02-dev-code-reviewer-review.json] +- dev-security-auditor: approved, 0 findings (нет injection-поверхности — только присваивание возврата чистой integer-switch функции; md5 — синтетический ключ отображения, не security-control) → [task-02-dev-security-auditor-review.json] +- dev-test-reviewer: approved, 0 critical/major, 1 minor (усилить проверку `Msg` до `assert.Contains(..., "jit=off")` по прецеденту `TestNew_StatIOTimeView`) → [task-02-dev-test-reviewer-review.json] + +Minor от dev-test-reviewer применён в 304ee62 (`assert.Contains(t, jit.Msg, "jit=off")` пинит load-bearing роль `Msg` из Decision 4). + +**Verification:** +- `go test ./internal/view/...` → ok; `go test ./record/...` (filterViews-тесты) → ok +- `go build ./...` → clean; `go vet ./internal/view/... ./record/...` → clean +- Три count-теста зелёные: `TestNew`=27, `Test_filterViews` (+1 на 6 строках), `TestView_VersionOK` (160000→27) +- `report/report.go` не изменён vs wave-1 база 8631ffd (git diff пуст) +- Примечание: live-DB тесты (`Test_app_record` и пр.) падают с connection-refused/nil-pointer — это pre-existing, требуют живого PG, воспроизводятся на HEAD~1; вне scope Task 02 + +--- + +## Task 03: TUI menu item + x-cycle wiring — top/menu.go, top/config_view.go + +**Status:** Done +**Commit:** 1d8d678 (menu item + handler + x-cycle), 217b69d (review round 1 fix — обновление существующих regression-тестов) +**Agent:** jit-tui-dev (general-purpose) +**Summary:** Чистая TUI-обвязка нового view `statements_jit` (зарегистрирован в Task 02) через два штатных входа pgss. В `top/menu.go` `selectMenuStyle` `case menuPgss` добавлен 7-й пункт (индекс 6) `" pg_stat_statements JIT compilation"` сразу после `" pg_stat_statements WAL usage"` (leading-space-формат как у соседей; высота меню авто-размер от `len(s.items)` — геометрия не тронута). В `menuSelect` `case menuPgss` добавлен `case 6: viewSwitchHandler(app.config, "statements_jit")` строго между `case 5` и `default`. В `top/config_view.go` `statementsNextView` x-цикл расширен: `case "statements_wal"` теперь `next = "statements_jit"`, добавлен `case "statements_jit": next = "statements_timings"` — порядок цикла стал `… local → wal → jit → timings …`. `top/keybindings.go` не тронут (`x`/`X` уже подключены и уже гейтят пустой `ExtPGSSSchema`). +**Deviations:** TDD-якорь task-спека («у этого TUI-слоя нет существующего unit-покрытия, тесты не писать») оказался ФАКТИЧЕСКИ НЕВЕРНЫМ — найдено dev-test-reviewer (round 1, critical, подтверждено эмпирически `go test ./top/`). Слой покрыт table-driven тестами: `Test_selectMenuStyle` (счётчик пунктов `menuPgss`), `Test_statementsNextView` и `Test_switchViewTo` (x-цикл, включая переход `statements_wal`). Мои изменения сломали 3 существующих теста. Отклонение от плана: вместо «no tests» обновил три устаревших assertion'а под новое поведение (НЕ спекулятивные новые тесты — правка существующих regression-тестов): `menuPgss` count 6→7; `statements_wal → statements_jit` + новый `statements_jit → statements_timings` в обоих cycle-тестах. Это минимальная правка, восстанавливающая зелёный `make test` (CI-гейт проекта), и она же закрывает покрытие нового JIT-перехода. Причина расхождения в спеке зафиксирована для lead'а (TDD Anchor task-03 нуждается в исправлении). +**Tech debt:** Нет. Pre-existing: `Test_doReload` падает nil-pointer на `top/reload.go:18` — требует живого PostgreSQL, воспроизводится идентично на HEAD~2 (чистый worktree), вне scope Task 03. + +**Reviews:** + +*Round 1:* +- dev-code-reviewer: approved, 0 critical/major/minor (cross-file consistency: индекс пункта 6 совпадает с `case 6`, view `statements_jit` зарегистрирован в Task 02 → `viewSwitchHandler` резолвит реальный view, не zero-value; геометрия меню не тронута) → [task-03-dev-code-reviewer-review.json] +- dev-test-reviewer: **failed**, 2 critical + 1 major — устаревший TDD-якорь, сломаны 3 существующих теста (`Test_selectMenuStyle`, `Test_statementsNextView`, `Test_switchViewTo`) → [task-03-dev-test-reviewer-review.json] + +Critical-замечания dev-test-reviewer применены в 217b69d (обновлены три assertion'а + добавлен новый JIT-переход в оба cycle-теста). + +*Round 2:* +- dev-test-reviewer: **passed**, 0 findings (все 3 регрессии устранены, litmus-целостность сохранена — тесты проверяют реальное имя view/счётчик, а не моки; новый JIT-переход покрыт) → [task-03-dev-test-reviewer-review-round2.json] + +**Verification:** +- `go build ./...` → clean; `go vet ./top/...` → clean +- `go test ./top/ -run 'Test_selectMenuStyle|Test_statementsNextView|Test_switchViewTo'` → все PASS (включая `Test_switchViewTo/14`) +- `make build` → собирает `bin/pgcenter` без ошибок +- Manual (local PG17, верифицирует user): `X` → 7-й пункт «pg_stat_statements JIT compilation», выбор открывает JIT-экран; `x` циклит `… wal → jit → timings …` + +--- + +## Task 04: Pre-deploy QA + +**Status:** Done +**Commit:** (wave-4 chore) +**Agent:** основной агент (team lead) +**Summary:** Прогон pre-deploy QA на ветке feature/pg-stat-statements-jit. Фича-поверхность зелёная: `make build` → bin/pgcenter; `go vet ./...` clean; `gofmt -l` clean; целевые тесты `internal/query` (TestSelectStatStatementsJITQuery), `internal/view` (TestNew=27, TestView_VersionOK 160000→27, TestNew_StatementsJITView), `record` (Test_filterViews), `top` (Test_selectMenuStyle/Test_statementsNextView/Test_switchViewTo) — все PASS. Manual TUI-проверка на локальном PG17 пользователем — OK (меню X 7-й пункт «pg_stat_statements JIT compilation», цикл x wal→jit→timings). +**Deviations:** golangci-lint/gosec не установлены локально — полный lint-гейт делегирован CI. Полный `make test` локально не зелёный из-за пре-существующих live-DB тестов (`Test_app_record`, `Test_doReload`, exec-подтесты PG14-18), которые падают/скипаются без PG-кластера и идентично воспроизводятся на develop — гейтятся CI-матрицей PG14-18 на push. Не регрессия (record.go/reload.go фичей не тронуты). +**Tech debt:** Нет (в рамках задачи). Для постмортема: code-research §5 не выявил тест-покрытие TUI-слоя (Test_selectMenuStyle/Test_statementsNextView/Test_switchViewTo) — из-за чего task-03 ошибочно заявил «нет тест-слоя»; пойман dev-test-reviewer (round 1 failed → round 2 passed). + +**Reviews:** Нет — QA/верификационная задача (reviewers: []). + +**Verification:** +- `make build` → ok; `go vet ./...` → clean; `gofmt -l` → clean +- targeted feature tests (query/view/record/top) → все PASS +- full lint + PG14-18 exec matrix → CI (on push) +- Manual TUI (PG17) → OK (user-confirmed) + +--- diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-execution-plan.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-execution-plan.md new file mode 100644 index 0000000..7170259 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-execution-plan.md @@ -0,0 +1,44 @@ +# Execution Plan: pg_stat_statements JIT screen + +**Создан:** 2026-06-22 +**Branch:** feature/pg-stat-statements-jit + +--- + +Цепочка последовательная (каждая задача оставляет билд зелёным; одна задача на волну). + +## Wave 1 + +### Task 01: JIT query consts + version selector +- **Skill:** code-writing +- **Reviewers:** dev-code-reviewer, dev-security-auditor, dev-test-reviewer +- **Verify:** bash — `go test ./internal/query/...` +- **Files:** internal/query/statements.go, internal/query/statements_test.go + +## Wave 2 (зависит от Wave 1) + +### Task 02: Register statements_jit view + Configure + count-test fixes +- **Skill:** code-writing +- **Reviewers:** dev-code-reviewer, dev-security-auditor, dev-test-reviewer +- **Verify:** bash — `go test ./internal/view/... ./record/...` +- **Files:** internal/view/view.go, internal/view/view_test.go, record/record_test.go + +## Wave 3 (зависит от Wave 2) + +### Task 03: TUI menu item + x-cycle wiring +- **Skill:** code-writing +- **Reviewers:** dev-code-reviewer, dev-test-reviewer +- **Verify:** user — local PG17: `X` → JIT; `x` cycles wal→jit→timings; `make build` +- **Files:** top/menu.go, top/config_view.go + +## Wave 4 (зависит от Wave 1–3) + +### Task 04: Pre-deploy QA +- **Skill:** pre-deploy-qa +- **Reviewers:** none +- **Verify:** bash — `make test && make lint` green; CI matrix PG14–18 green + +## Проверки, требующие участия пользователя + +- [ ] Task 03: пользователь проверяет на локальном PG17 — `X` открывает JIT-под-экран; `x` циклит `wal→jit→timings`; колонки/сортировка/фильтр работают. +- [ ] После всех волн: финальное QA + подтверждение перед merge в develop. diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-interview.yml b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-interview.yml new file mode 100644 index 0000000..0462b4c --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-interview.yml @@ -0,0 +1,149 @@ +# Interview Plan for Feature 007 — pg_stat_statements JIT screen + +interview_metadata: + feature_name: "pg-stat-statements-jit" + feature_base: "docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit" + work_type: "feature" + started: "2026-06-21" + last_updated: "2026-06-21" + status: "completed" + can_resume: true + current_question_num: 0 + feature_size: "S" + +conversation_history: [] + +phase1_feature_overview: + feature_description: + score: 85 + status: "complete" + value: > + New 7th pg_stat_statements sub-screen under the X menu showing JIT compilation + metrics per (user, database, queryid): jit_functions, generation/inlining/ + optimization/emission counts+times (PG15+); PG17 adds deform_count/deform_time. + Built by analogy with statements_io. TUI-only, NotRecordable. Closes release 0.11.0. + gaps: "" + required: true + user_problem: + score: 80 + status: "complete" + value: > + No way in pgcenter to see JIT compilation cost per query. With jit=on (default), + JIT overhead can dominate planning/execution for short queries; a DBA needs to spot + which normalized statements pay heavy generation/optimization/emission time. + gaps: "" + required: true + target_users: + score: 80 + status: "complete" + value: "Practicing PostgreSQL DBA troubleshooting query latency / JIT overhead." + required: false + success_criteria: + score: 60 + status: "partial" + value: "JIT sub-screen reachable via X menu and x-cycle; correct columns per PG version; CI green PG14-18." + gaps: "Confirm exact acceptance criteria and zero-JIT row handling" + required: true + constraints: + score: 85 + status: "complete" + value: > + PG15+ feature (view absent <15). TUI-only, NotRecordable. pgss extension required. + Single query per version (PG15/16 base, PG17+ adds deform_*). Synthetic md5 queryid + UniqueKey -> columns cannot be hidden, must use per-version column set. + gaps: "" + required: true + testing_strategy: + score: 70 + status: "partial" + value: > + S feature. Unit: SelectStatStatementsJITQuery version branching (query_test.go pattern). + Fix the two count-tests broken by adding a view (view_test.go::TestNew, + record/record_test.go::Test_filterViews). Full PG14-18 gate in CI. + gaps: "Confirm whether config_view/menu need tests" + required: true + +phase2_user_experience: + user_stories: + score: 50 + status: "partial" + value: "" + gaps: "Need concrete step-by-step scenarios" + required: true + acceptance_criteria: + score: 50 + status: "partial" + value: "" + gaps: "Testable checklist" + required: true + edge_cases: + score: 40 + status: "partial" + value: "" + gaps: "jit=off (all-zero), PG<15, zero-JIT queries, deform on PG17+" + required: true + error_scenarios: + score: 60 + status: "partial" + value: "pgss absent -> 'pg_stat_statements not found' (existing X-menu behavior)." + gaps: "PG<15 behavior" + required: true + verification_strategy: + score: 50 + status: "partial" + value: "" + gaps: "Local PG17 manual check + CI matrix" + required: true + ui_ux_requirements: + score: 55 + status: "partial" + value: "7th menu item; x-cycle wal->jit->timings; columns TBD; optional jit=off hint." + gaps: "Exact column set/order, hint decision, zero-row filtering" + required: true + risks: + score: 60 + status: "partial" + value: "Lowest-risk feature. Main pitfall: count-test breakage masked locally (no PG)." + gaps: "" + required: true + technical_decisions: + score: 60 + status: "partial" + value: "" + gaps: "Column set, hint, zero-row filter, total vs interval columns" + required: false + +phase3_integration: + integration_points: + score: 80 + status: "complete" + value: > + internal/query/statements.go (new query const + SelectStatStatementsJITQuery selector), + internal/view/view.go (new statements_jit view + DiffIntvl/UniqueKey/Ncols), + top/menu.go (menuPgss item + menuSelect case), top/config_view.go (statementsNextView link). + gaps: "" + required: true + dependencies: + score: 80 + status: "complete" + value: "pg_stat_statements extension; existing pgss view/menu/cycle infrastructure." + required: false + data_requirements: + score: 70 + status: "partial" + value: "JIT columns from pg_stat_statements; counts diffable, times rounded ms; md5 queryid." + gaps: "Confirm exact column list via code research" + required: false + migration_needs: + score: 100 + status: "complete" + value: "None — additive TUI feature." + required: false + +phase4_completion: + userspec_file: "docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit.md" + status: "approved" + +decision_rules: + required_threshold: 85 + optional_threshold: 50 diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-metrics-summary.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-metrics-summary.md new file mode 100644 index 0000000..099b6b9 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-metrics-summary.md @@ -0,0 +1,46 @@ +# Metrics Summary: pg-stat-statements-jit + +## Context + +| Dimension | Value | +|-----------|-------| +| Model | claude-opus-4-8[1m] | +| Feature size | S | +| Started | 2026-06-21 | +| Completed | 2026-06-22 | + +## Timeline + +Phase-level timing was not captured (metrics phases left null during the run). +The feature ran across two calendar days through the full SDD pipeline +(user-spec → tech-spec → decomposition → wave execution → QA → done). + +## Quality + +| Metric | Value | +|--------|-------| +| Validation rounds | user_spec: 1, tech_spec: 1, task_decomposition: 1 | +| Validation findings (crit/major/minor) | 0 / 2 / 9 | +| Review rounds (by task) | task_01: 1, task_02: 1, task_03: 2 | +| Review findings (crit/major/minor) | 2 / 1 / 3 | +| First pass rate | 67% (2 of 3 code tasks clean on review round 1) | + +Notable: task-03 (TUI wiring) failed dev-test-reviewer round 1 with 2 critical + 1 major — +stale assertions in `top/` menu/cycle tests (`Test_selectMenuStyle`, `Test_statementsNextView`, +`Test_switchViewTo`) that the code-research had not surfaced. Resolved in round 2. +The two tech-spec "major" findings (adequacy: tech-detail in user-spec; arch: undocumented +sort invariant) were doc-only and fixed before approval. + +## Volume + +| Metric | Value | +|--------|-------| +| Interview questions | 4 | +| Tasks | 4 (in 4 waves) | +| Agents spawned | ~26 (researcher, 2 userspec + 5 techspec validators, 4 task-creators, 3 task-validators, 3 implementers, ~8 reviewers) | +| Commits | 11 | + +## Outcome + +Shipped: 7th `pg_stat_statements` sub-screen (JIT), PG 15–18, TUI-only `NotRecordable`. +CI green (full gate: golangci-lint/gosec/govulncheck/test/build/E2E). Closes release 0.11.0. diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-metrics.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-metrics.json new file mode 100644 index 0000000..89dbc45 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-metrics.json @@ -0,0 +1,35 @@ +{ + "meta": { + "schema_version": "1.0", + "feature_id": "007", + "feature_name": "pg-stat-statements-jit", + "feature_base": "docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit", + "project": "pgcenter", + "feature_size": "S", + "model": "claude-opus-4-8[1m]", + "date_started": "2026-06-21", + "date_completed": "2026-06-22" + }, + "phases": { + "user_spec": null, + "tech_spec": null, + "task_decomposition": null, + "feature_execution": null, + "done": null + }, + "quality": { + "validation_rounds": { "user_spec": 1, "tech_spec": 1, "task_decomposition": 1 }, + "validation_findings": { "critical": 0, "major": 2, "minor": 9 }, + "review_rounds": { "task_01": 1, "task_02": 1, "task_03": 2 }, + "review_findings": { "critical": 2, "major": 1, "minor": 3 }, + "first_pass_rate_pct": 67, + "note": "Phase timing not captured (metrics phases left null during the run). Validation findings: userspec adequacy 1 major + quality 2 minor; techspec 0 critical/major across 5 validators; task validation 5 minor. Review findings: task-03 dev-test-reviewer round 1 had 2 critical + 1 major (stale TUI test assertions), resolved round 2." + }, + "volume": { + "interview_questions": 4, + "tasks_count": 4, + "waves_count": 4, + "agents_spawned": 26, + "commits_count": 11 + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-quality-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-quality-review.json new file mode 100644 index 0000000..a4778ef --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-quality-review.json @@ -0,0 +1,49 @@ +{ + "status": "approved", + "checks": { + "completeness": "pass", + "edge_cases": "pass", + "acceptance_criteria": "pass", + "contradictions": "pass", + "template_compliance": "pass", + "size_check": "warning", + "clarity": "pass" + }, + "findings": [ + { + "check": "size_check", + "severity": "minor", + "issue": "Заявлен size S, но число критериев приёмки (12) превышает порог >10. Спек по объёму и детализации (таблицы колонок по версиям, 3 сценария, 3 риска) тяготеет к нижней границе M. Фактический объём кода действительно мал (1-3 файла + правки count-тестов), поэтому это не противоречие, а пограничный случай.", + "location": "frontmatter (size: S) / Критерии приёмки", + "fix": "Оставить S оправданно (мало файлов, аддитивная фича), либо явно отметить, что часть критериев — это обязательные правки count-тестов, а не отдельные пользовательские флоу. Действия не требуется, информативно." + }, + { + "check": "acceptance_criteria", + "severity": "minor", + "issue": "Критерий «смена колонки сортировки и фильтр / работают» формулирует ожидаемый результат через слово «работают» без конкретного наблюдаемого исхода. Контекст (дефолт gen_total desc, duration-aware сортировка из сценария 2) делает его проверяемым, поэтому не critical/major.", + "location": "Критерии приёмки (6-й пункт)", + "fix": "Уточнить ожидаемый результат: «смена колонки сортировки пересортировывает строки численно (включая текст-длительности HH:MM:SS), фильтр / сужает набор по query/database»." + } + ], + "interview_coverage": { + "covered": [ + "feature_description (7-й pgss под-экран JIT, TUI-only, NotRecordable, закрывает 0.11.0)", + "user_problem (нет способа увидеть стоимость JIT-компиляции по запросам, jit=on overhead)", + "target_users (DBA, troubleshooting латентности/JIT overhead)", + "constraints (PG15+, deform_* на PG17, pgss required, синтетический md5 queryid → per-version column set)", + "testing_strategy (unit на SelectStatStatementsJITQuery, правки count-тестов, CI PG14-18)", + "user_stories / scenarios (4 истории + 3 пошаговых сценария)", + "acceptance_criteria (12 тестируемых критериев)", + "edge_cases (jit=off, PG<15, нет запросов выше jit_above_cost, deform на PG17+)", + "error_scenarios (pgss отсутствует → NOTICE; PG<15 деградация)", + "verification_strategy (агент: make build/test/lint, версионные ветки, CI; пользователь: локальный PG17)", + "ui_ux_requirements (7-й пункт меню, x-цикл wal→jit→timings, набор колонок по версиям, хинт jit=off, фильтр jit_functions>0)", + "risks (count-тесты ломаются и маскируются локально; механика хинта; деградация на PG<15)", + "technical_decisions (образец statements_timings, только functions без *_count, фильтр jit_functions>0, селектор SelectStatStatementsJITQuery, сортировка gen_total, NotRecordable)", + "integration_points (statements.go, view.go, top/menu.go, top/config_view.go)", + "data_requirements (JIT-колонки pgss, диффы интервальных, md5 queryid)" + ], + "missing": [] + }, + "summary": "Спек качественный и готов к утверждению: все обязательные секции заполнены содержательно, критерии тестируемы (есть и негативные — PG<15 не падает, хинт jit=off), риски имеют митигации, решения по тестам обоснованы, темы интервью покрыты полностью. Единственное — size_check warning: 12 критериев превышают порог >10 для S, но фактический объём кода мал, так что это пограничный, а не блокирующий случай." +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-security-audit.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-security-audit.json new file mode 100644 index 0000000..05362ea --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-security-audit.json @@ -0,0 +1,21 @@ +{ + "status": "approved", + "summary": { + "totalFindings": 1, + "critical": 0, + "major": 0, + "minor": 1 + }, + "findings": [ + { + "severity": "minor", + "category": "A03: Injection", + "title": "Schema token {{.PGSSSchema}} is interpolated unquoted (pre-existing pattern, not a regression)", + "description": "The two new JIT query consts (PgStatStatementsJITPG15, PgStatStatementsJITDefault) interpolate {{.PGSSSchema}} directly into the FROM clause ('FROM {{.PGSSSchema}}.pg_stat_statements p') via text/template in query.Format(), with no identifier quoting. This is string interpolation into SQL, which is technically the SQL-injection-prone construct that a static analyzer would flag. However, the value is NOT user-controlled: PGSSSchema is populated by extensionSchema() (internal/stat/postgres.go:536), which runs query.GetExtensionSchema against pg_extension/pg_namespace in the connected DB and returns the schema name where the pg_stat_statements extension is actually installed. It is a server-returned catalog identifier, defaulting to empty string on error. The other token {{.PgSSQueryLenFn}} is built in NewOptions (query.go:57) exclusively from an int (fmt.Sprintf with %d over PgSSQueryLen, hardcoded 256 in top/top.go:60), so no string injection is possible through it. The JIT consts use these tokens identically to all six existing pgss consts (Timing/General/Io/Temp/Local/Wal) and the same query.Format() text/template path, the WHERE jit_functions > 0 predicate is a fixed constant with no interpolation, and no user-supplied value enters the diffable query at all (the $1-parameterized report path is untouched by this feature). Therefore the design introduces NO new input surface and NO security regression relative to the established baseline. Severity kept minor and informational because (a) the data source is a trusted PostgreSQL catalog, (b) this is a local admin/observability CLI run by a DBA against their own cluster, and (c) the pattern is unchanged from existing code that has shipped for years.", + "location": "Section: Architecture / internal/query/statements.go (new consts PgStatStatementsJITPG15, PgStatStatementsJITDefault); compare existing statements.go:18,46,53,67,77,90,98", + "impact": "None in practice. A theoretical SQL injection via the schema qualifier would require an attacker to first compromise pg_extension/pg_namespace catalog contents (i.e. already have DDL privileges to install pg_stat_statements into an arbitrarily-named schema) on the very database the DBA is connecting to with their own credentials, in which case the attacker already controls the database. No privilege boundary is crossed by pgcenter.", + "recommendation": "No change required for this feature; matching the existing pattern is correct and keeps the codebase consistent. If schema-identifier hardening is ever desired project-wide, it should be a separate cross-cutting task (e.g. validate the extension schema name against a [A-Za-z0-9_]+ allowlist, or use quote_ident, in extensionSchema()), applied to all pgss query consts at once rather than only the new JIT ones. Do not single out this feature for it.", + "cwe": "CWE-89" + } + ] +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-skeptic-techspec.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-skeptic-techspec.json new file mode 100644 index 0000000..e590996 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-skeptic-techspec.json @@ -0,0 +1,49 @@ +{ + "status": "approved", + "summary": "Checked 32 claims, found 0 critical mirages. All file paths, function/const names, struct fields, line anchors, version constants, count-test expectations and the duration-aware sort are confirmed against the real codebase. 1 minor naming note (report.go function is describeReport, not doDescribe — only in code-research, tech-spec is generic).", + "findings": [ + { + "severity": "minor", + "type": "name_mismatch", + "claim": "code-research.md §6 says report.go maps view-name -> description in doDescribe (report.go:604)", + "reality": "The function at report/report.go:605 is named describeReport, not doDescribe. The description map literal spans report.go:606-629 (research said 604-624). The tech-spec itself does NOT name the function (Decision 5 only says 'add no report.go description entry'), so the tech-spec is unaffected; this is a code-research-only inaccuracy. The substantive precedent claim (bgwriter/replslots/stat_io absent from the map) is CONFIRMED.", + "source": "007-feat-pg-stat-statements-jit-code-research.md, §6 NotRecordable mechanism", + "fix": "Optional: rename doDescribe -> describeReport and fix the line anchor in code-research.md. No impact on tech-spec or implementation." + } + ], + "stats": { + "total_claims_checked": 32, + "confirmed": 31, + "mirages_found": 1, + "verified_claims": [ + "internal/query/statements.go exists", + "internal/query/io.go::SelectStatIOQuery returns (string, int, [2]int) — io.go:87", + "internal/query/statements.go::SelectStatStatementsTimingQuery returns string — statements.go:306", + "PgStatStatementsTimingPG13/PG12/Default consts exist; timing query is 13 cols (DiffIntvl {6,10})", + "internal/view/view.go::View struct has Name,MinRequiredVersion,QueryTmpl,DiffIntvl,Ncols,OrderKey,OrderDesc,UniqueKey,ColsWidth,Msg,Filters,NotRecordable", + "view.go statements_timings entry: Ncols 13, DiffIntvl {6,10}, UniqueKey 11", + "view.go statements_io: Ncols 13, DiffIntvl {6,10}, UniqueKey 11", + "view.go statements_wal: MinRequiredVersion PostgresV13, DiffIntvl {3,6}, Ncols 9, UniqueKey 7", + "view.go stat_io: NotRecordable true, MinRequiredVersion PostgresV16, UniqueKey 0 — view.go:166", + "view.go stat_io_time Msg carries track_io_timing hint — view.go:190", + "Configure() statements_timings case swaps only QueryTmpl — view.go:373-375", + "Configure() stat_io case patches QueryTmpl/Ncols/DiffIntvl — view.go:386", + "view.New() returns 26 views (confirmed by count)", + "view_test.go::TestNew asserts 26 — view_test.go:11 (bump to 27 valid)", + "TestNew_StatIOView precedent exists — view_test.go:17", + "internal/stat/stat.go::VersionOK guard returns 'not supported by current version' — stat.go:199-200", + "PostgresV15=150000 PostgresV16 PostgresV17=170000 PostgresV18 in query.go:18-21", + "top/menu.go menuPgss has 6 items (0-5) — menu.go:53-60", + "top/menu.go menuSelect case menuPgss handles cy 0-5 + default — menu.go:155-172", + "top/config_view.go::statementsNextView cycles wal->timings — config_view.go:160-180", + "keybindings.go: 'x' switchViewTo statements (line 40), 'X' menuOpen menuPgss (line 45)", + "record/record.go::filterViews NotRecordable branch fires before version gate — record.go:208 vs 214", + "record_test.go::Test_filterViews has 6 rows, documented wantN/wantV — record_test.go:123-128", + "internal/stat/postgres.go::sort has duration-aware branch via parseDuration — postgres.go:379", + "statements_test.go::TestSelectStatStatementsTimingQuery exists — line 10", + "statements_test.go exec loop with t.Skipf, WAL PG13+ gate — lines 53,86", + "report.go description map: bgwriter/replslots/stat_io absent (NotRecordable precedent) — report.go:606-629", + "no pre-existing statements_jit / JIT const references in codebase (grep clean)" + ] + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-code-reviewer-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-code-reviewer-review.json new file mode 100644 index 0000000..db99737 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-code-reviewer-review.json @@ -0,0 +1,33 @@ +{ + "reviewer": "dev-code-reviewer", + "status": "approved", + "summary": "Additive, correct, and idiomatic. The two JIT query consts faithfully mirror the canonical pgss layout (PgStatStatementsTimingPG13 / PgStatStatementsIoDefault) and the SelectStatStatementsJITQuery selector follows the SelectStatIOQuery multi-return model. Column ordering matches the declared Ncols/DiffIntvl/UniqueKey for both branches exactly; build, vet, and tests pass.", + "criticalIssues": [], + "suggestions": [ + { + "file": "internal/query/statements.go", + "line": 305, + "severity": "minor", + "category": "maintainability", + "suggestion": "The two consts are byte-for-byte identical except for the single deform_total / deform_ms columns. This duplication is intentional and matches the existing project pattern (the timing consts are likewise duplicated per version rather than composed), so no change is required. Flagging only so a future reader does not 'fix' it into a shared base string — the positional align path (ADR 006) makes per-version full strings the correct, lowest-risk shape.", + "benefit": "Prevents a well-meaning future refactor from breaking positional column alignment.", + "optional": true + } + ], + "verifiedFindings": [ + "Column layout PG15/16 (13 cols): user(0), database(1), gen_total(2), inline_total(3), opt_total(4), emit_total(5), gen,ms(6), inline,ms(7), opt,ms(8), emit,ms(9), functions(10), queryid(11), query(12). DiffIntvl {6,10} covers the interval block + functions; UniqueKey 11 = md5 queryid. Matches selector return and task spec exactly.", + "Column layout PG17+ (15 cols): user(0)..emit_total(5), deform_total(6), gen,ms(7)..emit,ms(10), deform,ms(11), functions(12), queryid(13), query(14). DiffIntvl {7,12}, UniqueKey 13. Matches selector return and task spec exactly.", + "Column expressions correct: *_total uses date_trunc('seconds', round(p.jit_*_time)/1000 * '1 second'::interval)::text; *_ms uses round(p.jit_*_time); functions uses p.jit_functions; queryid uses left(md5(...),10); query is regexp_replace last. round() is applied consistently (avoids fractional ms), matching the timing-const pattern.", + "JIT source column names verified against task/tech-spec: jit_generation_time, jit_inlining_time, jit_optimization_time, jit_emission_time (PG15+), plus jit_deform_time (PG17+). The intentionally-dropped *_count metrics are correctly omitted; only jit_functions is surfaced as the single counter.", + "Both consts use template tokens {{.PGSSSchema}} and {{.PgSSQueryLenFn}}, end with JOIN pg_database d ON d.oid=p.dbid WHERE p.jit_functions > 0 (Decision 3), and use the concatenated E'\\\\s+' regexp style consistent with PgStatStatementsIoDefault (not the raw-backtick single-backslash style).", + "Cross-file consistency: selector branches on PostgresV17 (170000, query.go:20); SelectStatStatementsJITQuery signature (string, int, [2]int, int) mirrors SelectStatIOQuery (io.go:87) with the extra UniqueKey return as specified. No existing const or selector was modified.", + "Tests: TestSelectStatStatementsJITQuery covers all four version branches (150000/160000 -> PG15 const, 13/{6,10}/11; 170000/180000 -> Default const, 15/{7,12}/13). JIT exec sub-test added to Test_StatStatementsQueries, gated PG15+, per-version sub-tests with t.Skipf when PG unavailable (matches WAL-loop pattern, avoids the skip-in-loop bug). go build / go vet / go test ./internal/query/... all green." + ], + "metrics": { + "filesReviewed": 2, + "criticalIssuesCount": 0, + "majorIssuesCount": 0, + "minorIssuesCount": 1, + "testCoverageAssessment": "adequate" + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-security-auditor-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-security-auditor-review.json new file mode 100644 index 0000000..8556751 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-security-auditor-review.json @@ -0,0 +1,10 @@ +{ + "status": "approved", + "summary": { + "totalFindings": 0, + "critical": 0, + "major": 0, + "minor": 0 + }, + "findings": [] +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-test-reviewer-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-test-reviewer-review.json new file mode 100644 index 0000000..89795b5 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01-dev-test-reviewer-review.json @@ -0,0 +1,30 @@ +{ + "reviewer": "dev-test-reviewer", + "status": "passed", + "summary": "High-quality tests for Task 01. TestSelectStatStatementsJITQuery is a real behavioral table test covering both version branches (PG15/16 -> 13 cols, PG17/18 -> 15 cols) and asserting all four return values (query, Ncols, DiffIntvl, UniqueKey) against values that exactly match the implementation and verified column layout. The JIT exec sub-test correctly mirrors the WAL loop, gates PG15+, and skips cleanly without a real PG instance. Tests pass litmus: removing the selector's branch logic or returning wrong Ncols/Key would fail the assertions.", + "findings": [ + { + "severity": "minor", + "category": "missing_coverage", + "location": "internal/query/statements_test.go:34 (TestSelectStatStatementsJITQuery)", + "issue": "The unit test only covers versions >= 150000. The selector is intentionally two-branch (PG<15 gated upstream at the view layer, per task Details), so no PG<15 case is required by contract. Noting only that the lower boundary (e.g. 149999/140000 returning PgStatStatementsJITPG15) is not asserted; this is by design, not a defect.", + "recommendation": "Optional: add a single boundary case at version 169999 to lock the >= PostgresV17 cutoff (already covered indirectly by 160000 vs 170000). No change required for acceptance.", + "litmusTestFailed": false + } + ], + "metrics": { + "filesReviewed": 3, + "litmusTest": { + "checked": 2, + "passed": 2, + "failed": 0 + }, + "coverageAssessment": "adequate", + "pyramidBalance": { + "unit": 1, + "integration": 1, + "e2e": 0, + "assessment": "healthy" + } + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01.md new file mode 100644 index 0000000..0a43a42 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-01.md @@ -0,0 +1,201 @@ +--- +status: done # planned -> in_progress -> done +depends_on: [] # ID задач-зависимостей (строки: ["01", "02"]) +wave: 1 # волна параллельного выполнения +skills: [code-writing] # МАССИВ скиллов для загрузки +verify: bash — go test ./internal/query/... +reviewers: [dev-code-reviewer, dev-security-auditor, dev-test-reviewer] +teammate_name: +--- + +# Task 01: JIT query consts + version selector + +## Required Skills + +Перед выполнением задачи загрузи: +- `/skill:code-writing` — [skills/code-writing/SKILL.md](~/.claude/skills/code-writing/SKILL.md) + +## Description + +Добавить в `internal/query/statements.go` два версионно-зависимых query-конста для нового +sub-screen `statements_jit` (JIT-компиляция в `pg_stat_statements`), плюс селектор +`SelectStatStatementsJITQuery(version)`, возвращающий query + `Ncols` + `DiffIntvl` + +`UniqueKey`. + +Это фундамент фичи (Wave 1, без зависимостей) и единственный слой, где живёт разница колонок +между PG15/16 и PG17+. PG15/16 показывает 8 JIT-метрик (база), PG17+ добавляет +`jit_deform_count`/`jit_deform_time`. Из-за синтетического md5-`queryid` как `UniqueKey` +колонки нельзя скрывать по версии (выравнивание позиционное, ADR `[006-feat-pg-stat-io]`), +поэтому каждая версия получает свой набор колонок, а `Ncols`/`DiffIntvl`/`UniqueKey` должны +двигаться вместе с ним — отсюда мульти-return по модели `SelectStatIOQuery` (а не по модели +timing-селектора, который возвращает только string). + +Колоночный дизайн (Decision 2): один блок кумулятивных текстовых длительностей `*_total` +(показывается, не диффится) + один interval-блок `*_ms` + `functions` (диффится). Из 4 пар +phase count/time показываем только 4 phase TIME (как total+interval), а из счётчиков — +только `jit_functions` как единственный представительный counter; остальные `*_count` +метрики (inlining/optimization/emission/deform count) намеренно НЕ выводятся. Это укладывается +в ширину терминала (горизонтального скролла нет). + +Фильтрация (Decision 3): оба конста заканчиваются `WHERE p.jit_functions > 0`, чтобы под +дефолтным `jit=on` экран не был забит нулевыми строками, а под `jit=off` был пустым. + +## What to do + +1. Добавить в блок `const (...)` файла `internal/query/statements.go` (рядом с остальными + `PgStatStatements*` константами, по образцу `PgStatStatementsIoDefault` и timing-констант) + две новые константы: + - `PgStatStatementsJITPG15` — база PG15/16, 13 колонок. + - `PgStatStatementsJITDefault` — PG17+, 15 колонок (база + deform). +2. Обе константы используют шаблонные токены `{{.PGSSSchema}}` и `{{.PgSSQueryLenFn}}` и + заканчиваются `FROM {{.PGSSSchema}}.pg_stat_statements p JOIN pg_database d ON d.oid=p.dbid + WHERE p.jit_functions > 0`. Стиль строки — конкатенация `+ "..."` с `E'\\s+'` в + `regexp_replace` (как у `PgStatStatementsIoDefault`). +3. Добавить функцию `SelectStatStatementsJITQuery(version int) (string, int, [2]int, int)` + рядом с `SelectStatStatementsTimingQuery` (низ файла). Двухветочный switch по образцу + `SelectStatIOQuery` из `internal/query/io.go`. Возврат `UniqueKey` — это четвёртый элемент + (отличие от 3-tuple `SelectStatIOQuery`, т.к. ключ JIT стоит в конце и сдвигается с + `Ncols`). +4. Добавить в `internal/query/statements_test.go` тест `TestSelectStatStatementsJITQuery` + (по образцу `TestSelectStatStatementsTimingQuery`), проверяющий обе ветки: query, Ncols, + DiffIntvl, UniqueKey. +5. Добавить в `Test_StatStatementsQueries` exec-подтест для JIT, gated PG15+ (цикл по + версиям `[]int{150000, 160000, 170000, 180000}`, по образцу WAL-цикла PG13+), который + локально без PG скипается через `t.Skipf`. + +## TDD Anchor + +Тесты пишем ДО реализации, убеждаемся что падают (констант/функции ещё нет), потом пишем код. + +- `internal/query/statements_test.go::TestSelectStatStatementsJITQuery` — для PG15/16 + (150000, 160000) возвращает `(PgStatStatementsJITPG15, 13, [2]int{6,10}, 11)`; для PG17/18 + (170000, 180000) возвращает `(PgStatStatementsJITDefault, 15, [2]int{7,12}, 13)`. +- `internal/query/statements_test.go::Test_StatStatementsQueries` (JIT exec sub-test) — для + каждой версии PG15+ форматирует выбранный template через `Format()` и выполняет на реальном + PG без ошибки (валидирует имена JIT-колонок против реальной схемы); без PG — `t.Skipf`. + +## Acceptance Criteria + +- [ ] `PgStatStatementsJITPG15` (13 колонок) и `PgStatStatementsJITDefault` (15 колонок) + добавлены, следуют канонической pgss-разметке (user, database, *_total, *_ms + + functions interval, md5 queryid, query последним), оба содержат `WHERE p.jit_functions > 0`. +- [ ] Колонки `*_total` — кумулятивные текстовые длительности + `date_trunc('seconds', round()/1000 * '1 second'::interval)::text`; + `*_ms` — `round(p.jit_*_time)`; `functions` — `p.jit_functions`. +- [ ] PG17+ конст дополнительно содержит `deform_total` (idx 6) и `deform_ms` (interval), + использует `jit_deform_time`. +- [ ] `SelectStatStatementsJITQuery(version)` возвращает `(string, int, [2]int, int)`: + `version >= 170000` → `(Default, 15, [2]int{7,12}, 13)`; иначе → `(PG15, 13, [2]int{6,10}, 11)`. +- [ ] `TestSelectStatStatementsJITQuery` покрывает обе ветки (query/Ncols/DiffIntvl/UniqueKey). +- [ ] JIT exec sub-test добавлен в `Test_StatStatementsQueries`, gated PG15+, скипается без PG. +- [ ] `go test ./internal/query/...` зелёный локально (exec-подтесты скипаются без PG). +- [ ] Нет регрессий в существующих query-тестах. + +## Context Files + +**Feature artifacts:** +- [007-feat-pg-stat-statements-jit.md](007-feat-pg-stat-statements-jit.md) — user-spec +- [007-feat-pg-stat-statements-jit-tech-spec.md](007-feat-pg-stat-statements-jit-tech-spec.md) — tech-spec (Solution, Decision 1/2/3, Data Models) +- [007-feat-pg-stat-statements-jit-code-research.md](007-feat-pg-stat-statements-jit-code-research.md) — code-research (§1 JIT schema, §2 reference pattern, §8 anchors) +- 007-feat-pg-stat-statements-jit-decisions.md — decisions log (создаётся при завершении) + +**Project knowledge:** +- [project.md](.claude/skills/project-knowledge/overview.md) +- [architecture.md](.claude/skills/project-knowledge/architecture.md) +- [patterns.md](.claude/skills/project-knowledge/patterns.md) — code patterns, testing conventions, version branching + +**Code files:** +- [internal/query/statements.go](internal/query/statements.go) — добавить 2 конста + селектор (модель: `PgStatStatementsIoDefault` строки 55-67, timing-консты, `SelectStatStatementsTimingQuery` строки 305-315) +- [internal/query/io.go](internal/query/io.go) — образец мульти-return: `SelectStatIOQuery` строки 87-92 `(string, int, [2]int)` +- [internal/query/query.go](internal/query/query.go) — version-константы `PostgresV15`..`PostgresV18` строки 18-21 +- [internal/query/statements_test.go](internal/query/statements_test.go) — добавить тест (модель: `TestSelectStatStatementsTimingQuery` строки 10-32, WAL exec-цикл строки 86-102) + +## Verification Steps + +- Запустить `go test ./internal/query/...` — все тесты зелёные, JIT exec-подтесты скипаются + без PG (`t.Skipf`). +- Убедиться, что `TestSelectStatStatementsJITQuery` падал ДО реализации селектора и прошёл + ПОСЛЕ (TDD). +- Проверить, что существующие тесты (`TestSelectStatStatementsTimingQuery`, + `Test_StatStatementsQueries`, `TestSelectQueryReportQuery`) не сломались. + +## Details + +**Files:** +- `internal/query/statements.go` — добавить в `const (...)` блок (заканчивается строкой 303) + две константы и в конец файла (после `SelectQueryReportQuery`, строка 327) — селектор. +- `internal/query/statements_test.go` — добавить `TestSelectStatStatementsJITQuery` и + JIT exec-цикл внутри `Test_StatStatementsQueries`. + +**Точные имена JIT-колонок (verified vs PG official docs, code-research §1):** +- PG15/16: `jit_functions`(bigint), `jit_generation_time`, `jit_inlining_count`, + `jit_inlining_time`, `jit_optimization_count`, `jit_optimization_time`, + `jit_emission_count`, `jit_emission_time` (все `*_time` — double precision, миллисекунды). +- PG17+ добавляет: `jit_deform_count`(bigint), `jit_deform_time`(double). +- ВАЖНО: `*_count` метрики (inlining/optimization/emission/deform count) в выводе НЕ + используются — показывается только `jit_functions` как единственный counter; 4 (PG17+: 5) + phase TIME показываются как total + interval. + +**Колоночная разметка (Decision 2):** +- PG15/16 (13 колонок, 0-based): `user`(0), `database`(1), `gen_total`(2), `inline_total`(3), + `opt_total`(4), `emit_total`(5), `gen_ms`(6), `inline_ms`(7), `opt_ms`(8), `emit_ms`(9), + `functions`(10), `queryid`(11), `query`(12). `DiffIntvl {6,10}` (interval-блок 6-9 + + functions 10), `UniqueKey 11`. +- PG17+ (15 колонок): `user`(0), `database`(1), `gen_total`(2), `inline_total`(3), + `opt_total`(4), `emit_total`(5), `deform_total`(6), `gen_ms`(7), `inline_ms`(8), + `opt_ms`(9), `emit_ms`(10), `deform_ms`(11), `functions`(12), `queryid`(13), `query`(14). + `DiffIntvl {7,12}`, `UniqueKey 13`. + +**Выражения колонок (по образцу timing-констант statements.go:8-18):** +- `*_total`: `date_trunc('seconds', round(p.jit_generation_time) / 1000 * '1 second'::interval)::text AS gen_total` + (аналогично inline/opt/emit/deform с соответствующими `jit_*_time`). +- `*_ms` (interval): `round(p.jit_generation_time) AS "gen,ms"` (alias-нейминг по образцу + соседей — см. как timing использует `"all,ms"`). +- `functions`: `p.jit_functions AS functions`. +- `queryid`: `left(md5(p.userid::text || p.dbid::text || p.queryid::text), 10) AS queryid`. +- `query`: `regexp_replace({{.PgSSQueryLenFn}}, E'\\s+', ' ', 'g') AS query`. +- `user`/`database`: `pg_get_userbyid(p.userid) AS user, d.datname AS database`. + +**Селектор (модель `SelectStatIOQuery` io.go:87-92, но 4-tuple):** +```go +// SelectStatStatementsJITQuery returns the statements_jit query, column count, +// diff interval and unique-key index depending on Postgres version. +func SelectStatStatementsJITQuery(version int) (string, int, [2]int, int) { + if version >= PostgresV17 { + return PgStatStatementsJITDefault, 15, [2]int{7, 12}, 13 + } + return PgStatStatementsJITPG15, 13, [2]int{6, 10}, 11 +} +``` +(`version >= PostgresV17` либо `version >= 170000` — `PostgresV17 = 170000`, query.go:20; +match neighbours.) + +**Dependencies:** нет внешних пакетов. Использует существующие `query.Format()`, +`PostgresV15`..`PostgresV18`. Селектор вызывается только из `Configure()` для PG15+ (gate на +view-слое в Task 2), поэтому двухветочный switch достаточен — ветка PG<15 не нужна. + +**Edge cases:** +- Ncols/DiffIntvl/UniqueKey должны быть строго консистентны с реальным числом колонок — + главный риск (silent diff/align bug). Проверяется юнит-тестом + exec-тестом на CI PG15-18. +- `*_time` — double precision в мс; `round()` обязателен (как в timing), иначе дробные мс. +- `WHERE p.jit_functions > 0` — обязателен в обоих константах (Decision 3). + +**Implementation hints:** +- Пиши `regexp_replace(..., E'\\s+', ...)` с двойным бэкслешем (конкатенированный стиль), как + у `PgStatStatementsIoDefault` (statements.go:66), НЕ raw-backtick стиль с одинарным. +- JOIN — именно `JOIN pg_database d ON d.oid=p.dbid` (как у всех pgss-диффабельных констант). +- В exec-цикле используй `NewOptions(version, "f", "off", 256, "public")` и + `postgres.NewTestConnectVersion(version)` (как WAL-цикл statements_test.go:86-102). +- Не трогай существующие константы/селекторы — только добавление. + +## Reviewers + +- **dev-code-reviewer** → `007-feat-pg-stat-statements-jit-task-01-dev-code-reviewer-review.json` +- **dev-security-auditor** → `007-feat-pg-stat-statements-jit-task-01-dev-security-auditor-review.json` +- **dev-test-reviewer** → `007-feat-pg-stat-statements-jit-task-01-dev-test-reviewer-review.json` + +## Post-completion + +- [ ] Записать краткий отчёт в `007-feat-pg-stat-statements-jit-decisions.md` (Summary: 1-3 предложения, ревью со ссылками на JSON, без таблиц файндингов и дампов) +- [ ] Если отклонились от спека — описать отклонение и причину +- [ ] Обновить user-spec/tech-spec если что-то изменилось diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-code-reviewer-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-code-reviewer-review.json new file mode 100644 index 0000000..31fd5fa --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-code-reviewer-review.json @@ -0,0 +1,21 @@ +{ + "reviewer": "dev-code-reviewer", + "status": "approved", + "summary": "Task 02 cleanly registers the statements_jit view and its Configure() branch, modeling statements_io for the pgss layout and stat_io for the NotRecordable+MinRequiredVersion precedent. Cross-file consistency is correct: the Configure() call matches the task-01 selector signature query.SelectStatStatementsJITQuery(version) (string, int, [2]int, int) and the static PG15 defaults in the view map (QueryTmpl=PgStatStatementsJITPG15, Ncols=13, DiffIntvl={6,10}, UniqueKey=11) match the selector's PG15 return. All three count-tests were updated correctly and the diff-affected tests pass.", + "criticalIssues": [], + "suggestions": [], + "metrics": { + "filesReviewed": 3, + "criticalIssuesCount": 0, + "majorIssuesCount": 0, + "minorIssuesCount": 0, + "testCoverageAssessment": "adequate" + }, + "notes": [ + "Cross-file consistency (dim 10) verified: Configure() at view.go assigns all 4 return values (QueryTmpl, Ncols, DiffIntvl, UniqueKey) from SelectStatStatementsJITQuery — matches the 4-tuple signature at internal/query/statements.go:347 and its PG15 return (PgStatStatementsJITPG15, 13, {6,10}, 11) at statements.go:351, which equals the static map defaults. PG17+ override path (15, {7,12}, 13) is exercised by the selector's own unit tests (statements_test.go). UniqueKey patching (correctly added beyond the stat_io 3-tuple precedent) is justified because the JIT md5 key sits at the end and shifts with Ncols.", + "filterViews ordering confirmed: record/record.go NotRecordable branch (line ~208) fires before the VersionOK gate (line ~214), so the statements_jit view is dropped on every version regardless of MinRequiredVersion=PostgresV15. This makes the uniform wantN +1 across all 6 Test_filterViews rows (including PG<15) correct; wantV unchanged is correct since dropped views never join the remaining set.", + "All three count-tests handled per task spec: TestNew 26->27, TestView_VersionOK 160000 row 26->27 (VersionOK true only at 150000+, lower rows untouched — correct), Test_filterViews +1 on all 6 rows. New guard test TestNew_StatementsJITView pins NotRecordable, MinRequiredVersion, and the PG15 default layout fields — good defensive coverage mirroring TestNew_StatIOView.", + "report.go correctly NOT modified — NotRecordable views (bgwriter/replslots/stat_io precedent) have no doDescribe entry by design; explicitly out of scope per instructions.", + "Test execution: go test ./internal/view/... passes; Test_filterViews and the other filterViews tests pass in isolation. Test_app_record panics (nil-pointer at record.go:167, DB connection path) — verified this panic is PRE-EXISTING on parent commit HEAD~1 and is caused by the absence of a live PostgreSQL test instance, NOT by this diff. Not a finding against task 02." + ] +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-security-auditor-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-security-auditor-review.json new file mode 100644 index 0000000..8556751 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-security-auditor-review.json @@ -0,0 +1,10 @@ +{ + "status": "approved", + "summary": { + "totalFindings": 0, + "critical": 0, + "major": 0, + "minor": 0 + }, + "findings": [] +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-test-reviewer-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-test-reviewer-review.json new file mode 100644 index 0000000..0280372 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02-dev-test-reviewer-review.json @@ -0,0 +1,30 @@ +{ + "reviewer": "dev-test-reviewer", + "status": "passed", + "summary": "Count-test changes for Task 02 are correct and the guard test pins the right invariants. The three count-tests (TestNew 26->27, Test_filterViews wantN +1 on all 6 rows, TestView_VersionOK 160000 row 26->27) form a coherent TDD anchor: each delta traces directly to registering one new NotRecordable, PG15-gated view. The new TestNew_StatementsJITView guard mirrors TestNew_StatIOView and pins every behaviorally-significant field (NotRecordable, MinRequiredVersion, Ncols, DiffIntvl, UniqueKey, OrderKey, OrderDesc, non-empty Msg). All count tests pass in isolation. Count math verified manually and empirically.", + "findings": [ + { + "severity": "minor", + "category": "anti_pattern", + "location": "internal/view/view_test.go:29", + "issue": "Msg invariant is asserted only as non-empty (assert.NotEqual(t, \"\", jit.Msg)). The task and Decision 4 give Msg a load-bearing role: it must carry the 'no rows when jit=off' empty-state hint. A non-empty check passes for any string, so it does not pin the documented behavior. Note this exactly mirrors the existing TestNew_StatIOView pattern (line 47), whereas TestNew_StatIOTimeView (line 65) uses assert.Contains for its track_io_timing hint — so a stronger assertion has precedent in the same file.", + "recommendation": "Replace assert.NotEqual(t, \"\", jit.Msg) with assert.Contains(t, jit.Msg, \"jit=off\") (matching the TestNew_StatIOTimeView precedent). This pins the empty-state hint that Decision 4 requires, so an accidental Msg edit that drops the jit=off note is caught.", + "litmusTestFailed": false + } + ], + "metrics": { + "filesReviewed": 5, + "litmusTest": { + "checked": 4, + "passed": 4, + "failed": 0 + }, + "coverageAssessment": "adequate", + "pyramidBalance": { + "unit": 4, + "integration": 0, + "e2e": 0, + "assessment": "healthy" + } + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02.md new file mode 100644 index 0000000..00c107c --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-02.md @@ -0,0 +1,211 @@ +--- +status: done # planned -> in_progress -> done +depends_on: ["01"] # ID задач-зависимостей (строки: ["01", "02"]) +wave: 2 # волна параллельного выполнения +skills: [code-writing] # МАССИВ скиллов для загрузки +verify: bash # инструмент верификации (опционально: curl, bash, user) +reviewers: [dev-code-reviewer, dev-security-auditor, dev-test-reviewer] # явно указать. Пусто = fallback на defaults +teammate_name: # имя агента-исполнителя (опционально; если не задано — генерируется по описанию задачи) +--- + +# Task 02: Register statements_jit view + Configure + count-test fixes + +## Required Skills + +Перед выполнением задачи загрузи: +- `/skill:code-writing` — [skills/code-writing/SKILL.md](~/.claude/skills/code-writing/SKILL.md) + +## Description + +Регистрируем новый экран `statements_jit` (7-й под-экран `pg_stat_statements`, JIT-компиляция) +как полноценный view в pgcenter: добавляем запись в карту `view.New()` и ветку в +`view.Views.Configure()`, которая на старте подставляет в view версионно-выбранный query и его +layout-метаданные (`QueryTmpl`/`Ncols`/`DiffIntvl`/`UniqueKey`). + +View завязан на селектор `query.SelectStatStatementsJITQuery(version)` и JIT-консты из задачи 01 +(зависимость по Wave 1). View гейтится `MinRequiredVersion: query.PostgresV15`, помечается +`NotRecordable: true` (TUI-first принцип релиза 0.11.0, прямой прецедент — +`bgwriter`/`replslots`/`stat_io`), сортируется по `gen_total` desc (`OrderKey: 2`, +`OrderDesc: true`), а `Msg` несёт подсказку про пустой экран при `jit=off`. + +Поскольку JIT-колонки и их количество отличаются между PG15/16 (13 колонок) и PG17+ (15 колонок), +`Configure()` должна патчить все четыре поля (как существующая ветка `stat_io`, которая зовёт +`SelectStatIOQuery` и патчит `QueryTmpl`/`Ncols`/`DiffIntvl`), а не только `QueryTmpl` как ветка +`statements_timings` (view.go:373) — у timings количество колонок неизменно (13), а у JIT меняется. + +Добавление любого view ломает count-тесты, которые считают общее число view — их нужно +подправить: `view_test.go::TestNew` (26 → 27) и `record/record_test.go::Test_filterViews` +(`wantN +1` на всех 6 строках, потому что `NotRecordable` отбрасывает view ещё ДО версионного +гейта в `filterViews`). + +`report.go` НЕ трогаем — `NotRecordable` view не записывается, значит и описания для отчёта не +нужно (тот же прецедент: bgwriter/replslots/stat_io не имеют записи в `doDescribe`). + +## What to do + +1. **Добавить запись `"statements_jit"`** в карту, возвращаемую `view.New()` + (internal/view/view.go). Моделировать по записи `statements_io` (view.go:218-229) с добавлением + полей `MinRequiredVersion` + `NotRecordable` по образцу `stat_io` (view.go:166-179): + - `Name: "statements_jit"` + - `MinRequiredVersion: query.PostgresV15` + - `NotRecordable: true` + - `OrderKey: 2`, `OrderDesc: true` (сортировка по первому `*_total`, `gen_total`) + - `Msg`: подсказка про пустой экран, например + `"Show statements JIT compilation statistics (no rows when jit=off)"` + - `ColsWidth: map[int]int{}`, `Filters: map[int]*regexp.Regexp{}` + - Статические `QueryTmpl`/`Ncols`/`DiffIntvl`/`UniqueKey` — это плейсхолдеры, которые патчит + `Configure()`. Задать им PG15-базовые значения как разумный дефолт: `QueryTmpl: + query.PgStatStatementsJITPG15`, `Ncols: 13`, `DiffIntvl: [2]int{6, 10}`, `UniqueKey: 11` + (значения для PG15/16 из tech-spec Decision 2; селектор/консты приходят из задачи 01). + +2. **Добавить `case "statements_jit":`** в `view.Views.Configure()` (internal/view/view.go, + рядом с другими pgss/stat_io кейсами, view.go:373-390). Вызвать + `query.SelectStatStatementsJITQuery(opts.Version)` и присвоить все четыре возвращаемых значения + в `view.QueryTmpl`, `view.Ncols`, `view.DiffIntvl`, `view.UniqueKey`, затем `v[k] = view`. + Моделировать по ветке `stat_io` (view.go:385-387), но JIT возвращает 4 значения (с + `UniqueKey`), а не 3 — это форма селектора из задачи 01. + +3. **Поправить `internal/view/view_test.go::TestNew`** (view_test.go:9-12): поднять ожидаемое + число view 26 → 27 и обновить комментарий. Опционально добавить guard-тест + `TestNew_StatementsJITView` по образцу `TestNew_StatIOView` (view_test.go:17-30): проверить + наличие view `statements_jit`, `NotRecordable == true`, `MinRequiredVersion == query.PostgresV15`, + PG15-дефолты `Ncols`/`DiffIntvl`/`OrderKey`/`OrderDesc`/`UniqueKey` и непустой `Msg`. + +4. **Поправить `record/record_test.go::Test_filterViews`** (record_test.go:101-136): поднять + `wantN` на +1 на ВСЕХ 6 строках (`wantV` без изменений), и дополнить пояснительный комментарий. + Причина: новый view `NotRecordable: true`, а в `filterViews` (record.go:208) ветка + `NotRecordable` срабатывает РАНЬШЕ версионного гейта (record.go:214) — поэтому view + отбрасывается на каждой версии независимо от `MinRequiredVersion: PostgresV15`. Конкретные + строки: см. таблицу в разделе Details. + +5. НЕ менять `report/report.go` (нет описания для NotRecordable view). + +## TDD Anchor + +Тесты, которые нужно написать ДО реализации (точнее — обновить существующие count-тесты): они +начинают падать как только новый view добавлен/сконфигурирован, и проходят после корректных +правок. Пишем правки тестов → запускаем → убеждаемся что падают на старом коде → добавляем +view + Configure → убеждаемся что проходят. + +- `internal/view/view_test.go::TestNew` — `len(New())` должно быть 27 после добавления + `statements_jit` в карту view. +- `internal/view/view_test.go::TestNew_StatementsJITView` (опционально, новый) — `statements_jit` + зарегистрирован, `NotRecordable=true`, `MinRequiredVersion=query.PostgresV15`, PG15-дефолты + `Ncols=13` / `DiffIntvl={6,10}` / `UniqueKey=11`, `OrderKey=2`, `OrderDesc=true`, `Msg` непустой. +- `record/record_test.go::Test_filterViews` — на каждой из 6 строк `wantN` увеличено на 1 + (NotRecordable отбрасывает view до версионного гейта), `wantV` неизменно. + +## Acceptance Criteria + +- [ ] В `view.New()` добавлена запись `statements_jit`: `MinRequiredVersion: query.PostgresV15`, + `NotRecordable: true`, `OrderKey: 2`, `OrderDesc: true`, `Msg` с подсказкой про `jit=off`, + `ColsWidth`/`Filters` — пустые карты, PG15-дефолтные `QueryTmpl`/`Ncols: 13`/`DiffIntvl: {6,10}`/`UniqueKey: 11`. +- [ ] В `Configure()` добавлен `case "statements_jit":`, патчащий `QueryTmpl`/`Ncols`/`DiffIntvl`/`UniqueKey` + из `query.SelectStatStatementsJITQuery(opts.Version)`. +- [ ] `view_test.go::TestNew` ожидает 27 (комментарий обновлён). +- [ ] `record/record_test.go::Test_filterViews` имеет `wantN +1` на всех 6 строках, `wantV` без изменений. +- [ ] `report/report.go` НЕ изменён (нет записи описания для NotRecordable view). +- [ ] `go test ./internal/view/... ./record/...` зелёный; нет регрессий в существующих view/record тестах. + +## Context Files + +**Feature artifacts:** +- [007-feat-pg-stat-statements-jit.md](007-feat-pg-stat-statements-jit.md) — user-spec +- [007-feat-pg-stat-statements-jit-tech-spec.md](007-feat-pg-stat-statements-jit-tech-spec.md) — tech-spec (Solution, Decision 1/4/5, How it works) +- [007-feat-pg-stat-statements-jit-code-research.md](007-feat-pg-stat-statements-jit-code-research.md) — code-research (§3 view registration, §4 count-tests, §6 NotRecordable) +- [007-feat-pg-stat-statements-jit-decisions.md](007-feat-pg-stat-statements-jit-decisions.md) — decisions log +- [007-feat-pg-stat-statements-jit-task-01.md](007-feat-pg-stat-statements-jit-task-01.md) — задача-зависимость (JIT консты + `SelectStatStatementsJITQuery`) + +**Project knowledge:** +- [overview.md](.claude/skills/project-knowledge/overview.md) — фичи, поддерживаемая статистика +- [architecture.md](.claude/skills/project-knowledge/architecture.md) — раскладка пакетов, поток данных, обработка версий PG +- [patterns.md](.claude/skills/project-knowledge/patterns.md) — паттерны кода, конвенции тестов, версионное ветвление + +**Code files:** +- [internal/view/view.go](internal/view/view.go) — добавить запись `statements_jit` + `case` в `Configure()` +- [internal/view/view_test.go](internal/view/view_test.go) — поднять `TestNew` 26→27 (+ опц. guard-тест) +- [record/record_test.go](record/record_test.go) — `Test_filterViews` `wantN +1` на всех строках +- [internal/query/io.go](internal/query/io.go) — образец формы селектора `SelectStatIOQuery` (читать) +- [record/record.go](record/record.go) — `filterViews`: порядок `NotRecordable` до версионного гейта (читать) + +## Verification Steps + +- Запустить `go test ./internal/view/... ./record/...` — должно быть зелёным. +- Проверить, что `TestNew` ожидает 27, `Test_filterViews` проходит со всеми 6 поднятыми `wantN`. +- Убедиться, что `report/report.go` не изменён (git diff пуст по этому файлу). +- (опц.) `make build` — компиляция чистая; новый `case` в `Configure()` использует селектор/консты из задачи 01. + +## Details + + + +**Files:** +- `internal/view/view.go` — (1) новая запись в карте `New()` (текущая карта — 26 записей, + view.go:38-350; pgss-блок 194-266). Образец структуры: `statements_io` (view.go:218-229) для + pgss-формы; `stat_io` (view.go:166-179) для `MinRequiredVersion`+`NotRecordable`+`UniqueKey` + вместе. (2) Новый `case "statements_jit":` в `Configure()` (switch view.go:363-391); образец — + ветка `stat_io` (view.go:385-387), но с 4 присваиваниями (включая `UniqueKey`). +- `internal/view/view_test.go` — `TestNew` на view_test.go:9-12 (`assert.Equal(t, 26, len(v))` → + 27, комментарий). Образец guard-теста — `TestNew_StatIOView` (view_test.go:17-30). +- `record/record_test.go` — таблица `Test_filterViews` на record_test.go:123-128. Поднять `wantN` + по таблице ниже, `wantV` оставить как есть, дополнить комментарий-пояснение (record_test.go:108-122). + +**Таблица правок `Test_filterViews` (wantN old → new, wantV без изменений):** + +| version | pgssSchema | wantN old → new | wantV | +|---------|-----------|-----------------|-------| +| 140000 | "" | 10 → 11 | 16 | +| 140000 | public | 4 → 5 | 22 | +| 130000 | public | 7 → 8 | 19 | +| 120000 | public | 10 → 11 | 16 | +| 110000 | public | 12 → 13 | 14 | +| 100000 | public | 12 → 13 | 14 | + +**Dependencies:** +- Задача 01 (Wave 1): должна предоставить консты `query.PgStatStatementsJITPG15` / + `query.PgStatStatementsJITDefault` и селектор + `query.SelectStatStatementsJITQuery(version) (string, int, [2]int, int)` (возвращает + query + `Ncols` + `DiffIntvl` + `UniqueKey`). Этот таск компилируется и проходит тесты только + после слияния задачи 01. +- Никаких новых пакетов. + +**Edge cases:** +- `Test_filterViews` останавливается на версии 140000 (нет строки 150000), но `+1` применяется + единообразно на всех строках, потому что `NotRecordable`-сброс не зависит от версии (срабатывает + до версионного гейта). PG<15 строки тоже +1. +- `Test_filterViews` НЕ имеет отдельной name-map, которую надо пополнять. Другие тесты в + `record_test.go` (`TestFilterViews_NotRecordable`, `TestFilterViews_dropsExplicitNotRecordable`, + `TestFilterViews_Recordable`) строят собственные ad-hoc `view.Views{}` литералы и НЕ затронуты. +- `Test_app_record` (record_test.go:32) использует `countRecordable(view.New())` — `NotRecordable` + view автоматически исключается, дополнительных правок не требует. +- `TestView_VersionOK` (view_test.go:205-229) считает view, проходящие `VersionOK` для версии. + Новый view с `MinRequiredVersion: PostgresV15` пройдёт `VersionOK` только на 150000+; в таблице + теста есть строка 160000 (`total: 26`), где `statements_jit.VersionOK(160000) == true` — + значит `total` на этой строке нужно поднять 26 → 27. ВНИМАНИЕ: это третий count-тест, который + ломается; обязательно прогнать весь `./internal/view/...` и поправить строку 160000. Остальные + строки (140000 и ниже) не меняются — там `VersionOK(<150000) == false`. + +**Implementation hints:** +- Форма селектора JIT отличается от `stat_io`: `stat_io` возвращает 3 значения + `(string, int, [2]int)` (io.go:87), а JIT — 4 (добавлен `UniqueKey`, т.к. md5-ключ у JIT стоит + в конце и сдвигается с `Ncols`, тогда как у `stat_io` он фиксирован на col 0). `Configure()` + должна присвоить все 4. +- Ветка `statements_timings` в `Configure()` (view.go:373-375) патчит только `QueryTmpl` — это НЕ + образец для JIT, потому что у timings `Ncols`/`DiffIntvl`/`UniqueKey` константны. Образец — + ветка `stat_io` (view.go:385-387), но расширенная до `UniqueKey`. +- `Msg` служит двойную роль (Decision 4): это и подпись экрана в cmdline, и подсказка про пустой + экран при `jit=off` — текст должен явно упоминать пустое состояние. +- `report/report.go::doDescribe` НЕ трогать: bgwriter/replslots/stat_io (все `NotRecordable`) не + имеют там записей — это прямой прецедент (code-research §6). + +## Reviewers + +- **dev-code-reviewer** → `007-feat-pg-stat-statements-jit-task-02-dev-code-reviewer-review.json` +- **dev-security-auditor** → `007-feat-pg-stat-statements-jit-task-02-dev-security-auditor-review.json` +- **dev-test-reviewer** → `007-feat-pg-stat-statements-jit-task-02-dev-test-reviewer-review.json` + +## Post-completion + +- [ ] Записать краткий отчёт в [007-feat-pg-stat-statements-jit-decisions.md](007-feat-pg-stat-statements-jit-decisions.md) (Summary: 1-3 предложения, ревью со ссылками на JSON, без таблиц файндингов и дампов) +- [ ] Если отклонились от спека — описать отклонение и причину +- [ ] Обновить user-spec/tech-spec если что-то изменилось diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-code-reviewer-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-code-reviewer-review.json new file mode 100644 index 0000000..5c25c65 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-code-reviewer-review.json @@ -0,0 +1,14 @@ +{ + "reviewer": "dev-code-reviewer", + "status": "approved", + "summary": "Pure TUI plumbing for Task 03 is implemented exactly per spec: a 7th menuPgss item at index 6, its menuSelect case 6 handler, and statements_jit inserted into the statementsNextView cycle between wal and timings. Cross-file consistency holds — the statements_jit view is registered in internal/view/view.go (Task 02), viewSwitchHandler signature matches the call, index/slice alignment is correct, and `go build ./...` is clean. No issues found.", + "criticalIssues": [], + "suggestions": [], + "metrics": { + "filesReviewed": 2, + "criticalIssuesCount": 0, + "majorIssuesCount": 0, + "minorIssuesCount": 0, + "testCoverageAssessment": "adequate" + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-test-reviewer-review-round2.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-test-reviewer-review-round2.json new file mode 100644 index 0000000..3b3141b --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-test-reviewer-review-round2.json @@ -0,0 +1,21 @@ +{ + "reviewer": "dev-test-reviewer", + "status": "passed", + "summary": "Round-2 re-review of Task 03 (pg_stat_statements JIT) cumulative diff (impl 1d8d678 + test fix 217b69d). All three round-1 regressions are resolved. The stale assertions in top/menu_test.go and top/config_view_test.go were updated to match the new behavior, and the new statements_jit transition is now explicitly asserted. Verified empirically: `go test ./top/ -run 'Test_selectMenuStyle|Test_statementsNextView|Test_switchViewTo'` is GREEN — all three previously-failing tests pass, including Test_switchViewTo/14. The tests retain litmus-test integrity: they assert the actual next-view name and menu item count (real behavior), so removing the JIT case lines in statementsNextView/menu.go would make them fail. Test_doReload still FAILs, but this is pre-existing and out of scope: it panics with a nil pointer dereference at reload.go:18 (requires a live PostgreSQL connection), and was confirmed to fail identically on HEAD~2 (a clean worktree checkout reproduced the same panic at the same line). No new findings.", + "findings": [], + "metrics": { + "filesReviewed": 4, + "litmusTest": { + "checked": 3, + "passed": 3, + "failed": 0 + }, + "coverageAssessment": "adequate", + "pyramidBalance": { + "unit": 3, + "integration": 0, + "e2e": 0, + "assessment": "healthy" + } + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-test-reviewer-review.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-test-reviewer-review.json new file mode 100644 index 0000000..31e4cff --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03-dev-test-reviewer-review.json @@ -0,0 +1,46 @@ +{ + "reviewer": "dev-test-reviewer", + "status": "failed", + "summary": "The task's central premise — that top/menu.go selectMenuStyle/menuSelect and top/config_view.go statementsNextView have NO existing Go unit-test coverage — is factually wrong. Both functions ARE covered by existing table-driven tests in top/menu_test.go and top/config_view_test.go. The diff changes their behavior (7th menu item, wal->jit->timings cycle) but does not update the stale assertions, so it breaks 3 existing tests. Confirmed empirically: `go test ./top/` fails in Test_selectMenuStyle, Test_statementsNextView, and Test_switchViewTo/14. Since `make test` (race + coverage) is the project CI gate, the change as committed breaks the build pipeline. The 'no speculative tests for this layer' decision is reasonable as a forward-looking principle, but it does not excuse leaving the EXISTING regression tests broken.", + "findings": [ + { + "severity": "critical", + "category": "anti_pattern", + "location": "top/config_view_test.go:301 (Test_statementsNextView) and top/config_view_test.go:219 (Test_switchViewTo case 14)", + "issue": "The diff changes statementsNextView so that statements_wal now returns statements_jit (was statements_timings), but two existing test assertions still expect statements_timings. `go test ./top/` fails: Test_statementsNextView expected 'statements_timings' got 'statements_jit', and Test_switchViewTo/14_ expected 'statements_timings' got 'statements_jit'. These are real regression tests that passed the litmus test (they assert the actual next-view name, not a mock), and the diff left them stale.", + "recommendation": "Update both stale assertions to match the new cycle and add the missing jit hop. In Test_statementsNextView (config_view_test.go:301) change {current: \"statements_wal\", want: \"statements_timings\"} to {current: \"statements_wal\", want: \"statements_jit\"} and insert a new case {current: \"statements_jit\", want: \"statements_timings\"}. In Test_switchViewTo (config_view_test.go:219) change {current: \"statements_wal\", to: \"statements\", want: \"statements_timings\"} to want: \"statements_jit\" and add {current: \"statements_jit\", to: \"statements\", want: \"statements_timings\"}.", + "litmusTestFailed": false + }, + { + "severity": "critical", + "category": "anti_pattern", + "location": "top/menu_test.go:15 (Test_selectMenuStyle)", + "issue": "The diff adds a 7th menuPgss item, but Test_selectMenuStyle still asserts {menu: menuPgss, want: 6}. `go test ./top/` fails: Test_selectMenuStyle expected 6 got 7. This is a real existing test that verifies the menuPgss item count and now contradicts the implementation.", + "recommendation": "Change menu_test.go:15 from {menu: menuPgss, want: 6} to {menu: menuPgss, want: 7} so the asserted item count matches the new 7-item menuPgss slice.", + "litmusTestFailed": false + }, + { + "severity": "major", + "category": "missing_coverage", + "location": "Section: TDD Anchor / Acceptance Criteria (task-03.md)", + "issue": "The task TDD Anchor states 'every prior pgss-menu wiring and the x-cycle entries were added the same way and verified only by make build plus a manual TUI check' and 'these have no existing Go unit-test coverage'. This is false. Test_selectMenuStyle covers menuPgss item count and Test_statementsNextView + Test_switchViewTo cover the statements cycle, including the exact statements_wal transition this task modifies. Because the premise is wrong, the task omits the mandatory 'update the existing regression tests' step from the acceptance criteria, and the implementer correctly followed the (incorrect) task and shipped a build-breaking change. The new jit menu index (case 6) and the new wal->jit->timings hop are also left uncovered after the fix, even though the surrounding cases are tested.", + "recommendation": "Correct the TDD Anchor to acknowledge existing coverage in top/menu_test.go and top/config_view_test.go, and add an acceptance-criterion: 'Existing tests updated for the new item count and cycle order; new jit transition asserted; make test green.' Extend Test_statementsNextView with the statements_jit -> statements_timings case (catches the new hop). Optionally extend Test_switchViewTo with the statements_jit row. No new test file or speculative gocui-driver test is needed — the layer already has cheap, pure-function coverage, so the no-speculative-tests principle is satisfied by simply updating existing tests.", + "litmusTestFailed": false + } + ], + "metrics": { + "filesReviewed": 4, + "litmusTest": { + "checked": 3, + "passed": 3, + "failed": 0 + }, + "coverageAssessment": "adequate", + "pyramidBalance": { + "unit": 3, + "integration": 0, + "e2e": 0, + "assessment": "healthy" + } + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03.md new file mode 100644 index 0000000..c665e96 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-03.md @@ -0,0 +1,160 @@ +--- +status: done # planned -> in_progress -> done +depends_on: ["02"] # ID задач-зависимостей (строки: ["01", "02"]) +wave: 3 # волна параллельного выполнения +skills: [code-writing] # МАССИВ скиллов для загрузки +verify: user # local PG17: X → JIT opens; x cycles wal → jit → timings; make build clean +reviewers: [dev-code-reviewer, dev-test-reviewer] +teammate_name: +--- + +# Task 03: TUI menu item + x-cycle wiring + +## Required Skills + +Перед выполнением задачи загрузи: +- `/skill:code-writing` — [skills/code-writing/SKILL.md](~/.claude/skills/code-writing/SKILL.md) + +## Description + +Expose the new `statements_jit` view (registered in Task 02) through the two TUI entry points +that every other pgss sub-screen already uses: + +1. The uppercase `X` menu — add a 7th `menuPgss` item ("pg_stat_statements JIT compilation") + and its `menuSelect` handler so a DBA can pick the JIT screen explicitly. +2. The lowercase `x` cycle — insert `statements_jit` into `statementsNextView` between + `statements_wal` and `statements_timings`, so repeatedly pressing `x` walks through the JIT + screen in order. + +This is pure TUI plumbing — no query logic, no view registration (Task 02 owns that), and no +keybinding changes. The `X` key (`menuOpen(menuPgss, …)`) and the `x` key +(`switchViewTo(app, "statements")`) are already wired in `top/keybindings.go` and already guard +the absence of `pg_stat_statements` (empty `ExtPGSSSchema`), so they are not touched here. + +This task depends on Task 02: the `statements_jit` view must exist in the view map, otherwise +`viewSwitchHandler` would resolve a zero-value `View{}` when the menu/cycle targets it. + +## What to do + +1. In `top/menu.go`, `selectMenuStyle` → `case menuPgss` items slice (currently 6 entries, + indices 0–5), add a 7th entry at index 6 immediately after `" pg_stat_statements WAL usage"`: + `" pg_stat_statements JIT compilation"`. Keep the leading space to match the existing item + formatting. The menu UI auto-sizes its height to `len(s.items)` (menu.go:115) — do NOT touch + the `SetView` geometry. +2. In `top/menu.go`, `menuSelect` → `case menuPgss` inner `switch cy` (currently handles + `case 0`–`case 5` then `default`), add `case 6: viewSwitchHandler(app.config, "statements_jit")` + immediately after `case 5` (`statements_wal`) and before `default`. Leave the existing + `default` (`statements_timings`) unchanged. +3. In `top/config_view.go`, `statementsNextView`, insert `statements_jit` into the cycle between + `wal` and `timings`: change the `case "statements_wal":` body from + `next = "statements_timings"` to `next = "statements_jit"`, and add a new + `case "statements_jit": next = "statements_timings"`. Leave the trailing `default` unchanged. +4. Do NOT modify `top/keybindings.go` — `x` and `X` are already wired and already guard the + missing-pgss case. +5. Run `make build` to confirm the package compiles cleanly. + +## TDD Anchor + + + +There is no automated test for this layer and none is added here. The TUI menu list and the +`x`-cycle next-view chain have no existing Go unit-test coverage — every prior pgss-menu wiring +(`statements_io`, `statements_wal`) and the `x`-cycle entries were added the same way and +verified only by `make build` plus a manual TUI check. `top/menu.go` and `top/config_view.go` +exercise gocui UI objects and the live view map that cannot be driven without a running terminal ++ PostgreSQL. Verification for this task is therefore: `make build` (compile) + manual TUI check +on local PG17 (see Verification Steps). Do not invent or add speculative tests for this layer. + +## Acceptance Criteria + +- [ ] `top/menu.go`: `menuPgss` items slice has a 7th entry (index 6) + `" pg_stat_statements JIT compilation"` right after `" pg_stat_statements WAL usage"`. +- [ ] `top/menu.go`: `menuSelect` `case menuPgss` has `case 6: + viewSwitchHandler(app.config, "statements_jit")` before `default`. +- [ ] `top/config_view.go`: `statementsNextView` cycle is `… wal → jit → timings …` + (`case "statements_wal"` → `statements_jit`; new `case "statements_jit"` → + `statements_timings`). +- [ ] `top/keybindings.go` is unchanged. +- [ ] `make build` is clean. +- [ ] Manual (local PG17): pressing `X` shows a 7th menu item; selecting it opens the JIT screen. + Pressing `x` cycles `wal → jit → timings`. + +## Context Files + +**Feature artifacts:** +- [007-feat-pg-stat-statements-jit.md](007-feat-pg-stat-statements-jit.md) — user-spec +- [007-feat-pg-stat-statements-jit-tech-spec.md](007-feat-pg-stat-statements-jit-tech-spec.md) — tech-spec (Task 3; Solution §3; "How it works" steps 1–2; Backward Compatibility note on cycle order) +- [007-feat-pg-stat-statements-jit-code-research.md](007-feat-pg-stat-statements-jit-code-research.md) — code-research §5 (menu + cycle wiring, exact anchors) +- [007-feat-pg-stat-statements-jit-decisions.md](007-feat-pg-stat-statements-jit-decisions.md) — decisions log + +**Project knowledge:** +- [overview.md](.claude/skills/project-knowledge/overview.md) — project context (no project.md in this repo; overview.md is the entry point) +- [architecture.md](.claude/skills/project-knowledge/architecture.md) — package layout, data flow, view/menu wiring +- [patterns.md](.claude/skills/project-knowledge/patterns.md) — code/testing conventions, version branching + +**Code files:** +- [top/menu.go](top/menu.go) — modify: `selectMenuStyle` `case menuPgss` items (menu.go:53–60); `menuSelect` `case menuPgss` switch (menu.go:155–171) +- [top/config_view.go](top/config_view.go) — modify: `statementsNextView` (config_view.go:160–180) +- [top/keybindings.go](top/keybindings.go) — read only: `x` (keybindings.go:40), `X` (keybindings.go:45) already wired; NO change + +## Verification Steps + +- `make build` → builds `./bin/pgcenter` with no errors. +- Manual on local PG17 (`bin/pgcenter`): + - Press `X` → menu lists 7 items, the last being "pg_stat_statements JIT compilation". + Select it → the JIT screen opens (view `statements_jit`, its `Msg` shown on the cmdline). + - From a pgss screen press `x` repeatedly → the cycle reaches `statements_wal`, then the JIT + screen, then `statements_timings` (order: `… wal → jit → timings …`). + - Confirm no regression: every other pgss screen is still reachable via `X` and `x`. + +## Details + + + +**Files:** +- `top/menu.go` — current `case menuPgss` items slice has exactly 6 entries (menu.go:54–59): + timings, general, input/output, temp files input/output, temp tables (local) input/output, + WAL usage. Add the 7th. The `menuSelect` `case menuPgss` `switch cy` (menu.go:156–171) maps + `0→statements_timings, 1→statements_general, 2→statements_io, 3→statements_temp, + 4→statements_local, 5→statements_wal, default→statements_timings`. Add `case 6` → + `statements_jit` before `default`. The index must line up with the new item's slice position + (6). Menu height auto-sizes from `len(s.items)` at menu.go:115 — no geometry edit. +- `top/config_view.go` — `statementsNextView` (config_view.go:160–180) currently chains + `timings→general→io→temp→local→wal→timings`. Change `case "statements_wal"` target from + `statements_timings` to `statements_jit`, and add `case "statements_jit"` → + `statements_timings`. `default` stays `statements_timings`. + +**Dependencies:** Task 02 (the `statements_jit` view must be registered in `view.New()` / +`config.views`, otherwise `viewSwitchHandler` resolves an empty `View{}`). + +**Edge cases:** +- Missing `pg_stat_statements`: already handled — `menuOpen(menuPgss, …)` prints + "NOTICE: pg_stat_statements not found" when `ExtPGSSSchema == ""` (menu.go:110–113), and + `switchViewTo(app, "statements")` prints a NOTICE and keeps the current view (config_view.go:105). + No change needed. +- PG<15: the JIT view is gated by `MinRequiredVersion: query.PostgresV15` (Task 02). The runtime + collector guard (`view.VersionOK`) returns "selected statistics is not supported by current + version of Postgres" — same graceful degrade as `statements_wal` on PG<13. The menu/cycle + target it unconditionally; no extra guard is added here (consistent with existing behavior). +- Index drift: the `case 6` index and the slice position (index 6) must match; an off-by-one + here silently maps the menu item to the wrong view. + +**Implementation hints:** +- Match the existing leading-space + label formatting exactly (e.g. `" pg_stat_statements JIT + compilation"`). +- Insert `case 6` strictly between `case 5` and `default` in `menuSelect`; do not reorder other + cases. +- The cycle change is the one user-visible behavior shift (one extra stop inserted between `wal` + and `timings`) — the tech-spec Backward Compatibility section explicitly approves it; no other + transition is reordered. + +## Reviewers + +- **dev-code-reviewer** → `007-feat-pg-stat-statements-jit-task-03-dev-code-reviewer-review.json` +- **dev-test-reviewer** → `007-feat-pg-stat-statements-jit-task-03-dev-test-reviewer-review.json` + +## Post-completion + +- [ ] Записать краткий отчёт в [007-feat-pg-stat-statements-jit-decisions.md](007-feat-pg-stat-statements-jit-decisions.md) (Summary: 1-3 предложения, ревью со ссылками на JSON, без таблиц файндингов и дампов) +- [ ] Если отклонились от спека — описать отклонение и причину +- [ ] Обновить user-spec/tech-spec если что-то изменилось diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-04.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-04.md new file mode 100644 index 0000000..a754044 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-04.md @@ -0,0 +1,154 @@ +--- +status: done # planned -> in_progress -> done +depends_on: ["01", "02", "03"] # ID задач-зависимостей (строки: ["01", "02"]) +wave: 4 # волна параллельного выполнения +skills: [pre-deploy-qa] # МАССИВ скиллов для загрузки +verify: bash — make test && make lint green; CI matrix PG14-18 green +reviewers: [] # QA/verification task — ревьюеров нет +teammate_name: # имя агента-исполнителя (опционально; если не задано — генерируется по описанию задачи) +--- + +# Task 04: Pre-deploy QA + +## Required Skills + +Перед выполнением задачи загрузи: +- `/skill:pre-deploy-qa` — [skills/pre-deploy-qa/SKILL.md](~/.claude/skills/pre-deploy-qa/SKILL.md) + +## Description + +Финальная волна фичи 007 (`pg_stat_statements` JIT screen) — приёмочное тестирование уже +реализованного функционала (задачи 01–03 завершены и закоммичены). Цель: убедиться, что фича +собирается, проходит тесты и линт, и что выполнены **все** критерии приёмки из user-spec +(«Критерии приёмки») и tech-spec («Acceptance Criteria»). + +Это QA/верификационная задача — **код не меняется**. Если в ходе проверки находится дефект, +он не правится здесь: задача фиксирует факт и возвращает результат QA наверх (исправление — +отдельной итерацией в соответствующей задаче 01–03). + +Фича — TUI-only, `NotRecordable`. Часть критериев (поведение JIT-экрана, цикл `x`, фильтр `/`, +deform-колонки на реальной БД) проверяется не юнит-тестами, а вручную на локальном PG17 и через +CI-матрицу PG14–18. Локальная среда без PostgreSQL даёт `connection refused` на exec-тестах — +это нормально (тесты гейтятся через `t.Skipf`); реальная версионная проверка — на CI. + +## What to do + +- Прогнать сборку и проверки: `make build`, `make test` (race + coverage), `make lint`. Все три + должны быть зелёными. Зафиксировать фактический вывод (pass/fail, новые замечания линта). +- Пройтись по каждому критерию приёмки из user-spec и tech-spec (полный список — в секции + Acceptance Criteria ниже) и отметить, выполнен он или нет, с указанием, чем подтверждается + (тест / ручная проверка / CI). +- Проверить версионную ветку селектора по факту тестов: `go test ./internal/query/...` — обе + ветки (PG15/16: Ncols 13, DiffIntvl {6,10}, UniqueKey 11; PG17+: 15, {7,12}, 13) проходят. +- Проверить count-тесты: `go test ./internal/view/... ./record/...` — `TestNew` ожидает 27, + `Test_filterViews` `wantN +1` на всех строках. +- Убедиться, что в `report.go` **не** добавлена description-запись для `statements_jit` + (NotRecordable-прецедент). +- Вручную на локальном PG17 (с `pg_stat_statements` и `jit=on`) проверить поведение JIT-экрана: + `X` → 7-й пункт открывает `statements_jit`; `x` циклит `wal → jit → timings`; колонки, + сортировка по `gen_total` desc, фильтр `/` работают; при отсутствии JIT-активности экран пуст + и показан хинт. +- Подтвердить, что CI-матрица PG14–18 зелёная: PG14 → под-экран недоступен («not supported»), + PG17/18 → присутствуют deform-колонки. +- Собрать итог QA: PASS/FAIL по каждому критерию + общий вердикт (можно ли деплоить). + +## Acceptance Criteria + +Полный чек-лист приёмки фичи (объединение user-spec и tech-spec). Каждый пункт нужно проверить +и подтвердить источником (тест / ручная проверка / CI): + +- [ ] `make build` — бинарь собирается без ошибок. +- [ ] `make test` (race + coverage) — зелёный; новый тест селектора проходит; count-тесты + обновлены и проходят. +- [ ] `make lint` — без новых замечаний. +- [ ] В меню `X` — 7-й пункт `pg_stat_statements JIT`; выбор открывает `statements_jit`. +- [ ] Цикл `x` проходит через JIT: `… wal → jit → timings …`. +- [ ] На PG15/16 — базовый набор фаз (13 колонок); на PG17/18 — он же + `deform_total`/`deform_ms` + (15 колонок). +- [ ] Интервальные колонки (`*_ms`, `functions`) диффятся; `*_total` — кумулятивные текст-итоги. +- [ ] Строки с `jit_functions = 0` не показываются (SQL-фильтр `WHERE jit_functions > 0`). +- [ ] Дефолтная сортировка — по `gen_total` (desc); `OrderKey: 2`, `OrderDesc: true`. +- [ ] Смена колонки сортировки переупорядочивает строки; `*_total` сортируются численно по + длительности (3+ значные часы и `N days …` — не лексикографически). +- [ ] Фильтр `/` по `query`/`database` сужает набор строк. +- [ ] На PG < 15 под-экран недоступен, приложение не падает, меню/цикл деградируют корректно + («not supported», без пустого `View{}`). +- [ ] При `jit=off`/пустом наборе показывается хинт об отсутствии JIT-активности (`Msg`). +- [ ] View помечен `NotRecordable: true` — `pgcenter record`/`report` его пропускают; в + `report.go` нет description-записи. +- [ ] `SelectStatStatementsJITQuery` покрыт юнит-тестом на обе ветки (PG15/16 и PG17+): + query + Ncols + DiffIntvl + UniqueKey. +- [ ] Count-тесты обновлены: `view_test.go::TestNew` → 27; `record/record_test.go::Test_filterViews` + → `wantN +1`. +- [ ] Нет регрессий в существующих view/record/query тестах. +- [ ] CI-матрица PG14–18 зелёная (PG14 → «not supported»; PG17/18 → deform-колонки присутствуют). +- [ ] Итоговый вердикт QA сформирован (PASS/FAIL по каждому пункту + готовность к деплою). + +## Context Files + +**Feature artifacts:** +- [007-feat-pg-stat-statements-jit.md](docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit.md) — user-spec (секции «Критерии приёмки», «Как проверить») +- [007-feat-pg-stat-statements-jit-tech-spec.md](docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-tech-spec.md) — tech-spec (секции «Acceptance Criteria», «Agent Verification Plan») +- [007-feat-pg-stat-statements-jit-decisions.md](docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-decisions.md) — decisions log (отчёты задач 01–03) + +**Project knowledge:** +- [overview.md](.claude/skills/project-knowledge/overview.md) — фичи, поддерживаемая статистика, аудитория +- [architecture.md](.claude/skills/project-knowledge/architecture.md) — раскладка пакетов, поток данных, обработка версий PG +- [patterns.md](.claude/skills/project-knowledge/patterns.md) — паттерны кода, конвенции тестирования, версионное ветвление +- [deployment.md](.claude/skills/project-knowledge/deployment.md) — релизный процесс, CI/CD, версионная матрица + +**Code files (под проверку, не под правку):** +- [internal/query/statements.go](internal/query/statements.go) — JIT-консты + `SelectStatStatementsJITQuery` (задача 01) +- [internal/view/view.go](internal/view/view.go) — view `statements_jit` + `Configure()` (задача 02) +- [top/menu.go](top/menu.go), [top/config_view.go](top/config_view.go) — меню + `x`-цикл (задача 03) + +## Verification Steps + +- Шаг 1: `make build` → бинарь собирается без ошибок. +- Шаг 2: `make test` → зелёный; селектор-тест и count-тесты проходят; нет регрессий. +- Шаг 3: `make lint` → без новых замечаний. +- Шаг 4: `go test ./internal/query/...` → обе версионные ветки возвращают ожидаемые + Ncols/DiffIntvl/UniqueKey. +- Шаг 5: `go test ./internal/view/... ./record/...` → `TestNew` 27, `Test_filterViews` +1. +- Шаг 6: grep по `report.go` → нет description-записи для `statements_jit`. +- Шаг 7: ручная проверка на локальном PG17 → JIT-экран открывается из `X`, циклится `x`, + сортировка/фильтр/колонки/хинт работают. +- Шаг 8: CI-матрица PG14–18 → все джобы зелёные (PG14 «not supported», PG17/18 deform-колонки). +- Результат: чек-лист приёмки полностью отмечен; общий вердикт сформирован. + +## Details + +**Files:** код не меняется. Все перечисленные файлы — под чтение/проверку, а не под правку. + +**Dependencies:** зависит от задач 01, 02, 03 — должны быть завершены и закоммичены до старта QA. +Локально требуется Go 1.25+, `make`. Для ручной проверки — локальный PostgreSQL 17 с установленным +расширением `pg_stat_statements` и `jit=on`. CI-матрица PG14–18 — на стороне CI после push. + +**Edge cases:** +- Локально без PostgreSQL exec-тесты (версионная матрица) гейтятся `t.Skipf` — это ожидаемо, + не считать за провал. Реальная версионная проверка — на CI. +- PG14 (ниже `MinRequiredVersion PostgresV15`) → под-экран должен отдавать «statistics is not + supported by current version of Postgres», не пустой `View{}`, без паники. +- `jit=off` / нет запросов выше `jit_above_cost` → экран пуст по дизайну (фильтр + `WHERE jit_functions > 0`); подтверждается наличие хинта в `Msg`, а не баг. +- Count-тесты (`TestNew`, `Test_filterViews`) ловятся локально без PG — если они красные, это + реальный провал, а не skip (урок фичи 006). + +**Implementation hints:** +- Источник критериев приёмки — секция «Критерии приёмки» user-spec и «Acceptance Criteria» + tech-spec; «Agent Verification Plan» tech-spec задаёт, что проверяется автоматикой, а что + вручную/на CI. +- Ручную проверку JIT-генерации можно спровоцировать тяжёлым аналитическим запросом с + `SET jit_above_cost = 0` (см. user-spec «Пользователь проверяет»). +- Если найден дефект — зафиксировать (что, где, чем подтверждается) и вернуть в итоге QA; не + править код в этой задаче. + +## Reviewers + +Нет — QA/верификационная задача. Верификация = сам результат QA (вердикт по чек-листу). + +## Post-completion + +- [ ] Записать краткий отчёт в [007-feat-pg-stat-statements-jit-decisions.md](docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-decisions.md) (Summary: вердикт QA + статус критериев, без дампов) +- [ ] Если найдены отклонения от спека или дефекты — описать их и причину +- [ ] Обновить user-spec/tech-spec если что-то изменилось по итогам QA diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-reality-batch1.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-reality-batch1.json new file mode 100644 index 0000000..bbf6018 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-reality-batch1.json @@ -0,0 +1,27 @@ +{ + "validator": "dev-reality-checker", + "feature_base": "docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit", + "tasks_checked": ["01", "02", "03"], + "status": "approved", + "findings": [ + { + "severity": "minor", + "category": "feasibility", + "task": "01", + "issue": "Task line anchors for statements.go are slightly off but harmless: const block actually ends at line 303 (task says 303 — correct); PgStatStatementsIoDefault is at lines 55-67 (task says 55-67 — correct); SelectStatStatementsTimingQuery is at 305-315 (task says 305-315 — correct); SelectQueryReportQuery ends at 327 (task says 327 — correct). All verified accurate. No mismatch found; recorded only to note all anchors were checked.", + "fix": "No action needed — anchors verified accurate." + }, + { + "severity": "minor", + "category": "hints", + "task": "01", + "issue": "Implementation hints in Details section contain a concrete Go code block for SelectStatStatementsJITQuery (lines 160-168) and per-column SQL expression snippets. These are close to prescribing the full solution rather than pointing to a pattern. Borderline against the 'hints not implementations' rule, but mitigated because they explicitly mirror the existing SelectStatIOQuery pattern (io.go:87-92) which the task correctly references as the model.", + "fix": "Acceptable as-is given they mirror an existing in-repo pattern; could trim the full function body to a one-line signature + 'model SelectStatIOQuery' reference." + } + ], + "stats": { + "tasks_checked": 3, + "claims_verified": 41, + "issues_found": 0 + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-reality-batch2.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-reality-batch2.json new file mode 100644 index 0000000..a8cb2e3 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-reality-batch2.json @@ -0,0 +1,27 @@ +{ + "validator": "dev-reality-checker", + "feature_base": "docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit", + "tasks_checked": ["04"], + "status": "approved", + "findings": [ + { + "severity": "minor", + "category": "feasibility", + "task": "04", + "issue": "Task 04 references the description registry as being in 'report.go' (e.g. 'в report.go нет description-записи для statements_jit'). The per-view description map actually lives in report/report.go around line 622 (entries like \"statements_timings\": pgStatStatementsTimingsDescription). There is also a report/describe.go file with no such registry, and other report.go files exist (top/report.go, cmd/report/report.go). The unqualified 'report.go' is slightly ambiguous but resolves correctly to report/report.go; the QA check (grep for statements_jit in the registry) is sound.", + "fix": "Optional: qualify as report/report.go (description map at line ~622) to remove ambiguity with top/report.go and cmd/report/report.go." + }, + { + "severity": "minor", + "category": "feasibility", + "task": "04", + "issue": "Task 04 is a Final Wave QA task with depends_on: [01,02,03]; tasks 01-03 are NOT yet implemented in the codebase (verified: no PgStatStatementsJIT consts in internal/query/statements.go, no statements_jit entry anywhere in *.go, TestNew still asserts 26 not 27, statementsNextView still goes wal->timings with no jit stop). This is expected for a pre-implementation reality check of a QA task — Task 04's referenced artifacts (selector branches, view fields, Ncols/DiffIntvl/UniqueKey numbers, count-test bumps) are deliverables of 01-03, not preconditions. The dependency is correctly declared, so this is informational, not a defect.", + "fix": "No action — QA task must run after 01-03 are committed. The acceptance numbers (Ncols 13/15, DiffIntvl {6,10}/{7,12}, UniqueKey 11/13, TestNew 27, filterViews +1) trace exactly to tech-spec and must be re-verified once 01-03 land." + } + ], + "stats": { + "tasks_checked": 1, + "claims_verified": 14, + "issues_found": 2 + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-validation-batch1.json b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-validation-batch1.json new file mode 100644 index 0000000..f363bfb --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-task-validation-batch1.json @@ -0,0 +1,52 @@ +{ + "validator": "dev-task-validator", + "feature_base": "docs/features/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit", + "tasks_checked": ["01", "02", "03", "04"], + "status": "approved", + "findings": [ + { + "severity": "minor", + "category": "frontmatter", + "task": "01", + "section": "frontmatter / verify", + "issue": "verify field value is 'bash — go test ./internal/query/...' (a string), which is fine, but it duplicates the verify tool plus a command in one field where the template/agent treats verify as a single tool token (e.g. 'bash'). Tasks 03/04 mix styles too (03: 'user' + trailing comment; 04: 'bash — ...'). Consistency only — not blocking.", + "fix": "Optional: set verify to the bare tool ('bash'/'user') and keep the command in the per-task verification mapping. Cosmetic; value is a valid string so the type contract holds." + }, + { + "severity": "minor", + "category": "content", + "task": "01", + "section": "Details / Implementation hints", + "issue": "Details includes a ~10-line Go code block for the SelectStatStatementsJITQuery selector signature/body. This leans toward HOW (concrete implementation) rather than WHAT. It is a small signature sketch (mirrors SelectStatIOQuery) and is bounded, so it is borderline rather than a true violation, but the rules flag pseudocode/implementation in Details.", + "fix": "Optional: reduce the code block to the function signature line only ('func SelectStatStatementsJITQuery(version int) (string, int, [2]int, int)') and describe the two-branch return values as prose, instead of showing the full if/return body." + }, + { + "severity": "minor", + "category": "content", + "task": "01", + "section": "Context Files", + "issue": "Two feature-artifact references are plain text, not markdown links: '007-feat-pg-stat-statements-jit-decisions.md — decisions log' and the code-research entry has a link but decisions log does not. Mandatory decisions log link is present elsewhere as plain text only on task 01.", + "fix": "Wrap the decisions log as a markdown link: [007-feat-pg-stat-statements-jit-decisions.md](007-feat-pg-stat-statements-jit-decisions.md) — matches tasks 02/03/04 which already link it." + }, + { + "severity": "minor", + "category": "content", + "task": "01", + "section": "Context Files / Project knowledge", + "issue": "Link label says 'project.md' but the path points to '.claude/skills/project-knowledge/overview.md'. Label/target mismatch (tasks 02/03/04 correctly label it 'overview.md').", + "fix": "Rename the label to [overview.md](.claude/skills/project-knowledge/overview.md) to match the actual target and the other three tasks." + }, + { + "severity": "minor", + "category": "consistency", + "task": "04", + "section": "Context Files", + "issue": "Feature-artifact links use full repo-root-relative paths (docs/features/007-.../007-...md) whereas tasks 01-03 use bare-filename links relative to the task file's own directory. Both resolve, but the batch is internally inconsistent. Post-completion decisions-log link in task 04 also uses the long form.", + "fix": "Optional: normalize task 04's feature-artifact links to bare filenames (e.g. [007-feat-pg-stat-statements-jit.md](007-feat-pg-stat-statements-jit.md)) for consistency with tasks 01-03." + } + ], + "stats": { + "tasks_checked": 4, + "issues_found": 5 + } +} diff --git a/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-tech-spec.md b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-tech-spec.md new file mode 100644 index 0000000..ddb27b8 --- /dev/null +++ b/docs/features/archive/007-feat-pg-stat-statements-jit/007-feat-pg-stat-statements-jit-tech-spec.md @@ -0,0 +1,297 @@ +--- +created: 2026-06-22 +status: approved +branch: feature/pg-stat-statements-jit +size: S +--- + +# Tech Spec: pg_stat_statements JIT screen + +## Solution + +Add a 7th `pg_stat_statements` sub-screen (`statements_jit`) showing per-statement JIT +compilation cost, modeled on the existing `statements_timings` sub-screen. The screen is built +from three layers, all reusing established pgcenter patterns: + +1. **Query layer** (`internal/query/statements.go`) — two version-specific query consts + (PG15/16 base, PG17+ with deform columns) and a `SelectStatStatementsJITQuery(version)` + selector returning query + `Ncols` + `DiffIntvl` + `UniqueKey` (the `SelectStatIOQuery` + model, because the JIT column *count* — not just names — changes across versions). The + SELECT follows the canonical pgss row layout (`user, database, <*_total cumulative>, + <*_ms + functions interval>, queryid, query`) and filters to rows with JIT activity via + `WHERE jit_functions > 0`. +2. **View registration** (`internal/view/view.go`) — a `statements_jit` view entry gated by + `MinRequiredVersion: query.PostgresV15`, marked `NotRecordable: true`, sorted by `gen_total` + desc, with a `Msg` that doubles as the empty-screen hint; plus a `Configure()` case that + patches `QueryTmpl/Ncols/DiffIntvl/UniqueKey` from the selector per detected version. +3. **TUI wiring** (`top/menu.go`, `top/config_view.go`) — a 7th `menuPgss` item + `menuSelect` + case, and insertion of `statements_jit` into the `x`-cycle (`… wal → jit → timings …`). + +No record/report wiring (TUI-first principle, `NotRecordable`). No keybinding changes (`X`/`x` +already route to the pgss menu/cycle). PG<15 degrades gracefully via the existing runtime +`VersionOK` guard. + +## Architecture + +### What we're building/modifying + +- **`internal/query/statements.go`** — add `PgStatStatementsJITPG15` and + `PgStatStatementsJITDefault` consts + `SelectStatStatementsJITQuery(version) (string, int, + [2]int, int)` selector. Purpose: produce the version-correct JIT query and its layout + metadata. +- **`internal/view/view.go`** — add the `statements_jit` view entry and a `Configure()` case. + Purpose: register the screen, gate it to PG15+, mark it non-recordable, and bind the + version-selected query/layout at startup. +- **`top/menu.go`** — add the menu item + select case. Purpose: make JIT reachable from the + `X` menu. +- **`top/config_view.go`** — extend `statementsNextView`. Purpose: include JIT in the `x` + cycle. +- **Test files** — `internal/query/statements_test.go` (selector + exec coverage), + `internal/view/view_test.go` (count bump + optional view guard), `record/record_test.go` + (filtered-count bump). + +### How it works + +1. At startup `view.New()` registers `statements_jit` in the view map (regardless of PG + version). `view.Views.Configure(opts)` runs the `statements_jit` case, calls + `SelectStatStatementsJITQuery(opts.Version)`, and patches the view's `QueryTmpl`, `Ncols`, + `DiffIntvl`, `UniqueKey`; then `query.Format()` resolves `{{.PGSSSchema}}` / + `{{.PgSSQueryLenFn}}`. +2. The DBA presses `X` → selects "pg_stat_statements JIT" (or cycles with `x`). + `viewSwitchHandler` swaps the active view; `printCmdline` shows the view `Msg`. +3. The collector runs `view.VersionOK(version)` first — on PG<15 it returns "selected + statistics is not supported by current version of Postgres" and never queries (same as + `statements_wal` on PG<13). On PG15+ it runs `view.Query`, diffs the `DiffIntvl` columns, + matches rows across samples on the `UniqueKey` (md5 queryid), and sorts by `OrderKey` (2, + `gen_total`, desc) — the duration-aware branch in `internal/stat/postgres.go::sort` (via + `parseDuration`, handling `HH:MM:SS` with 3+ digit hours and `N days HH:MM:SS`) orders the + `*_total` text durations numerically, the same path the timings-screen totals rely on. This + is an undocumented invariant: `OrderKey` pointing at a `*_total` text column sorts correctly + only because of that branch. +4. Rows are pre-filtered in SQL by `WHERE jit_functions > 0`, so under `jit=off`/low activity + the screen is empty; the `Msg` text explains why. + +## Decisions + +### Decision 1: Selector returns layout metadata (query + Ncols + DiffIntvl + UniqueKey) +**Decision:** `SelectStatStatementsJITQuery(version int) (string, int, [2]int, int)` returns the +query template, `Ncols`, `DiffIntvl`, and `UniqueKey`; `Configure()` patches all four onto the +view. Two-way branch: `version >= 170000` → `PgStatStatementsJITDefault` (15 cols), else → +`PgStatStatementsJITPG15` (13 cols). +**Rationale:** Unlike `statements_timings` (whose column *count* stays 13 across PG12/13/17 +variants, so only `QueryTmpl` is swapped), the JIT column count changes (13 vs 15) because PG17 +adds `deform_total`/`deform_ms`. With the synthetic md5 `queryid` as `UniqueKey`, columns cannot +be hidden (the align path floors every column at width 8 and is positional — ADR +`[006-feat-pg-stat-io]`), so each version needs a distinct column *set*, and `Ncols`/`DiffIntvl`/ +`UniqueKey` must move with it. Returning all four is explicit and mirrors `SelectStatIOQuery` +(which returns `(string, int, [2]int)`; we add `UniqueKey` because, unlike `stat_io`'s key at a +fixed col 0, the JIT key sits at the end and shifts with `Ncols`). +**Alternatives considered:** (a) Return only the query (timings model) — rejected: leaves stale +`Ncols`/`DiffIntvl`/`UniqueKey` for PG17. (b) Return 3-tuple and compute `UniqueKey = Ncols-2` +in `Configure()` — works but hides a layout invariant in arithmetic; explicit return is clearer +and test-checkable. (c) Hide deform columns on PG15 via `ColsWidth` — impossible (ADR [006]). + +### Decision 2: Column layout — single cumulative-total + single interval block (no doubling) +**Decision:** Columns: `user(0), database(1), gen_total, inline_total, opt_total, emit_total +[, deform_total], gen_ms*, inline_ms*, opt_ms*, emit_ms* [, deform_ms*], functions*, queryid, +query`. `*_total` are cumulative text durations (`date_trunc('seconds', round(