Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Introduces a research-grade “DST accepted boundaries” registry to document deliberate non-simulated entropy boundaries, and annotates push-with-retry.sh with a pointer to its registry entry.
Changes:
- Add
docs/research/dst-accepted-boundaries.mddefining a registry schema and the first boundary entry fortools/git/push-with-retry.sh. - Update
tools/git/push-with-retry.shheader comments to include a DST boundary classification pointer to the registry.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/git/push-with-retry.sh | Comment-only header update pointing to the new accepted-boundary registry entry. |
| docs/research/dst-accepted-boundaries.md | New registry document with schema, first entry, and cross-references/promotion notes. |
…sification (Amara 19th #3) Amara 19th-ferry correction #3 asked for push-with-retry.sh to either (a) document as external-boundary exception with rationale, or (b) convert to investigation-wrapper. Audit finding: the script already implements (a) with high discipline — root-cause investigation block in the header, targeted-5xx-only retries (not blind), exponential backoff, max-attempts cap, per-attempt logging to stderr, full error-text preservation via tee, distinct exit codes for transient-retry-exhausted vs non-transient vs env-error. Amara's concern was based on the doc-level visibility gap, not an implementation gap. This PR closes the gap by: 1. Creating docs/research/dst-accepted-boundaries.md — the accepted-boundaries registry Amara's correction #2 + the DST-compliance-criteria doc (PR #346) both require. Schema for entries locked. First entry: push-with-retry.sh with full rationale, investigation summary, retry- discipline breakdown, and revisit triggers. 2. Adding a classification pointer in the script's header comment block: cites the registry + "First classified 2026-04-23, formally registered Otto-168 2026-04-24." Registry research-grade today; promotes to docs/DST-ACCEPTED- BOUNDARIES.md top-level when PR 1 of the 19th-ferry revised roadmap lands (entropy-scanner + ADR). Next migration candidates named: DiskBackingStore stays BLOCKER until PR 5 simulates it (not accepted-boundary — simulation is correct answer there). Future multi-node network I/O stays BLOCKER until PR 8. Composes with PR #346 dst-compliance-criteria doc, PR #344 19th-ferry absorb, .claude/skills DST guide, existing push-with-retry.sh implementation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ening - thread PRRT_kwDOSF9kNM59Wql1 (line 12, P1): self-resolving; docs/research/dst-compliance-criteria.md now exists on main via a later merge; cross-reference is valid, no edit needed. - thread PRRT_kwDOSF9kNM59Wqmv (line 42, P2): schema relaxed from "one of the 12 DST entropy classes" to "one or more of the 12 DST entropy classes, comma- or `+`-separated when a site genuinely crosses multiple (e.g. a network boundary whose only retry policy is itself a distinct entropy source)". Keeps the first entry's combined "external network I/O + retry-on-failure" value accurate — retry-on-failure is a policy IN the network-I/O boundary, not a separate class. - thread PRRT_kwDOSF9kNM59Wqm8 (line 172, P1): same self-resolve as thread 1; docs/research/dst-compliance-criteria.md exists. - thread PRRT_kwDOSF9kNM59WqnP (line 125, P1): DiskBackingStore.fs and ISimulationFs are not in src/; rephrased as forward-looking with explicit "(planned, not yet landed)" parentheticals plus a sentence clarifying this row is a placeholder the scan can compare against once PR 5 of the 19th-ferry roadmap arrives.
02847ee to
c918c28
Compare
| - Amara 19th ferry — `docs/aurora/2026-04-24-amara-dst- | ||
| audit-deep-research-plus-5-5-corrections-19th-ferry.md` |
There was a problem hiding this comment.
P1 (xref/doc): The inline code path to the Amara 19th-ferry doc is split across two lines (backtick opens on line 180 and closes on 181). Markdown inline code spans can’t contain newlines, so this will render oddly and makes the path hard to copy/paste. Keep the entire docs/aurora/... path on a single line (or switch to a normal link / reference-style link).
| - Amara 19th ferry — `docs/aurora/2026-04-24-amara-dst- | |
| audit-deep-research-plus-5-5-corrections-19th-ferry.md` | |
| - Amara 19th ferry — `docs/aurora/2026-04-24-amara-dst-audit-deep-research-plus-5-5-corrections-19th-ferry.md` |
| - `DiskBackingStore.fs` (planned, not yet landed in | ||
| `src/` — referenced here as the target of PR 5 of the | ||
| 19th-ferry revised roadmap) — will be classified BLOCKER | ||
| on landing and immediately migrated to SIMULATED via | ||
| `ISimulationFs` (also planned, not yet landed). The | ||
| BLOCKER → SIMULATED transition is the PR 5 body of | ||
| work; this registry entry is the forward-looking | ||
| placeholder so the accepted-boundary scan has a row to | ||
| compare against when the code arrives. |
There was a problem hiding this comment.
P1 (config-drift): This “Pending classifications” entry says DiskBackingStore.fs is “planned, not yet landed in src/”, but DiskBackingStore<'K> already exists in src/Core/DiskSpine.fs. Either update this row to reference the current implementation (and its actual entropy boundary: real filesystem I/O), or clarify that this is referring to a distinct, future DiskBackingStore.fs refactor so readers don’t assume disk I/O isn’t already present.
| - `DiskBackingStore.fs` (planned, not yet landed in | |
| `src/` — referenced here as the target of PR 5 of the | |
| 19th-ferry revised roadmap) — will be classified BLOCKER | |
| on landing and immediately migrated to SIMULATED via | |
| `ISimulationFs` (also planned, not yet landed). The | |
| BLOCKER → SIMULATED transition is the PR 5 body of | |
| work; this registry entry is the forward-looking | |
| placeholder so the accepted-boundary scan has a row to | |
| compare against when the code arrives. | |
| - `DiskBackingStore<'K>` already exists in | |
| `src/Core/DiskSpine.fs`; its current entropy boundary is | |
| real filesystem I/O, so this implementation is pending | |
| formal classification as BLOCKER until it is simulated. | |
| The 19th-ferry PR 5 roadmap item refers to the planned | |
| refactor path (`DiskBackingStore.fs` plus | |
| `ISimulationFs`, both not yet landed), not to the | |
| existence of disk-backed storage itself. That future work | |
| is expected to perform the BLOCKER → SIMULATED | |
| transition. |
Summary
Closes Amara 19th-ferry correction #3 on
tools/git/push-with-retry.sh. Audit finding: the script already has the discipline Amara asked for (root-cause block, targeted 5xx retries, logging, backoff, cap, distinct exit codes). The gap was doc-level visibility, not implementation.Two changes:
docs/research/dst-accepted-boundaries.mdregistry with schema + push-with-retry.sh as first entry (full rationale, investigation summary, retry discipline breakdown, revisit triggers).Registry research-grade; promotes to
docs/DST-ACCEPTED-BOUNDARIES.mdtop-level when PR 1 of the 19th-ferry revised roadmap lands the entropy-scanner + ADR.Test plan
🤖 Generated with Claude Code