diff --git a/.dev-loop/INGEST_REPORT.md b/.dev-loop/INGEST_REPORT.md index 55ccfd1..dede5d6 100644 --- a/.dev-loop/INGEST_REPORT.md +++ b/.dev-loop/INGEST_REPORT.md @@ -1,53 +1,254 @@ -# 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. +2 ingested here, 1 folded into open PR #52 (no sibling duplicate opened). ## 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. +### I1 — an adapter's required-field set comes from running the consumer, not from its docstring + +**Claim.** When mapping one module's records into a second module's payload, call the +real consumer once with a mapped record before writing the rest of the adapter, then +split the fields it reads into *loud* (presence check / direct subscript → raises on +the first record) and *silent* (read with a default → no error, wrong value), and give +every silent field its own two-run assertion. + +**Sources checked (opened this session).** + +- https://docs.pact.io/ — "The contract is generated during the execution of the + automated consumer tests"; contract tests "check that all the calls to your test + doubles return the same results as a call to the real application would"; and + "unlike a schema or specification (eg. OAS), which is a static artefact that + describes all possible states of a resource, a Pact contract is enforced by + executing a collection of test cases, each of which describes a single concrete + request/response pair." This is the source for preferring an execution over the + documented shape. +- https://json-schema.org/understanding-json-schema/reference/object — "By default, + the properties defined by the `properties` keyword are not required." An example + payload therefore carries no required/optional information at all. + +**Verification.** Reproduced the loud/silent asymmetry locally (Python 3, 12-line +script): one consumer read `assignee_id` via `if "assignee_id" not in item: raise +ValueError` and `desc` via `item.get("desc", "")`. Dropping `assignee_id` raised on +the first record; dropping `desc` raised nothing and moved the returned score from +21.0 to 1.0. Field evidence from the harvest: the same mapping produced 100% +`ValueError` for the missing `assignee_id` and a silent 5.3x under-estimate +(1.63 → 0.31) for the missing `desc`. + +**Not verified, and excluded from the page.** I attempted to cite the Python docs for +`dict.get` never raising `KeyError`; both fetches of +`docs.python.org/3/library/stdtypes.html` (with and without the `#dict.get` anchor) +returned content truncated before the Mapping Types section, so no quote was +available. The local reproduction stands in for it and no Python-docs URL is cited. + +**Confidence: verified** (two official docs quoted + local reproduction). + +### I2 — the cooldown mark belongs after a send that reported success + +**Claim.** Write a notification-suppression mark only on a send whose status was +success; give the send its own exit status; make the send path injectable; assert the +succeeding-sender and failing-sender worlds as two separate tests. + +**Sources checked (opened this session).** + +- https://pkg.go.dev/github.com/prometheus/alertmanager/notify — the pipeline ordering + is stated in the stage doc comments: `RetryStage` "notifies via passed integration + with exponential backoff until it succeeds. It aborts if the context is canceled or + timed out."; `SetNotifiesStage` "sets the notification information about passed + alerts. **The passed alerts should have already been sent to the receivers.**"; + `DedupStage` "filters alerts. Filtering happens based on a notification log." So the + log that suppression reads is written only after delivery — a production system + stating exactly this ordering. +- https://runbooks.prometheus-operator.dev/runbooks/general/watchdog/ — the Watchdog is + "an alert meant to ensure that the entire alerting pipeline is functional", "always + firing", and "if not firing then it should alert external systems that this alerting + system is no longer working." Supports the external-heartbeat step, not the ordering. +- https://prometheus.io/docs/alerting/latest/configuration/ — `repeat_interval` is + keyed to a prior *notification*, not to a prior attempt. + +**Not verified, and excluded from the page.** I tried to source the "the alerting +pipeline must not fail together with what it monitors" argument from +https://sre.google/sre-book/monitoring-distributed-systems/. The chapter does not say +it: it argues for monitoring being "kept simple and comprehensible" and for "distinct +systems with clear, simple, loosely coupled points of integration" between monitoring +and *other inspection tools*, which is a different claim. The correlated-failure point +is therefore stated in the page only as an Edge-cases row whose remedy is the Watchdog +heartbeat (which *is* sourced), and the SRE book is not cited for it. + +**Verification.** Field measurement from the harvest, re-read against the code: +`rtb-mac-server-k8s bin/gitops-deploy.sh` wrote the `alert-main-fetch` marker after a +send whose webhook lookup had failed, so the next invocation suppressed the alert as +"in cooldown". Applying `notify "$@" || return 1` before the marker, in a copy outside +the repo, turned three existing tests red — the always-failing stub had fixed the +pre-send ordering as the expected contract. + +**Confidence: verified** (Alertmanager stage contracts quoted from the package docs + +reproduced field measurement). + +### I3 — source-text assertions must be made against code with comments removed (folded, see below) + +**Claim as queued.** Strip comments from the source before asserting on it, using +`src.replace(/\/\*[\s\S]*?\*\//g,'').replace(/\/\/.*$/gm,'')`. + +**Sources checked (opened this session).** + +- https://eslint.org/docs/latest/extend/custom-rules — "While comments are not + technically part of the AST, ESLint provides the `sourceCode.getAllComments()`..." + and rules visit "nodes while traversing the abstract syntax tree (AST as defined by + ESTree)". This is the mechanism: a structural check runs over a tree comments do not + appear in, a text check runs over the file where they do. +- https://docs.semgrep.dev/writing-rules/pattern-syntax — "Semgrep automatically + searches for code that is semantically equivalent" (constant propagation, AC + matching). Supports "match the structure, not the characters"; it does **not** state + anything explicit about comments, and the page does not claim it does. + +**Verification — and a correction to the queued directive.** Measured 2026-08-10 in +Node against a fixture containing a JSDoc block, a line comment, and a URL string: + +| Assertion | Raw source | After the queued strip regex | +|---|---|---| +| `; fi`; in a service, a send that returns an error instead of logging and + continuing. A notifier that always succeeds gives the marker nothing to + condition on. + +| Shell shape | Behaviour | +|---|---| +| `if notify "$@"; then ; fi` | The marker is unreachable on failure, at top level and inside a function alike, and the caller's status is unchanged | +| `notify "$@" \|\| return 1` inside a function whose caller checks the status | Equivalent, and it also stops the rest of that function — under `set -e` the nonzero status propagates and aborts the caller, so use it only where aborting is the intent | +| `notify "$@" \|\| return 1` at the top level of a script | `return` outside a function is an error; execution falls through to the marker line and the script still exits 0 — the defect this page is about, hidden behind a success code | + +3. **Make the send path injectable** — a command name, a function reference, or + an interface the test substitutes — so the suppression logic can be exercised + against both a succeeding and a failing sender. + +4. **Assert both worlds, as the three tests below:** + +| Test world | Assert | +|---|---| +| Send succeeds | The mark exists, and a second tick within the window sends nothing | +| Send fails | No mark exists, and the next tick attempts the send again | +| Send fails, then succeeds | Exactly one delivery total, and the mark dates from the successful attempt | + +5. **Keep the suppression window shorter than the time you are willing to be + blind**, and pair it with a heartbeat that proves the delivery path still + works — the Watchdog pattern is "an alert meant to ensure that the entire + alerting pipeline is functional", always firing, so that "if not firing then + it should alert external systems that this alerting system is no longer + working" ([infrastructure-observability-alerting]). + +6. **When the stub in an existing suite always fails, treat any test that + asserts the mark exists as a specification of the defect** and rewrite it + before changing the code — otherwise the correct fix arrives as a red suite + and reads as a regression ([testing-quality-tests-that-cannot-fail]). + +## Edge cases + +| Case | Then | +|------|------| +| The transport is fire-and-forget (UDP, a webhook whose 202 means "queued") | Mark on the strongest acknowledgement the transport gives, and state in the code comment what that acknowledgement does and does not prove | +| A retry inside the send already covers transient failure | Keep the mark after the retry loop's overall result, not after the first attempt | +| The failure is a permanent 4xx (bad webhook URL, revoked token) | Retrying every tick emits an unbounded error stream — mark it, and route the send-failure itself as its own condition so the broken channel is visible | +| The condition and the notifier fail together (one network partition, one dead cluster) | Delivery cannot be repaired from inside the failing system; the heartbeat in step 5 is what makes the silence visible from outside | +| The mark is a file whose write can fail | A failed mark write repeats the notification; a failed send that marks suppresses it — prefer the repeat, and log the mark-write failure | +| Several processes share one mark | The mark is shared state; give it an atomic write (rename-into-place) so a partial write is not read as a valid recent send | + +## Instead of + +| If you are about to | Do this instead | Why | +|---------------------|-----------------|-----| +| Write the cooldown mark before calling the notifier | Call the notifier, check its status, write the mark on success | A delivery failure then buys silence for the whole window, at the moment the condition is live | +| Let the notifier swallow its own error and always return 0 | Return the send's status and put the mark inside `if notify "$@"; then … fi` | With no status there is no way to distinguish "sent" from "attempted" | +| Accept a suite that only ever runs the always-failing stub | Add the succeeding-sender world as a second harness fixture | A single-world harness reports the same verdict for correct and defective ordering ([testing-quality-harness-reverse-controls]) | +| Widen the cooldown window because the channel is noisy | Group or route at the alert level and keep the window short | A long window and a lost send compound: the first failure hides the condition for the full window | + +## Sources + +- https://pkg.go.dev/github.com/prometheus/alertmanager/notify — stage contracts: `RetryStage` "notifies via passed integration with exponential backoff until it succeeds. It aborts if the context is canceled or timed out."; `SetNotifiesStage` "sets the notification information about passed alerts. The passed alerts should have already been sent to the receivers."; `DedupStage` "filters alerts. Filtering happens based on a notification log." — dedup reads the log that is written only after delivery +- https://runbooks.prometheus-operator.dev/runbooks/general/watchdog/ — the Watchdog is "an alert meant to ensure that the entire alerting pipeline is functional", "always firing", and "if not firing then it should alert external systems that this alerting system is no longer working" — the external heartbeat of step 5 +- https://prometheus.io/docs/alerting/latest/configuration/ — `repeat_interval` is "How long to wait before repeating the last notification"; the suppression clock is described in terms of a notification, and the page states nothing about delivery attempts (so the attempt-vs-delivery distinction rests on the notify-package citation above, not on this one) +- Field measurement 2026-08-07 (rtb-mac-server-k8s, `bin/gitops-deploy.sh`): the `alert-main-fetch` marker was written after a send whose webhook lookup had failed, so the following invocation suppressed the alert as "in cooldown". Moving the marker inside `if slack "$@"; then printf '%s' "$now" > "$f"; fi`, in a copy outside the repo, turned three existing tests red — the suite's always-failing stub had fixed the pre-send ordering as the expected contract