Skip to content

knowledge: adapter required-field discovery (loud vs silent) + cooldown mark after a successful send (1 folded into #52) - #72

Open
dch0202-rsquare wants to merge 3 commits into
choiyounggi:mainfrom
dch0202-rsquare:knowledge/dch0202-rsquare-20260810-112909
Open

knowledge: adapter required-field discovery (loud vs silent) + cooldown mark after a successful send (1 folded into #52)#72
dch0202-rsquare wants to merge 3 commits into
choiyounggi:mainfrom
dch0202-rsquare:knowledge/dch0202-rsquare-20260810-112909

Conversation

@dch0202-rsquare

Copy link
Copy Markdown
Contributor

Knowledge flush — 3 insight(s)

2 ingested here, 1 folded into open PR #52 (no sibling duplicate opened).

Verified best-practice

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
<BlockDetailPanel occurrence count (true value 1) 2 1
/Number\.isFinite/ present (true value false) true false

So the queued insight's premise reproduces exactly. But the same run showed the
queued regex is itself defective: it truncated
const endpoint = "https://api.example.com//v2/items" to const endpoint = "https:,
because // inside a string literal is consumed as a comment start. A string-aware
alternation variant fixed the URL and still truncated a regex literal containing //
(const re = /a//b/const re = /a). The page therefore takes comment removal from
the language's own tokenizer/parser plus a control run, and records the regex form only
as an Instead of row with this measurement as the reason — shipping the queued
one-liner would have replaced a false-positive class with a silent-corruption class.

Confidence: verified (official ESLint docs quoted + reproducible Node measurement,
including the counter-measurement against the queued directive).

Existing-layer check

Routed via INDEX.md → domain index.md for backend, infrastructure and testing, then
read every page whose "load when" line overlapped.

Pages read: backend-common-change-impact-call-site-enumeration,
backend-common-integrations-externally-owned-defaults,
backend-common-api-design-unenforced-declarations,
infrastructure-observability-alerting,
infrastructure-observability-logs-metrics-signals,
testing-quality-tests-that-cannot-fail, testing-quality-harness-reverse-controls,
testing-quality-guard-shape-vs-consequence, testing-quality-spec-artifact-checks,
testing-quality-behavior-not-implementation, testing-quality-minimum-case-set

Findings:

New categories: none. Both pages land in existing categories
(backend/common/integrations, infrastructure/observability).

Open-PR check

Listed via gh pr list --repo choiyounggi/dev-loop --state open --search "head:knowledge/"
— 15 open heads: #69, #68, #66, #64, #62, #61, #58, #57, #56, #55, #52, #51, #50, #49, #47.
Diffed each head's wiki/ changes against origin/main; for the
two fork-hosted heads that git fetch could not resolve (#52, #49) I listed and read
the files through the GitHub API at each PR's head SHA instead.

Candidate Overlapping open head Verdict
I1 — consumer required fields #49 testing/quality/unasserted-return-fields.md (nearest); no open head touches backend/common/integrations/ new#49 governs returned fields the suite never asserts; I1 governs input fields the consumer silently defaults. Different artifact, different trigger, and I1's primary directive (execute the consumer to learn its required set) has no counterpart in #49
I2 — suppression mark after a successful send none — no open head touches infrastructure/observability/ new
I3 — comments contaminate source-text assertions #52 testing/quality/source-text-wiring-assertions.md fold — pushed to that branch; not ingested here

Fold detail for I3: #52's page already owns the trigger "a test reads a source file as
a string and asserts by regex". Its step 5 even prescribes the control that detects
this defect ("change a comment ... require green") but does not say what to do when
that control goes red, and its step 2 ("count the anchor's occurrences before using
it") is the step comments break — my fixture's JSDoc line took an anchor count from 1
to 2. The additions therefore belong on that page, not on a sibling. Because I own the
head repository (dch0202-rsquare/dev-loop), the commit went onto #52's branch and a
comment on #52 records what changed and why, so the owner reviews it in one pass.

Routing decision

Insight Target New category?
I1 backend / common/integrations / consumer-required-fields.md (id backend-common-integrations-consumer-required-fields) No. integrations is described in the domain index as consuming external-API responses and externally-owned defaults; an adapter feeding a module the repo does not own is the same boundary seen from the producing side. change-impact was rejected because its trigger is a callee you own and are changing; api-design was rejected because its pages are HTTP contract design
I2 infrastructure / observability / suppression-state-and-delivery-failure.md (id infrastructure-observability-suppression-state-and-delivery-failure) No. Sits beside alerting (which decides routing, not delivery bookkeeping). backend/common/jobs was rejected: the artifact is a notifier's suppression state, not a queue consumer
I3 testing / quality / source-text-wiring-assertions.md on open PR #52's branch No — fold, no new page

Plumbing updated: wiki/backend/index.md (+1 row), wiki/infrastructure/index.md
(+1 row), reciprocal related: on infrastructure/observability/alerting.md and
backend/common/integrations/externally-owned-defaults.md, log.md (+1 entry).
Checked before commit: both new pages are under the 120-line body limit (76 and 78),
carry no banned vague qualifiers, and every related: id and inline [page-id]
reference resolves to a file in wiki/.

Decision Log

Cross-Check: independent adversarial review (claude CLI headless, --permission-mode plan,
prompted to refute and to re-fetch every cited URL) returned 1 Critical + 11 Warnings +
4 Info; all were reproduced locally and fixed before this PR was opened — see below.

Intent. Drain the 3 pending ~/.dev-loop/queue candidates into reviewed wiki
content: 2 as new pages here, 1 folded into open PR #52 because that PR already owns
the trigger. No sibling duplicate PR was opened, and nothing is auto-merged.

What the cross-check changed (nothing here is the queued text as harvested).

# Finding Fix applied
Critical 1 The suppression page prescribed notify "$@" || return 1 before the marker. Reproduced: at a script's top level return errors, execution falls through to the marker write, and the script exits 0 — the page's own snippet reproduced the defect the page exists to prevent. The cited repo ships if slack "$@"; then …; fi (bin/gitops-deploy.sh:91) Prescribe the shipped shape; add a 3-row table covering top-level, in-function, and set -e propagation
Warning 3 The notify package docs state the stages' contracts, not their order Present the ordering as following from SetNotifiesStage's precondition + DedupStage's notification log, not as stated by the page
Warning 4 Current repeat_interval docs no longer carry the attempt-vs-delivery wording Dropped that clause; the distinction now rests only on the notify citation
Warning 6 ts.createSourceFile was named as the stripper. Verified against TypeScript 5: sourceFile.comments === undefined, no comment node in the AST — a silent no-op, the exact failure class the page is about Name ts.getLeadingCommentRanges, espree.parse(…, {comment:true}), @babel/parser parse(src).comments
Warning 8 "green by construction" was claimed for the whole step-6 control, but its reformat half still reddens a bounded pattern with no comment involved (reproduced) Scoped to the comment-only half; the reformat half tests N
Warning 9 The no-parser fallback filtered whole comment lines, which misses trailing comments — the leak that motivates the page (reproduced: count 2 vs 1) Cut each line to EOL
Warning 11 The two-run differ assertion passes vacuously on a non-deterministic consumer (LLM scorer, sampling) — a class the page explicitly invites Added the edge row: pin seed/temperature or stub before comparing
Warnings 2, 5, 10, 12; Info 13–15 prose/table count mismatch; a measurement labelled "parser-equivalent" when a regex was run; N's subject unstated; index "load when" clauses absent from "When this applies"; unverifiable line-count and version pins; one prohibition outside Instead of All applied

Where the reviewer was wrong, and how I checked. Info 14 claimed the loud/silent
split was not reproducible because a sibling copy reads it["desc"] by subscript. I
opened the actual engine: manday-sp/engine.py:399 is d = it.get("desc") or ""
(silent) and check_assignee_ids at 511 enforces key presence (loud). The measurement
stands; the reviewer's underlying point — that the split is a property of the read
site, so pin file:line — was right and is now in the page.

Alternatives rejected. (a) Ingesting I3 as its own page — rejected: #52 owns the
trigger, and a sibling page is the pile-up this skill exists to prevent (#39). (b)
Only commenting on #52 instead of pushing — rejected: the change is a new step plus
six table rows, which is a diff to review, not a note. (c) Shipping the queued
strip-regex directive verbatim — rejected: measured, it corrupts string and regex
literals, so it would have traded a false-positive class for a silent-corruption one.
(d) Adding the reciprocal related: id to call-site-enumeration — rejected: five
open PRs already edit that file and the backlink would be a pure conflict.

Where a reviewer should look. The suppression page's step-2 shell table (the
Critical); the folded page's step 2 API list (a wrong API there is undetectable at
runtime); and whether backend/common/integrations is the right home for I1 versus a
change-impact sibling — that routing call is the least certain thing in this PR.

Not done deliberately. No gh pr merge; both branches are pushed for your review.

…gs) to both new pages

- suppression: the prescribed shell shape was wrong. `notify "$@" || return 1` at
  a script's top level errors and falls through to the marker write, exiting 0 —
  reproducing the defect the page exists to prevent. Prescribe the shape the cited
  repo actually ships (`if notify "$@"; then <write mark>; fi`) and table the
  three shell shapes including set -e propagation.
- suppression: the notify package docs state the stages' contracts, not their
  order; present the ordering as following from SetNotifiesStage's precondition.
- suppression: drop the 'not to a prior attempt' clause the current
  repeat_interval docs no longer support.
- suppression/consumer: fold each index 'load when' promise into 'When this applies'.
- consumer: add the non-deterministic-consumer edge (the two-run differ assertion
  passes vacuously); pin the loud/silent read sites by file:line; inline the
  reproduction's formula; convert the one prohibition outside 'Instead of'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant