drain(#226 follow-up 2): CC log schema + populated-state alignment#434
drain(#226 follow-up 2): CC log schema + populated-state alignment#434
Conversation
There was a problem hiding this comment.
Pull request overview
Aligns the conflict-output portion of the memory reconciliation design doc with the actual docs/CONTRIBUTOR-CONFLICTS.md state and schema (populated CC-### table), correcting the previously-described “unpopulated + CONF--” format.
Changes:
- Updates the design to reference the existing populated CC-### schema (CC-001..CC-003) in
docs/CONTRIBUTOR-CONFLICTS.md. - Replaces the prior
CONF-<YYYY-MM-DD>-<NNN>bullet format with an example row matching the existing Markdown table columns. - Adds guidance to continue the CC counter from the current maximum and preserve existing manually-curated rows.
Codex post-merge findings on PR #226 (after the first follow-up #433 merged): P1 (line 244) — CONTRIBUTOR-CONFLICTS.md is already populated: The doc said the file is 'present-with-schema-but- unpopulated'. Wrong: CC-001..CC-003 are filled in (no-name-attribution-rule scope, Stabilize-vs-keep-opening frames, absent-artifact-citation discipline). Updated text to reflect the populated state. P1 (line 270) — Conflict-row format mismatch: Proposed format used '### CONF-<date>-<nnn>' + bullet fields. Actual file uses CC-### IDs in a markdown table with columns (Conflict ID | Date | Question | Parties | Positions | Resolution | Scope | Source). Replaced the proposed format with a template that matches the existing table schema. CC-### counter continues from the highest existing ID (next auto-detected = CC-004). Generator preserves manually-curated rows and only appends auto- detected ones.
P1 (L269) — clarify Open-table targeting: Replaced "appending machine-generated rows" (ambiguous given the file has separate Open / Resolved / Stale tables) with "inserting machine-generated rows into the Open table (or a delimited autogenerated subsection)". Without this, a literal append-to-EOF implementation wouldn't land in Open. P1 (L290) — specify idempotent generator strategy: Added explicit canonical-key → CC-NNN mapping. Generator updates in-place when key already matches; allocates new CC-NNN only when unmapped. Avoids unbounded growth / duplicate entries on repeated CI runs. Documents both strategies (in-place update + delimited autogenerated subsection). P2 (L275) — placeholder labels match column headers: Renamed schema placeholders from "Parties:" / "Resolution:" to "Between:" / "Resolution-so-far:" matching the actual headers in docs/CONTRIBUTOR-CONFLICTS.md (verified via grep). Reduces drift between the design doc and the live schema.
7d6a39b to
acac9d4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acac9d4d2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| explicit mapping from canonical key | ||
| (`<subject>::<predicate>::<normalized-object>` plus the | ||
| sorted set of contributing MF-IDs) → CC-NNN. On each run: |
There was a problem hiding this comment.
Match existing conflicts without MF-ID set in dedupe key
The idempotency key currently includes the sorted MF-ID set, so a normal fact supersession (same canonical disagreement, new MF-... id) will no longer match the prior CC row and will allocate a new CC-... on the next run. That breaks the stated in-place update behavior and can duplicate logically identical open conflicts while discarding accumulated Resolution-so-far context for the same subject/predicate dispute.
Useful? React with 👍 / 👎.
| Generator MUST preserve all existing manually-curated rows | ||
| verbatim — both in **Open** (treat human-edited rows as | ||
| read-only) and in **Resolved** / **Stale**. Auto-detected | ||
| rows update in-place when matched; new rows are inserted |
There was a problem hiding this comment.
Define a durable discriminator for auto-generated CC rows
This section requires the generator to keep human-curated rows read-only while updating auto-detected rows in place, but the row schema shown above does not define any explicit auto/manual marker. Without a required discriminator (e.g., a dedicated column or mandatory autogenerated block), implementations cannot reliably tell which rows are safe to mutate, which risks either overwriting manual conflict entries or skipping intended updates.
Useful? React with 👍 / 👎.
…ion CC schema) (#440) Otto-268 backfill: drain-log for PR #434 covering 3 substantive design- correctness findings on the memory-reconciliation algorithm spec. Per Otto-250 training-signal discipline. Pattern observations capture four load-bearing patterns: design-doc placeholders should match live-schema column names (verify-via-grep + rename); idempotency requirements need explicit canonical-key strategies; three-table schema awareness vs single-table append (Open / Resolved / Stale); per-PR reviewer-coverage signal (Copilot-only on design-spec text vs Codex-favored surfaces like shell + math + security).
…e) (#463) Otto-268 follow-on: drain-log for the 3-finding first cascade PR #433 (post-merge follow-up to #226 memory reconciliation algorithm design v0). Captures three orthogonal algorithm-correctness improvements. Per Otto-250 training-signal discipline. Pattern observations: 1. chain_head-liveness vs priority-tie-break-for-winner distinction: two invariants conflated at high level (Invariant 2 vs Invariant 6) distinguished. Spec-correctness findings benefit from per-invariant orthogonal-checks reasoning. 2. Paired-delimiter-vs-raw-character is a normalization-rule precision class. Same shape as #206's K-relations subset-vs-superset precision error. `_internal_var` should be preserved (single delimiters); `_text_` should be unwrapped (paired). 3. Index-rendering dedup is its own correctness class — multiple typed facts from same source_path → ONE index row. 4. Memory-reconciliation algorithm spec benefits from multiple cascade waves: #226 → #433 → #434 walked through schema + invariants + normalization + dedup + CC alignment + idempotent generator across three iterations. Each wave catches a different class of gap.
…reword) Otto-268 backfill: drain-log for PR #435 (drain follow-up to #148: why-the-factory-is-different live-lock cadence claim + grammar), covering 3 threads across 2 waves with a clean self-induced-cascade pattern. Per Otto-250 training-signal discipline. Pattern observations capture four load-bearing patterns: 1. Cross-reviewer convergence on Wave 1 (Codex P2 + Copilot P1 flagging the same missing-FACTORY-HYGIENE-row) raised quality signal — same shape as #432's `warn` unbound finding. 2. Self-induced cascade: my Wave-1 fix introduced the Wave-2 finding (claim "separate BACKLOG items" implied plural; actual BACKLOG state is one row with multiple sub-items). Pattern: when fixing a claim, verify the new claim is also accurate against current-state. 3. Reword-option-(a)-vs-(b) decision template generalizes: when doc asserts X but X doesn't exist, prefer reword-to-current-truth over add-the-thing-asserted (unless thing is small + isolated). 4. PR-mechanics: 4 of 7 cascade-PRs in this session (#135, #231, #432, #435) went through wave-1 + wave-2 cascade pattern; the reviewer-cascade is a consistent property of the merge-trigger surface, not a per-PR oddity. Closes the session-drain-log backfill (Otto-268) for the major PRs drained in this session: #135 / #235 / #432 / #434 / #195 / #219 / #206 / #377 / #231 / #85 / #435 (11 PRs total covered across drain logs #437-#447).
Four Copilot catches converging on the same root cause: the shape-divergence section was citing drain-log PR-numbers (#437-#465) when readers will look for #437-drain-log.md etc. and not find them — drain-log FILE numbers reference the PRESERVED PR (e.g., #421/#422/#423), not the drain-log PR itself. The current file corpus ends at #434. Fixes: - :374 + :394 — drop the misleading "#437-#465" PR-range citation; cite the actual in-repo abbreviated-shape examples (#421/#422/#423) by file path; drop the unsupported "22+" numeric estimate in favour of "the 2026-04-25 backfill wave". - :380 — abbreviated template snippet now matches what the in-repo abbreviated logs actually use: `Finding:` bullet included, `Thread ID:` and `:LINE` placeholders dropped (those are canonical-shape fields, not abbreviated-shape). Reader can now copy/paste the template accurately. - :388 — soften the "Substance is preserved" overstatement to the objective claim: key metadata (reviewer/severity/outcome/ commit) is preserved; verbatim original-comment + verbatim reply text are NOT preserved; multi-section structure + file:line locator + Thread ID are also typically omitted. The fixes are themselves Class A (count-vs-list cardinality on "#437-#465" range) + truth-drift on a freshly-landed doc — appropriate self-application of the discipline this section documents.
…reword) (#447) * hygiene(#268): pr-preservation drain-log for #435 (live-lock cadence reword) Otto-268 backfill: drain-log for PR #435 (drain follow-up to #148: why-the-factory-is-different live-lock cadence claim + grammar), covering 3 threads across 2 waves with a clean self-induced-cascade pattern. Per Otto-250 training-signal discipline. Pattern observations capture four load-bearing patterns: 1. Cross-reviewer convergence on Wave 1 (Codex P2 + Copilot P1 flagging the same missing-FACTORY-HYGIENE-row) raised quality signal — same shape as #432's `warn` unbound finding. 2. Self-induced cascade: my Wave-1 fix introduced the Wave-2 finding (claim "separate BACKLOG items" implied plural; actual BACKLOG state is one row with multiple sub-items). Pattern: when fixing a claim, verify the new claim is also accurate against current-state. 3. Reword-option-(a)-vs-(b) decision template generalizes: when doc asserts X but X doesn't exist, prefer reword-to-current-truth over add-the-thing-asserted (unless thing is small + isolated). 4. PR-mechanics: 4 of 7 cascade-PRs in this session (#135, #231, #432, #435) went through wave-1 + wave-2 cascade pattern; the reviewer-cascade is a consistent property of the merge-trigger surface, not a per-PR oddity. Closes the session-drain-log backfill (Otto-268) for the major PRs drained in this session: #135 / #235 / #432 / #434 / #195 / #219 / #206 / #377 / #231 / #85 / #435 (11 PRs total covered across drain logs #437-#447). * drain(#447 follow-up): fix #435 drain-log Reviewer field + stable-identifier xref Codex P2 + Copilot threads on #447 caught: - Thread 1.2 missing the `Reviewer:` field even though the drain-log schema (intro paragraph) declares per-thread reviewer authorship. Added `Reviewer: copilot-pull-request-reviewer`. - Stale `docs/BACKLOG.md lines 1313-1328` citation: those lines now contain the Server Meshing section; the live-lock-smell cadence row drifted to ~L1452 in the P1 tooling section. Replaced with the stable identifier (heading text 'Live-lock smell cadence (round 44 auto-loop-46 absorb, landed as `tools/audit/ live-lock-audit.sh` + hygiene-history log)') so future readers don't chase a moving line-number target. Same stable-identifier-vs-line-number-xref pattern flagged on #423's `near line 4167` finding. Documented in `_patterns.md` — line numbers decay on every adjacent edit; stable identifiers decay only on rename. Adopting heading text as the stable cite. The bare `:111`/`:113` thread location format (Otto-250 file:line shape conformance) is the broader Otto-268-wave divergence documented in PR #467 known-divergence section — deferred to maintainer review per that framing.
…iated shape divergence (#467) * hygiene(_patterns.md): document Otto-250-canonical vs Otto-268-abbreviated drain-log shape divergence Multiple post-merge reviewer threads on the Otto-268-wave drain-logs (#437-#465) flagged that the abbreviated inline shape doesn't match the canonical Otto-250 multi-section format used in older drain-logs (#108, #247, etc.). The findings are correct: the shapes ARE different, and the abbreviated shape compresses verbatim reviewer/reply text into short prose. Documenting both shapes here in `_patterns.md` as a known divergence + three maintainer-decision options: (a) Rewrite — high churn, highest faithfulness. (b) Accept divergence — document both as valid. (c) Hybrid — canonical for substantive logs (math / crypto / algorithms / ADRs); abbreviated for low-substance. Until maintainer decides, drain-runners writing future logs default to canonical; existing Otto-268-wave logs stay abbreviated with this known-divergence pointer. This is the right place for the documentation per the synthesis- over-history surface-class declaration: `_patterns.md` reflects current corpus state including known shape divergences, separate from the per-log records. Composes with the per-PR drain-log threads currently flagging the shape-conformance gap on #437/#441/#442/#444/#445/#446/#447/#449/ #460/#461/#464/#466 — those threads can now reply with a one-line pointer to this section instead of repeating the maintainer- decision-pending framing. * hygiene(#467): fix 4 Copilot findings on shape-divergence section Four Copilot catches converging on the same root cause: the shape-divergence section was citing drain-log PR-numbers (#437-#465) when readers will look for #437-drain-log.md etc. and not find them — drain-log FILE numbers reference the PRESERVED PR (e.g., #421/#422/#423), not the drain-log PR itself. The current file corpus ends at #434. Fixes: - :374 + :394 — drop the misleading "#437-#465" PR-range citation; cite the actual in-repo abbreviated-shape examples (#421/#422/#423) by file path; drop the unsupported "22+" numeric estimate in favour of "the 2026-04-25 backfill wave". - :380 — abbreviated template snippet now matches what the in-repo abbreviated logs actually use: `Finding:` bullet included, `Thread ID:` and `:LINE` placeholders dropped (those are canonical-shape fields, not abbreviated-shape). Reader can now copy/paste the template accurately. - :388 — soften the "Substance is preserved" overstatement to the objective claim: key metadata (reviewer/severity/outcome/ commit) is preserved; verbatim original-comment + verbatim reply text are NOT preserved; multi-section structure + file:line locator + Thread ID are also typically omitted. The fixes are themselves Class A (count-vs-list cardinality on "#437-#465" range) + truth-drift on a freshly-landed doc — appropriate self-application of the discipline this section documents.
Codex post-merge review on #226 surfaced 2 findings about CONTRIBUTOR-CONFLICTS.md alignment:
### CONF-<date>-<nnn>+ bullets but actual file uses CC-### IDs in a markdown table.Both fixed; CC-### counter continues from highest existing ID.