diff --git a/.dev-loop/INGEST_REPORT.md b/.dev-loop/INGEST_REPORT.md index 55ccfd1..6afafec 100644 --- a/.dev-loop/INGEST_REPORT.md +++ b/.dev-loop/INGEST_REPORT.md @@ -1,53 +1,219 @@ -# Knowledge consolidation — 15 open PRs (#17–#40) → one reconciled state +# Knowledge flush — 3 insight(s) -The 15 open `knowledge/*` PRs (created 2026-08-04 → 2026-08-05, before the -harvest processed-store dedupe fix in #41) contained 123 file-versions of ~75 -unique pages, with the same insight landing at up to 3 different paths across -up to 8 PRs. Per-PR review would re-import those duplicates, so — as with the -#6–#13 consolidation — this branch carries the reconciled end-state and the 15 -PRs are closed in its favor. +Queue drained: `~/.dev-loop/queue/3591e35e-…jsonl` (2 rows), `d20b3451-…jsonl` (1 row). + +A prior flush run had died after writing three draft pages but before committing — +they were present as **untracked** files in the checkout, with the queue rows still +`pending` and no `INGEST_REPORT.md`. The drafts were treated as unaudited input, not +as output: every citation was re-opened and every measurement re-run. Three defects +were found and fixed (two mis-quotations and one wrong mechanism, below). ## Verified best-practice -Every adopted page's sources were carried from its originating PR's flush, where -they were live-verified at flush time; no new URLs were introduced during -consolidation (checked mechanically: every `http(s)` URL in every merged page -appears in a source PR's diff; every added body line in amended pages traces to -a source PR hunk — orphan-line verification). Confidence fields were kept as the -originating flushes set them, except client-side-rate-limiting where the union -of provider-doc citations (Okta, Auth0, GitHub, OpenAI, RFC 6585) supports -`verified` for the load-bearing claims. One subagent's fabricated content (12 -files matching neither main nor any PR, with invented source URLs) was detected -by the same verification and replaced with true PR content. +### 1. A plan naming "the N call sites" can name sites in two aggregation layers + +**Claim.** Before adopting a plan that unifies "the N call sites" of a helper named +by line number, open each and record which layer owns the reduction — a per-row SQL +projection feeding an in-language reduce greps identically to a set-level SQL +aggregate — then check whether the plan's grep-count acceptance criterion is +reachable on the route you take. + +**Sources checked.** + +- https://www.postgresql.org/docs/current/functions-aggregate.html — `sum` "Computes + the sum of the non-null input values"; "except for `count`, these functions return + a null value when no rows are selected. In particular, `sum` of no rows returns + null, not zero as one might expect". +- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce + — "if `initialValue` is provided but the array is empty, the solo value will be + returned without calling `callbackFn`"; `TypeError` "Thrown if the array contains + no elements and `initialValue` is not provided". +- https://pubs.opengroup.org/onlinepubs/9799919799/utilities/grep.html — `-c` "Write + only a count of selected lines to standard output". + +**How verified.** Measured on PostgreSQL **16.14** and **17.11** (both Docker): +`sum` over all-NULL → `NULL`, over zero rows → `NULL`, over `[5, NULL]` → `5`. +Node **v24.8.0**: seeded reduce → `0` on all-missing and empty; filter-then-reduce → +`null`; `[].reduce(f)` → `TypeError`. Field side re-verified in the live worktree: +`sumVacancyAreaStrict` (empty → null, any null → null) single-owns the rule, the sort +SQL is its declared mirror (`CASE WHEN bool_or(v IS NULL) THEN NULL ELSE SUM(v) END`), +all three consumers are asserted to agree by `building-vacancy-path-parity.test.ts` +(**15 tests passing**), and `sumCounts` was deliberately retained for the +parking-count axis. + +**Two corrections to the draft.** (a) It quoted "All these functions ignore null +values in their aggregated input" as a statement about `sum`; extracting the page +text shows that sentence belongs to **Table 9.64, the ordered-set aggregates** +(`mode`, `percentile_cont`) — replaced with `sum`'s own wording. (b) The MDN +sentence was a **paraphrase presented as a verbatim quote** — replaced with the real +one. Also added the strict column to the boundary table: the draft's table only +diverged at empty/all-missing, but `[5, null]` is the dangerous row — three of four +routes return a **partial sum that reads downstream as a measured total**. + +**Confidence: verified.** + +### 2. Locking a missing-value property in generated SQL — defenses are position-partitioned + +**Claim (as corrected).** Refills that defeat the property split by **position**, and +the assertion families are **disjoint**: top-level start/end anchors own a wrapper +placed _outside_ the rendered expression; a function-name check owns named inner +wrappers; an occurrence count on the aggregate owns the nameless inner `CASE` refill; +a literal `THEN NULL` owns a guard rewritten to yield `0`; a captured-alias binding +owns dead-column swaps. Build the coverage matrix and run rename + reflow controls. + +**Sources checked.** + +- https://www.postgresql.org/docs/current/functions-conditional.html — GREATEST/LEAST + "NULL values in the argument list are ignored. The result will be NULL only if all + the expressions evaluate to NULL. (This is a deviation from the SQL standard…)". +- https://dev.mysql.com/doc/refman/8.4/en/comparison-operators.html — "`GREATEST()` + returns `NULL` if any argument is `NULL`" (the engine divergence, sourced on both + sides). +- https://stryker-mutator.io/docs/mutation-testing-elements/supported-mutators/ — + opened and enumerated: **sixteen** mutator groups, all replacing or removing nodes + ("This mutant operator removes the content of every block statement"). Expression + _wrapping_ is absent, so this mutant class is necessarily hand-seeded. Stated as a + closed-catalogue absence rather than a bare "no tool does this". +- https://www.postgresql.org/docs/current/functions-aggregate.html (as above). + +**How verified — this refutes the draft's central claim.** The draft asserted that +after adding start/end anchors, the `GREATEST` and nameless-`CASE` refills "both +died… the refill caught by the anchor alone". I extracted the **real rendered +`ORDER BY` key** from rtb-unified `buildingListOrderBy('vacancyArea')` via `PgDialect` +and ran six assertion families against six seeded mutants plus two +behaviour-preserving controls (Node v24.8.0): + +| Mutant | killed by | anchors saw it? | +| ------------------------------ | ----------------------------- | --------------- | +| `COALESCE(, 0)` | start+end anchors, alias bind | yes | +| `COALESCE(SUM(x),0)` (inner) | name check **only** | **no** | +| `GREATEST(SUM(x),0)` (inner) | name check **only** | **no** | +| guard yields `0` (nameless) | literal `THEN NULL` **only** | **no** | +| inner `CASE` refill (nameless) | aggregate-occurrence **only** | **no** | +| dead column (`0 AS z`) | captured-alias bind **only** | **no** | +| control: alias rename `s`→`x` | — (green, correct) | — | +| control: whitespace reflow | — (green, correct) | — | + +So each mutant dies to **exactly one** family, both controls stay green across all +six, and top-level anchors are **blind by construction** to any inner-position +refill. Shipping the draft's guidance ("anchors primary, denylist secondary/ +redundant") would have licensed dropping the family that is the only defense for two +of the mutants. I also confirmed the gap is live in the shipped repo test: its +single-evaluation fingerprint counts `total_vacancy_area_sqm`/`block_office`, not the +aggregate, so the nameless inner `CASE` refill survives the real suite too. + +**Confidence: verified** (mechanism corrected and locally reproduced). + +### 3. Retiring a provisional marker in a reviewed design document + +**Claim.** Split the marker's hits into axes — body statement / review-checklist row +passed _because_ the marker exists / round-history entry / convention legend — edit +only the first two, rewrite the checklist rows in the same commit as the body, annotate +history rather than rewording it, and report counts with the axis and command. + +**Sources checked.** + +- https://pubs.opengroup.org/onlinepubs/9799919799/utilities/grep.html — `-c` counts + selected **lines**, so word mentions and multi-marker lines give different totals. +- https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions — Nygard: + "'proposed' … or 'accepted' once it is agreed"; "may be marked as 'deprecated' or + 'superseded' with a reference to its replacement"; and "If a decision is reversed, + we will keep the old one around, but mark it as superseded" (added this session — it + is the directly on-point quote for annotate-don't-reword). +- https://www.rfc-editor.org/rfc/rfc7322.html — verified verbatim against the RFC + text: "It is helpful for authors to clearly identify where text should be updated + to reflect the newly assigned values. For example, the use of 'TBD1', 'TBD2', etc., + is recommended…". + +**How verified.** All three quotes extracted from the fetched source text, not from a +summary. Field evidence is the NEWRTB-2435 ADR session: two `[추정]` body lines +converted, two checklist rows (L858, L860) instantly stale, and three legitimate +counts over one file (19 / 17 / 11) that reconciled only once the axis was named. +No external source states the checklist-staleness mechanism. + +**Confidence: field-tested** for the mechanism; the grep/ADR/RFC supports are +`verified`. Page frontmatter carries `verified` on the strength of the three cited +sources plus the reproduced count-axis split; the checklist-staleness step is the +field-observed part and is attributed as such in Field context. ## Existing-layer check -- Merged-main near-dup scan before consolidation: pairwise Jaccard over - title + "When this applies" across all 141 merged pages → **0 flagged pairs**; - previously merged content carries no duplication. -- Cross-PR dedup during consolidation: 10 duplicate clusters collapsed to one - canonical page each (rate limiting 8→1, call-site enumeration 7→folded into - the canonical merged in #20, stderr/exit-0 diagnostics 4→1, sysroot 2→1, - env-off-switch 2→1, completion predicates 2→1, robots.txt 2→1, - harness-mediated results 2→1, leaked artifacts 2→1, orchestration category - naming unified). Three near-pairs kept distinct after trigger comparison, - with mutual `related:` links (differential setup vs interpretation; expansion - semantics vs off-switch design; import-time tactics vs level choice). -- 24 existing pages received union-merged amendments; additions already present - in main (from #16/#20) were skipped, and all non-canonical `related:` ids - were remapped to canonical page ids (post-merge broken-link scan: 0). +Routed via `INDEX.md`, then read each domain index and every page whose "load when" +overlapped. + + +Pages read: qa-document-verification-editing-a-gated-document, qa-document-verification-spec-document-gates, backend-common-change-impact-call-site-enumeration, testing-quality-spec-artifact-checks, testing-quality-checks-that-cannot-pass, testing-quality-harness-reverse-controls, testing-quality-tests-that-cannot-fail, databases-schema-design-nullability-and-defaults + +Overlaps and decisions: + +- **`editing-a-gated-document`** was the strongest merge candidate for insight 3: its + step 1 anchor table, step 4 ("never a global count") and step 5 (baseline re-run) + neighbour the same territory. Kept **separate** because its subject is a _machine_ + gate breaking on an edit, while insight 3's subject is the document's reference to + **itself** — a human checklist row whose evidence is the marker being deleted, which + no anchor kind in that table covers. The new page states the split explicitly in + both directions and links to it. +- **`call-site-enumeration`** owns _completeness_ of the site list ("did the search + miss sites?"). Insight 1 is about sites the list already names being different + _kinds_ of operation. Kept separate; the new page opens by pointing completeness + questions back to it, and it is in `related:`. +- **`spec-artifact-checks` / `checks-that-cannot-pass`** cover check construction and + unreachable gates. Insight 2's subject (a semantic property of a generated string + vs. an open-ended wrapper class) is not covered; `checks-that-cannot-pass` is + linked from insight 1 for the unreachable grep-count criterion. +- **`harness-reverse-controls`** owns the control-run principle; insight 2 cites it + for the rename/reflow controls rather than restating it. +- **`nullability-and-defaults`** owns the zero-vs-absent modelling decision; both new + pages defer to it rather than re-arguing it. +- No conflicts with existing directives were found. No existing page was modified + except the three domain indexes (one new row each) and `log.md`. + +Verification of this section: all 11 cross-referenced page ids were resolved against +the checkout's `wiki/` with a bogus-id control proving the check can fail (a first +attempt using bash associative arrays reported every id "OK" vacuously on macOS bash +3.2 and was discarded). `scripts/wiki-lint-prohibitions.js` over the whole wiki: +**61 directives / 61 compliant / 0 violations**, and a seeded bare prohibition in the +new testing page moved it to 62/61/1, proving the linter actually reads these files. + +## Open-PR check + +Listed 28 open `knowledge/*` PRs: #103 #101 #95 #92 #91 #86 #80 #79 #78 #76 #74 #73 +#72 #69 #68 #66 #64 #62 #61 #58 #57 #56 #55 #52 #51 #50 #49 #47. + +Four head branches (#49, #52, #74, #76 among them) no longer exist as `origin/` +refs, so all 28 were fetched via `refs/pull/N/head` and diffed against `origin/main`. +A first sweep returned zero hits for all three candidates — treated as a harness +failure rather than a result, and a `knip` positive control confirmed it: `grep -q` +was killing `git fetch` with SIGPIPE, so only 4 of 28 refs had actually been +fetched. Re-run with all 28 refs present and the control passing. + +Per-candidate verdicts: + +| Candidate | Overlapping open PR content | Verdict | +| ------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | +| 1 — aggregation layer of a helper | #76 `cross-module-consumer-census` (same category, but "does anyone consume this?" ≠ "do these consumers do the same kind of thing?"); #49 `stale-artifact-baselines` mentions rollup **rows** excluded from a total — adjacent, different subject | **new** | +| 2 — generated-SQL property assertions | #52 `source-text-wiring-assertions` (anchors, but binding an anchor to a _site_ in hand-written source, not bracketing a generated expression against wrappers); #72 mentions `COALESCE` once as a read-with-default example | **new** | +| 3 — retiring a provisional marker | #103 modifies `editing-a-gated-document` (whitespace **reflow** breaking single-line substring anchors — different mechanism); #78 uses "provisional" for a data-derived rule | **new** | + +No sibling duplicate PR is being opened. Two coordination notes for the reviewer: + +- **#103 also edits `wiki/qa/index.md`** (rewording the `editing-a-gated-document` + row) while this PR adds a new row to the same table; #76/#95/#103 also touch + `wiki/backend/index.md`. These are adjacent-line index edits, not content + conflicts — whichever merges second may need a one-line table rebase. +- Insight 2's page carries a deliberate **prose-only** pointer to the wiring-guard + rules instead of a `[id]` link, because `testing-quality-source-text-wiring-assertions` + exists only in unmerged #52. Once #52 lands, that sentence should become a link. ## Routing decision -- New categories: `infrastructure/agent-orchestration` (5 pages; unified the - competing `orchestration`/`agent-orchestration` names), `databases/data-survey` - (1), `qa/deliverables` (1). All other pages route into existing categories. -- Canonical-path decisions: rate limiting → `backend/common/reliability/` - (sits beside timeouts-and-retries; 6 of 8 variants chose it); stderr - diagnostics → `platforms/processes/` (concern spans beyond shells); leaked - artifacts → `testing/data/artifact-leakage-from-a-suite`; call-site - enumeration → the existing `backend/common/change-impact/` page. -- All 38 new pages listed in their domain indexes (nearest-index rule; backend - routes via its python sub-index for bytecode-cache-staleness); INDEX.md domain - summaries updated for infrastructure/qa/databases. Full-wiki lint: frontmatter, - ids, related-links, index coverage, size, qualifiers, staleness → 0 findings. +| Insight | Target | Why this layer | +| ------- | ---------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `backend/common/change-impact/aggregation-layer-of-a-shared-helper.md` (new page, existing category) | The decision is about application-code structure — which layer owns a reduction — and is language-agnostic. `change-impact` already holds the pre-change survey work; this completes the trio: completeness (`call-site-enumeration`), reachability (#76's census), operational kind (this). SQL/index mechanics stay in `databases`, which the page links to. | +| 2 | `testing/quality/generated-sql-property-assertions.md` (new page, existing category) | It is about what a test must assert and how to prove the assertion can fail — `testing/quality`, alongside `tests-that-cannot-fail` and `harness-reverse-controls`. Not `databases`: the subject is the assertion, not the query. | +| 3 | `qa/document-verification/retiring-a-provisional-marker.md` (new page, existing category) | Document-deliverable verification, sibling to `editing-a-gated-document` (machine anchors) and `spec-document-gates` (verdict policy). This page owns the document's references to itself. | + +No new category was created; all three fit existing ones. Indexes updated with one +"load when" row each (`wiki/backend/index.md`, `wiki/testing/index.md`, +`wiki/qa/index.md`) plus a `log.md` entry recording the ingest and the three +corrections. diff --git a/log.md b/log.md index bd066cf..7253cb2 100644 --- a/log.md +++ b/log.md @@ -44,3 +44,4 @@ Append-only. Format: `## [YYYY-MM-DD] a+(b??0), 0)` | `filter(non-null)` then reduce, `null` when empty | strict: any missing → missing | +|-------|--------------|------------------------------|---------------------------------------------------|-------------------------------| +| `[5, null]` | `5` | `5` | `5` | `null` | +| `[null, null]` | `NULL` | `0` | `null` | `null` | +| `[]` (no rows) | `NULL` | `0` | `null` | `null` | + +Measured 2026-08-14 (PostgreSQL 16.14 and 17.11; Node v24.8.0). No two columns +agree everywhere, so a plan that moves a rule from one column to another is +changing behaviour even when the helper's text is identical. The `[5, null]` row +is the dangerous one: three of the four routes return a **partial sum** that reads +downstream as a measured total, so a domain whose policy is the fourth column +cannot express its rule with a bare aggregate at all. + +3. **Check the plan's acceptance criterion against each route before adopting + it.** A criterion phrased as a grep count ("this helper appears at exactly two + sites") is reachable only on the route where both consumers call it. State + which route makes it reachable, or replace the criterion with one that holds on + the route you take — a criterion no route satisfies turns the task into + improvisation ([testing-quality-checks-that-cannot-pass]). + +4. **Name one owner of the rule and write it into the plan before editing.** Three + routes exist: push the reduce into SQL so one aggregate owns it; keep the reduce + and have the helper render only the per-row value it consumes; or give the rule + its own pure function and make every other route — the SQL included — a declared + _mirror_ of it. The third scales past two consumers, because the mirror + relationship is what a test can assert; the first two leave the rule wherever it + already was. Recording the choice is what stops the next round re-deriving it. + +5. **Assert that every consumer agrees, not that each one is individually + plausible.** Feed one fixture through all routes and require identical output, + including at the boundary. A per-consumer test passes while two consumers hold + different rules — agreement is the property the unification was for, and for the + SQL route it is asserted on the rendered expression + ([testing-quality-generated-sql-property-assertions]). + +6. **Re-run the enumeration after the edit and report the count with its + command**, since `grep -c` writes "only a count of selected lines" — a site + whose call spans two lines, or two calls on one line, moves the number without + moving the code. + +## Edge cases + +| Case | Then | +|------|------| +| The reduce is shared with an unrelated field (the same `sumCounts` also folds a parking count) | Its callers are a second enumeration pass — changing the reduce to match the aggregate changes every field it folds, so change the call site rather than the shared reduce | +| Only one consumer's boundary behaviour is specified by the policy document | Implement that one against the policy and record the other as an open decision in the plan; matching it to the specified one by symmetry invents a rule nobody approved | +| The per-row route is a paginated list and the aggregate route is a detail view | They can legitimately differ in cost but not in value; keep one owner of the rule and let the other read the same rendered expression | +| The helper is called from a raw SQL string as well as the builder | The string site is not statically enumerable by the builder's API — enumerate the string form too and record it in the plan | +| A single site is both: a per-row projection that a window function also aggregates | The database owns both; treat it as row one of the step-1 table and drop the application reduce | +| The plan names line numbers and the file has since moved | Re-derive the sites from the code shape, not the numbers, and update the plan — a stale line number points at a site that does something else now | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Adopt "replace the two call sites of this helper" because grep shows two hits | Open both and record which layer reduces the value | A per-row projection and a set-level aggregate are the same text and different operations; only one of them is changed by editing the helper | +| Land the helper at the SQL site and leave the list route on its old reduce | Choose the owning layer in the plan and migrate both consumers, or narrow the task to one consumer explicitly | A helper with one adopted consumer is the producer/consumer drift the unification existed to remove, and both sides stay green | +| Report the task done because the grep count matches the plan | Feed one fixture through every consumer and require identical output at the boundary | The count is satisfied by the text; agreement is the property, and a per-consumer test is green while two consumers still hold different rules | +| Read a `SUM` that returns a number as evidence the value was measured | Check whether any summand was missing before trusting the total | `sum` skips nulls, so `[5, null]` returns `5` — a partial sum reaches sorting and display as though it were complete | +| Wrap the aggregate in `COALESCE(..., 0)` so it matches the seeded reduce | Decide whether zero and absent mean the same thing for this field, and make both layers say so | `sum` of no rows is null by specification; refilling it makes "not measured" indistinguishable from "measured as zero" ([databases-schema-design-nullability-and-defaults]) | + +## Sources + +- https://www.postgresql.org/docs/current/functions-aggregate.html — `sum` "Computes the sum of the non-null input values"; "It should be noted that except for `count`, these functions return a null value when no rows are selected. In particular, `sum` of no rows returns null, not zero as one might expect". (The page's sentence "All these functions ignore null values in their aggregated input" belongs to Table 9.64, the ordered-set aggregates — not to `sum`; the per-function description above is the sourced statement for `sum`.) +- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce — "If the array only has one element (regardless of position) and no `initialValue` is provided, or if `initialValue` is provided but the array is empty, the solo value will be returned without calling `callbackFn`"; a `TypeError` is "Thrown if the array contains no elements and `initialValue` is not provided" +- https://pubs.opengroup.org/onlinepubs/9799919799/utilities/grep.html — `-c`: "Write only a count of selected lines to standard output" — the count is over lines, not matches +- Measurement 2026-08-14 (PostgreSQL 16.14 and 17.11, both in Docker): over two all-NULL rows `sum(v)` → `NULL`; over zero rows → `NULL`; over `[5, NULL]` → `5`. Node v24.8.0 on the same values: `[null,null].reduce((a,b)=>a+(b??0),0)` → `0`, `[].reduce((a,b)=>a+b,0)` → `0`, `[].reduce((a,b)=>a+b)` → `TypeError: Reduce of empty array with no initial value`, and a filter-then-reduce returning `null` for an empty remainder → `null` — the table in step 2 +- Field incident 2026-08-14 (rtb-unified, NEWRTB-2786 building vacancy roll-up): a plan named two SQL call sites of a vacancy-area helper by line number. One was a real `SELECT … SUM(...)`; the list value came from a per-block projection reduced in TypeScript by `deriveDetailBlockRollup` → `sumCounts`, which filters nulls and returns null only when _all_ are null — the skip-missing column of step 2, while the policy wanted the strict column. Adopting the plan as written would have landed the helper at the aggregate while the list kept its reduce, and its "two call sites" acceptance grep was unreachable on that route. Verified in the worktree 2026-08-14: the shipped resolution took step 4's third route — a pure `sumVacancyAreaStrict` single-owns the rule (empty → null, any null → null), the sort SQL is a declared mirror (`CASE WHEN bool_or(v IS NULL) THEN NULL ELSE SUM(v) END`), all three consumers (detail, list, sort) are asserted to agree by `building-vacancy-path-parity.test.ts` (15 tests passing), and `sumCounts` was deliberately left in place for the parking-count axis whose missing semantics differ. The plan's line numbers had already drifted by the time of this check (`sumCounts` at `:1658`, not `:1644`), which is the last edge case above diff --git a/wiki/backend/index.md b/wiki/backend/index.md index 5689f57..0d76452 100644 --- a/wiki/backend/index.md +++ b/wiki/backend/index.md @@ -31,6 +31,7 @@ Match your situation to a "load when" line; load only matching pages. | Page | Load when | |------|-----------| +| [aggregation-layer-of-a-shared-helper](common/change-impact/aggregation-layer-of-a-shared-helper.md) | A plan, brief, or review comment says to unify or replace "the N call sites" of a helper and names them by line number rather than by what the code does; one site feeds a set-level SQL aggregate while another returns a per-row value that application code reduces later; a "shared helper" landed with one consumer adopting it and the other keeping its old semantics; deciding which layer owns a missing-value rule and whether the plan's grep-count acceptance criterion is reachable at all | | [call-site-enumeration](common/change-impact/call-site-enumeration.md) | Changing the contract of a function/method/constructor other code calls — adding, removing, reordering or redefining a parameter — and you need the complete call-site list; scoping such a migration from a search; a migration scoped from recon came back green and then failed on call sites the search never listed; deciding whether to append a parameter or make it keyword-only (release-level re-test scope → qa/process/regression-scope) | ### reliability diff --git a/wiki/qa/document-verification/retiring-a-provisional-marker.md b/wiki/qa/document-verification/retiring-a-provisional-marker.md new file mode 100644 index 0000000..05a8388 --- /dev/null +++ b/wiki/qa/document-verification/retiring-a-provisional-marker.md @@ -0,0 +1,92 @@ +--- +id: qa-document-verification-retiring-a-provisional-marker +domain: qa +category: document-verification +applies_to: [general] +confidence: verified +sources: + - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/grep.html + - https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions + - https://www.rfc-editor.org/rfc/rfc7322.html +last_verified: 2026-08-14 +related: [qa-document-verification-editing-a-gated-document, qa-document-verification-spec-document-gates, qa-process-acceptance-criteria] +--- + +# Converting a Provisional Marker to a Settled Statement in a Reviewed Design Document + +## When this applies + +You are turning provisional markers — `[추정]`, TBD, "assumed", DRAFT — into +settled statements in an ADR, RFC, or spec that has been through review rounds, so +the document also carries a review checklist and a round history that refer to +those markers. Also when a coordinator's marker count and yours disagree while you +are both reading the same file. + +Editing a document that automated grep/lint gates run over → +[qa-document-verification-editing-a-gated-document]; that page owns the machine +anchors, this one owns the document's references to itself. + +## Do this + +1. **Split the marker's hits into axes before editing, and treat each axis as a + separate edit target.** The same grep returns all four mixed together, and only + the first is what "resolve the open items" means: + +| Hit class | What it is | What the conversion does to it | +|-----------|------------|--------------------------------| +| Body statement carrying the marker | An open item | Remove the marker and state the settled rule | +| Review-checklist row passed _because_ the marker exists (`- [x] … marked as [추정]`) | An assertion about the document's own text | Becomes false the moment the body marker goes — rewrite the row's condition in the same edit | +| Round-history entry recording that the item was provisional | A dated record | Keep the wording and append a resolution note | +| Prose that names the marker convention (a legend, a template quote) | Vocabulary, not an open item | Leave it | + +2. **Report the count with the axis and the command that produced it.** POSIX + `grep -c` writes "only a count of selected lines", so a line carrying two + markers counts once and a bare mention of the word counts the same as a marked + claim. Three different numbers over one file are all correct on their own axis; + an unlabelled number is what makes a coordinator's baseline and yours disagree. + +3. **Rewrite each checklist row that the marker's presence satisfied, in the same + commit as the body edit.** The row asserted "the document flags this as + provisional"; after the conversion the settled statement is what must be + asserted. Leaving the `[x]` in place points the next round at a marking that no + longer exists, and the round cannot tell which of the two texts is current + ([qa-process-acceptance-criteria]). + +4. **Annotate history entries rather than rewording them.** Append "→ resolved as + settled in round N" and leave the original sentence intact. This is the ADR + convention for the same reason: "If a later ADR changes or reverses a decision, + it may be marked as 'deprecated' or 'superseded' with a reference to its + replacement" — the record of what was open, and when, is what stops the same + question from reopening. + +5. **Re-run the axis counts after the edit and require the split, not a single + zero**: the body axis at 0, the history axis unchanged or higher, the checklist + axis with every row's condition matching text that exists. A single global zero + means the history was erased along with the open items. + +## Edge cases + +| Case | Then | +|------|------| +| The checklist row lives in a different file (a task brief, a review log) | Grep the marker across the review artifacts too and fix the row there; the row is stale wherever it lives, and a cross-file row makes your edit look like someone else's regression | +| The settled rule contradicts what the provisional statement guessed | Record both: the settled rule in the body, and the superseded guess in the history entry with the evidence that decided it — a silent replacement reads as if the guess had been right | +| Only some of the marked items are settled this round | Convert those and leave the rest marked; report the body-axis count as the remaining number rather than zero, so partial progress is not read as completion | +| The marker also appears inside a fenced block quoting the checklist's own pattern | Scope the count to the normative region rather than the whole file ([qa-document-verification-editing-a-gated-document]) | +| A machine gate asserts the marker's presence | Update the gate and the document in the same commit and say so in the PR — a gate left asserting a retired marker fails forever on a correct document | +| The document has no round history section | Add the resolution note next to the settled statement instead, naming the round and the evidence; what step 4 requires is the note, not a section to hold it | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Strip every marker hit the grep returned | Split the hits by axis and edit only the body and checklist axes | The history entries and the convention legend are correct uses; stripping them erases when the item was open and what the marker means | +| Report "17 markers remaining" from one grep | Report the number with its axis and command ("11 unresolved body items, `grep -n '\[추정\]'` over §3–§9") | `grep -c` counts selected lines, so word mentions, multi-marker lines, and marked claims give different totals — and the coordinator's baseline is one of the other axes | +| Leave the `- [x]` checklist row alone because the body is now more accurate | Rewrite the row's condition to assert the settled statement | The row's evidence was the marker; with the marker gone the row asserts text that does not exist, and the next round cannot tell which is normative | +| Reword the round-history entry to match the settled rule | Append a resolution note and keep the original sentence | The history is the answer to "why was this open?"; overwriting it invites the same question to be reopened from scratch | + +## Sources + +- https://pubs.opengroup.org/onlinepubs/9799919799/utilities/grep.html — `-c`: "Write only a count of selected lines to standard output"; "By default, each selected input line shall be written to the standard output" — the count is per line, not per occurrence +- https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions — Nygard: "A decision may be 'proposed' if the project stakeholders haven't agreed with it yet, or 'accepted' once it is agreed"; "If a later ADR changes or reverses a decision, it may be marked as 'deprecated' or 'superseded' with a reference to its replacement"; and explicitly, "If a decision is reversed, we will keep the old one around, but mark it as superseded" — the record is retained rather than rewritten, which is the convention step 4 applies to a marker inside one document +- https://www.rfc-editor.org/rfc/rfc7322.html — the RFC Style Guide recommends _distinguishable_ placeholder tokens for exactly this reason: "It is helpful for authors to clearly identify where text should be updated to reflect the newly assigned values. For example, the use of 'TBD1', 'TBD2', etc., is recommended in the IANA Considerations section and in the body of the memo." Numbered tokens make the body axis in step 1 enumerable; an undifferentiated marker word does not +- Field measurement 2026-08-14 (rtb-unified, NEWRTB-2435 ADR, D6 sub-total rule): converting two `[추정]` body lines to settled statements left two checklist rows (L858, L860) that had been passed _because_ those lines carried the marker, both instantly stale. On the same file `grep -c "추정"` returned 19, `grep -n "\[추정\]"` returned 17 lines, and the actual unresolved body items numbered 11 — the coordinator reported 11 while the worker reported 17 and 19, and the two only reconciled after the axis was named diff --git a/wiki/qa/index.md b/wiki/qa/index.md index be96aca..7ea8f79 100644 --- a/wiki/qa/index.md +++ b/wiki/qa/index.md @@ -30,6 +30,7 @@ Match your situation to a "load when" line; load only matching pages. | Page | Load when | |------|-----------| | [spec-document-gates](document-verification/spec-document-gates.md) | Deciding whether passing a doc gate is enough to accept the deliverable: a document passed its checklist but the requirement is still unmet; choosing what the gate must assert beyond keyword presence (table structure, MUST-vs-SHOULD demotion, closed-set completeness, cross-section consistency); setting the release policy a gate verdict feeds (authoring or validating the check code itself → wiki/testing/quality/spec-artifact-checks.md, wiki/testing/quality/checks-that-cannot-pass.md) | +| [retiring-a-provisional-marker](document-verification/retiring-a-provisional-marker.md) | Turning provisional markers (`[추정]`, TBD, "assumed", DRAFT) into settled statements in an ADR/RFC/spec that has been through review rounds, so the document also carries a review checklist and a round history referring to those markers; a checklist row stayed `[x]` on evidence you just deleted; a coordinator's marker count and yours disagree while you read the same file | | [editing-a-gated-document](document-verification/editing-a-gated-document.md) | Editing or rewording a document that grep/regex gates or a lint config check; a gate fails on wording whose meaning did not change; describing what an upstream spec says without tripping a "do not redefine it" gate; a check matches the pattern your own document quotes; recording an audit verdict inside the document that was audited; deciding which checks to re-run after editing a gated document | ## environments diff --git a/wiki/testing/index.md b/wiki/testing/index.md index 6b26016..6489a03 100644 --- a/wiki/testing/index.md +++ b/wiki/testing/index.md @@ -33,6 +33,7 @@ Match your situation to a "load when" line; load only matching pages. | [checks-that-cannot-pass](quality/checks-that-cannot-pass.md) | Authoring a check whose target does not exist yet (grep/regex gate on an unwritten file or doc section, lint/scan rule, schema assertion on an unbuilt endpoint, a plan's verification command) and it has only ever been observed failing; reviewing a plan's gates before adopting them; separating "target missing" from "content missing" in a gate's exit status. This owns unwritten-target gate patterns for spec/RFC documents too — qa/document-verification does not | | [spec-artifact-checks](quality/spec-artifact-checks.md) | Authoring or reviewing the check itself: that a mapping table covers every rule/field/enum case, that ids resolve across documents; deciding whether a green check earned "verified" or only "present"; designing one negative control per check in a multi-check harness; parsing Markdown table rows programmatically in a doc-as-spec repo (deciding whether a passing gate is enough to *accept the deliverable* → wiki/qa/document-verification/spec-document-gates.md) | | [schema-additions-under-a-golden-gate](quality/schema-additions-under-a-golden-gate.md) | Adding a node kind, variant, discriminator value, or field to a document format (IR, JSON Schema, spec artifact) whose only automated gate builds its negatives by mutating one committed golden example; the gate or the whole suite comes back green right after a schema change; deciding which negative each new schema keyword needs, and whether a green suite that never loads the schema is evidence at all | +| [generated-sql-property-assertions](quality/generated-sql-property-assertions.md) | Asserting on the SQL a query builder renders (`.toSQL()`, `sqlToQuery()`, a compiled-string snapshot) because CI has no database, and the property is semantic rather than textual ("a missing value must stay missing", "the tenant filter must stay applied"); such an assertion is green while a hand-seeded mutant that refills the missing value survives; choosing between top-level structural anchors, a function-name check, an aggregate-occurrence count, and a captured-alias binding — and knowing which refill position each one owns | | [harness-reverse-controls](quality/harness-reverse-controls.md) | You built a harness that scores how well something is verified (mutation run, doc/spec gate suite, CI check matrix) and are about to cite its score in a commit, PR, README, or report; its verdicts come out uniform (every case caught, or every case green); deciding what control run proves the harness discriminates, how to score errored/never-ran cases, and what the harness's isolated working tree must contain | ## data diff --git a/wiki/testing/quality/generated-sql-property-assertions.md b/wiki/testing/quality/generated-sql-property-assertions.md new file mode 100644 index 0000000..68fd253 --- /dev/null +++ b/wiki/testing/quality/generated-sql-property-assertions.md @@ -0,0 +1,109 @@ +--- +id: testing-quality-generated-sql-property-assertions +domain: testing +category: quality +applies_to: [postgresql, mysql, general] +confidence: verified +sources: + - https://www.postgresql.org/docs/current/functions-conditional.html + - https://www.postgresql.org/docs/current/functions-aggregate.html + - https://dev.mysql.com/doc/refman/8.4/en/comparison-operators.html + - https://stryker-mutator.io/docs/mutation-testing-elements/supported-mutators/ +last_verified: 2026-08-14 +related: [testing-quality-tests-that-cannot-fail, testing-quality-harness-reverse-controls, databases-schema-design-nullability-and-defaults, backend-common-change-impact-aggregation-layer-of-a-shared-helper] +--- + +# Locking a Missing-Value Property in Generated SQL the Suite Never Executes + +## When this applies + +A test asserts on the SQL a query builder renders (`.toSQL()`, `sqlToQuery()`, a +compiled-string snapshot) because CI has no database to run it against, and the +property you need is semantic rather than textual — "a missing value must stay +missing", "the tenant filter must stay applied". Also when such an assertion is +green and a hand-seeded mutant that refills the missing value survives it. + +Asserting that a _call site_ still exists in hand-written source, rather than a +property of a generated string, is a different subject — that guard binds an +anchor to a site, this one binds a property to a whole expression. + +## Do this + +1. **Partition the refill by _position_ before choosing any pattern.** A refill + that defeats the property can be written in one of two places, and the two are + caught by disjoint assertion families. No single family covers both, so the + question "is this property locked?" has no answer until both positions are + named: + +| Position | Example | What can catch it | +|----------|---------|-------------------| +| Outside the expression | `COALESCE(, 0)` | Top-level start/end anchors (any wrapper, unnamed included) | +| Inside the expression | `ELSE COALESCE(SUM(v),0)`, `ELSE GREATEST(SUM(v),0)` | A function-name check — the anchors cannot see it | +| Inside, no name at all | `ELSE (CASE WHEN SUM(v) IS NULL THEN 0 ELSE SUM(v) END)` | An occurrence count on the aggregate itself | +| Inside, guard result | `WHEN bool_or(v IS NULL) THEN 0` (guard yields zero) | A literal assertion on what the guard returns | +| Inside, decoy operand | guard and aggregate both moved to an unrelated column | Binding both to the alias captured from the anchor | + +2. **Anchor the whole rendered expression — opening token and closing token — for + the outside position.** Any wrapper then has to add text before the opening or + between the close and whatever follows (the alias, the sort direction, the + comma), so the anchors catch the class rather than enumerated names. + +3. **Assert the guard's _result_, not just its presence, for the inside + position.** `WHEN THEN NULL` as a literal is what separates "a guard + exists" from "the guard yields missing"; a guard rewritten to yield `0` leaves + every structural anchor intact. + +4. **Assert the aggregate's occurrence count.** A nameless `CASE` refill has to + evaluate the aggregate a second time, so "`SUM(` appears exactly once" is the + only family that reddens on it — and it doubles as the single-evaluation + regression guard for a correlated subquery. + +5. **Bind the guarded identifier and the aggregated identifier to an alias + captured from the anchor** rather than hardcoding the name. Capturing keeps a + behaviour-preserving rename green while a swapped or dead column still reddens. + +6. **Build the coverage matrix, one hand-seeded mutant per row of step 1, and + record which family killed each.** A mutant no family kills is a missing + family; a family that kills nothing is dead weight. Stryker's published + catalogue enumerates sixteen mutator groups that replace or remove nodes — + Block Statement "removes the content of every block statement" — and wrapping + an expression in a call is not among them, so nothing seeds this class for you. + +7. **Run behaviour-preserving controls and require green**: rename the alias, and + reflow the rendered whitespace. Without them, a pattern narrowed until every + mutant dies is indistinguishable from one narrowed to nothing + ([testing-quality-harness-reverse-controls]). + +## Edge cases + +| Case | Then | +|------|------| +| The property is "missing stays missing" but the column is `NOT NULL` in the schema | The aggregate still produces `NULL` over zero rows — `sum` of no rows returns null — so keep the assertion and note that the schema does not supply it | +| A bare aggregate is the refill (`SUM` alone, no wrapper) | `SUM` skips nulls, so a partial sum reads as a measured total; the property needs the guard, and the guard is what step 3 pins ([backend-common-change-impact-aggregation-layer-of-a-shared-helper]) | +| The same expression is rendered into both the select list and `ORDER BY` | Anchor each occurrence separately: `ORDER BY` renders without an alias, so the select-list anchor does not fit it | +| The builder emits bind placeholders (`$1`) whose numbering shifts with unrelated query changes | Anchor the structure around the placeholder and assert the bound parameter list separately, so an added filter elsewhere does not redden the property test | +| The builder normalizes or reflows whitespace between versions | Match a token sequence with bounded gaps rather than a literal multi-line string, and keep the step-7 reflow control | +| CI can run a real database | Assert the property by executing the query over an all-missing fixture and requiring `null` — an executed result outranks any string assertion, and the string test becomes a fast redundancy | +| The refill is legitimate for this field (zero and absent genuinely mean the same thing) | Move the decision to the schema or the API contract and delete the assertion, rather than keeping a guard the code is meant to violate ([databases-schema-design-nullability-and-defaults]) | +| A mutant survives every family | Read it as a missing position in step 1, not a pattern to tighten: identify where the mutant put its refill, and add the family that owns that position | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Assert the inner expression (`/SUM\(\s*v\s*\)/`) and call the property locked | Add the top-level anchors and the three inside-position families | A wrapper preserves every token the inner assertion reads, so `COALESCE(SUM(v),0)` passes an assertion written to forbid exactly that | +| Treat top-level anchors as primary and the name check as redundant | Keep both, and know which position each owns | Measured: an inner `COALESCE`/`GREATEST` refill leaves both anchors matching — only the name check reddens it, so dropping the name check reopens the hole | +| Rely on names once you have anchors | Add the aggregate-occurrence count as well | A nameless `CASE WHEN … IS NULL THEN 0 ELSE …` refill passes the anchors _and_ the denylist; the duplicated aggregate is its only signature | +| Hardcode the alias in the anchor to make the pattern precise | Capture the alias from the anchor and require guard and aggregate to reference it | A hardcoded alias reddens on a rename that changes no behaviour, and a red run on correct code is what gets an assertion loosened | +| Ship the assertions because all seeded mutants died | Run the rename and reflow controls and require green | An over-narrowed pattern kills every mutant and every correct variant alike; the controls separate the two | +| Port a name denylist from another project's engine | State the engine and check that engine's NULL rule for each listed function | `GREATEST(NULL, 0)` is `0` on PostgreSQL and `NULL` on MySQL, so the same list protects different properties on the two | + +## Sources + +- https://www.postgresql.org/docs/current/functions-conditional.html — GREATEST/LEAST: "NULL values in the argument list are ignored. The result will be NULL only if all the expressions evaluate to NULL. (This is a deviation from the SQL standard...)"; COALESCE "returns the first of its arguments that is not null. Null is returned only if all arguments are null" +- https://www.postgresql.org/docs/current/functions-aggregate.html — "except for `count`, these functions return a null value when no rows are selected. In particular, `sum` of no rows returns null, not zero as one might expect" — the property under test exists because of this +- https://dev.mysql.com/doc/refman/8.4/en/comparison-operators.html — "`GREATEST()` returns `NULL` if any argument is `NULL`"; for LEAST, "If any argument is `NULL`, the result is `NULL`. No comparison is needed" — the engine-dependence behind the last `Instead of` row +- https://stryker-mutator.io/docs/mutation-testing-elements/supported-mutators/ — sixteen mutator groups, all replacing or removing nodes ("This mutant operator removes the content of every block statement"); wrapping an expression in a call is absent from the catalogue, so step-6 mutants are hand-seeded +- Measured 2026-08-14 (PostgreSQL 16.14 and 17.11, both in Docker): over two all-NULL rows and over zero rows `sum(v)` → `NULL`; over `[5, NULL]` → `5` (the partial sum); `GREATEST(NULL,0)` → `0`, `GREATEST(NULL,NULL)` → `NULL`, `LEAST(NULL,0)` → `0`; `COALESCE(sum(v),0)`, `GREATEST(sum(v),0)` and `CASE WHEN sum(v) IS NULL THEN 0 ELSE sum(v) END` all → `0` +- Local reproduction 2026-08-14 (Node v24.8.0, six assertion families against the real rendered ORDER BY key of rtb-unified `buildingListOrderBy('vacancyArea')`): each seeded refill was killed by exactly one family — outer `COALESCE` by the start/end anchors, inner `COALESCE`/`GREATEST` by the name check only (both anchors still matched), a nameless inner `CASE` refill by the aggregate-occurrence count only (anchors and name check both passed), a guard rewritten to yield `0` by the literal `THEN NULL`, and a dead-column swap by the captured-alias binding. Both controls (alias rename, whitespace reflow) stayed green across all six +- Field measurement 2026-08-14 (rtb-unified, NEWRTB-2786): assertions bounded to the inner `CASE`/`SUM` left `ELSE COALESCE(SUM(x),0)` and a whole-subquery `COALESCE` alive across a green run; adding a name check still left a nameless single-evaluation `CASE` refill alive. The shipped test file carries the anchors, the `THEN NULL` literal, the captured-alias binding and a name check, and its correlated-subquery fingerprint count is keyed on the source column rather than the aggregate — which is why the nameless refill was the one form still standing when the matrix was built