Skip to content

docs(todo): close the SE-audit CI-lock row into its Deferred buckets#665

Merged
igerber merged 4 commits into
mainfrom
chore/se-audit-row-restructure
Jul 9, 2026
Merged

docs(todo): close the SE-audit CI-lock row into its Deferred buckets#665
igerber merged 4 commits into
mainfrom
chore/se-audit-row-restructure

Conversation

@igerber

@igerber igerber commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Summary

Methodology references (required if estimator / math changes)

  • Method name(s): N/A - no methodology changes (TODO.md bookkeeping only).
  • Paper / source link(s): N/A
  • Any intentional deviations from the source (and why): None

Validation

  • Tests added/updated: No test changes (no code changed).
  • Backtest / simulation / notebook evidence (if applicable): N/A

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

Every sub-item is now resolved or bucketed: (a) G2 hetero/cluster regen
DONE (#644, 2026-07-07); C6 CLOSED (#658); C7/C8 TWFE live-R bands
tightened + df-convention root-caused (#663, with its own Actionable
row). The three remaining letters move to Deferred where they belong:
(b) PlaceboTests boundary_gap -> Paper-gated (user-locked 2026-07-09:
defer until a derivation/paper source exists); (c) StackedDiD intercept
SEs -> Won't-fix/waived (~0.3% reference-cell gap, not lockable);
(d) estimatr classical intercept SE -> Won't-fix/waived (documented
O(1/n) deviation, reference-only).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lbd6nqWmg4snvvBmegwqiw
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • PR is TODO.md bookkeeping only; no estimator, variance, weighting, inference, or default behavior changes.
  • The clustered-CR1 df convention remains documented in REGISTRY.md as a deviation note, so the retained TODO row is not a methodology defect.
  • PlaceboTests boundary_gap is properly moved to deferred/paper-gated work; no current code path computes that field.
  • The estimatr classical intercept-SE waiver is consistent with the registry’s documented O(1/n) classical deviation.
  • One P2 documentation formatting issue: two new “Won’t-fix / waived” rows have four cells under a three-column table header.

Methodology

No unmitigated findings.

Severity: P3 informational
Impact: The added deferrals/waivers document non-code work rather than changing methodology. The clustered-CR1 df convention is already documented as a REGISTRY deviation, and PlaceboTests boundary_gap is explicitly deferred pending derivation/source material.
Concrete fix: None required for methodology approval.

Code Quality

No findings. No code changed.

Performance

No findings. No runtime paths changed.

Maintainability

No findings beyond the documentation table issue listed below.

Tech Debt

No blocking findings.

Severity: P3 informational
Impact: The PR moves deferred SE-audit items into TODO.md buckets, which satisfies the project’s deferred-work tracking rule for non-silent, non-P0/P1 items.
Concrete fix: None required.

Security

No findings. The diff does not add secrets, executable code, workflows, or external calls.

Documentation/Tests

Severity: P2
Location: TODO.md:L141-L144
Impact: The “Won’t-fix / waived” table header has three columns (Decision, Location, Verified), but the two new rows add a fourth Low cell. Markdown renderers may drop or misalign that priority value, making the waiver table harder to read.
Concrete fix: Either add a Priority column to the header/separator or remove the trailing | Low | from the two new rows. No tests needed for this docs-only fix.

Review P2: the Won't-fix table is (Decision | Location | Verified); my
two rows carried a fourth Priority cell from the Deferred-table shape.
Reshaped with bolded decision leads and SE-audit/2026-07-09 Verified
stamps, matching the neighboring rows.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lbd6nqWmg4snvvBmegwqiw
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 248934cc3b727a1f7351678a85f5896bd64172b7


Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • This PR changes TODO.md only; no estimator, inference, weighting, or default behavior changed.
  • The prior P2 table-formatting issue is resolved: the new waived rows now match the three-column table at TODO.md:L141-L144.
  • Clustered-CR1 df remains documented as a REGISTRY deviation, so the retained TODO row is not a methodology defect.
  • PlaceboTests boundary_gap is properly deferred as paper-gated work, but its TODO location references a non-existent file.
  • No security, runtime, or performance risk introduced.

Methodology

Severity: P3
Impact: The affected methods are documentation-only references to PlaceboTests, StackedDiD, clustered CR1 inference df, and estimatr/classical intercept SE parity. No code path changed. The clustered CR1 residual-df convention is documented as a REGISTRY deviation at docs/methodology/REGISTRY.md:L362-L374, and the estimatr classical intercept exclusion is documented at docs/methodology/REGISTRY.md:L3231.
Concrete fix: None required for approval.

Code Quality

No findings. No code changed.

Performance

No findings. No runtime paths changed.

Maintainability

Severity: P2
Impact: The new deferred PlaceboTests boundary_gap row points to diff_diff/placebo.py, but that file does not exist; the placebo API is implemented in diff_diff/diagnostics.py. This can misdirect future follow-up work. Location: TODO.md:L66.
Concrete fix: Change the row location from diff_diff/placebo.py to diff_diff/diagnostics.py.

Tech Debt

Severity: P3
Impact: The PR moves SE-audit residual items into explicit Deferred / Documented or waived buckets, which satisfies the project’s deferred-work tracking rule for non-blocking documentation/test/parity items. Location: TODO.md:L66, TODO.md:L143-L144.
Concrete fix: None required.

Security

No findings. The diff adds no code, secrets, workflow changes, or external calls.

Documentation/Tests

Severity: P2
Impact: Same documentation-location issue as above: TODO.md:L66 references diff_diff/placebo.py instead of the actual diff_diff/diagnostics.py.
Concrete fix: Update the path. No tests needed for this docs-only correction.

…s not exist)

Review P2 x2 (same issue): PlaceboTests is implemented in
diff_diff/diagnostics.py; I wrote a nonexistent diff_diff/placebo.py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lbd6nqWmg4snvvBmegwqiw
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: d34b48ff8421c3fdaa89ad9b53c4e92ecec25d22


Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • Re-review scope: TODO.md only; no estimator, inference, weighting, variance, assumptions, or defaults changed.
  • Prior P2 is resolved: PlaceboTests boundary_gap now points to diff_diff/diagnostics.py at TODO.md:L66.
  • Clustered-CR1 df and estimatr classical-intercept deviations are documented in REGISTRY.md, so they are not methodology defects.
  • P2 docs issue remains/newly identified: the estimatr waiver row points to the fixest TWFE test file instead of the estimatr parity test file.
  • No runtime, performance, or security risk introduced.

Methodology

Severity: P3
Impact: Documentation-only reclassification of SE-audit items. The affected references are PlaceboTests, StackedDiD, clustered CR1 inference df, and estimatr IV/classical intercept SE parity, but no implementation path changed. The CR1 df convention is documented at docs/methodology/REGISTRY.md:L362-L374; estimatr classical/intercept exclusion is documented at docs/methodology/REGISTRY.md:L3229-L3231; PlaceboTests is documented under diff_diff/diagnostics.py at docs/methodology/REGISTRY.md:L3474-L3506.
Concrete fix: None required for approval.

Code Quality

No findings. No code changed.

Performance

No findings. No runtime paths changed.

Maintainability

Severity: P2
Impact: TODO.md:L144 describes an estimatr classical intercept SE waiver, but its Location column points to tests/test_fixest_did_twfe_parity.py. The relevant estimatr parity file is tests/test_estimatr_iv_robust_parity.py:L1-L7, with intercept-SE assertions/waiver context at tests/test_estimatr_iv_robust_parity.py:L109-L117. This can misdirect future SE-audit follow-up.
Concrete fix: Change the row location from tests/test_fixest_did_twfe_parity.py to tests/test_estimatr_iv_robust_parity.py.

Tech Debt

Severity: P3
Impact: Moving PlaceboTests boundary_gap into Deferred / Documented at TODO.md:L66 properly tracks paper-gated work. The waived StackedDiD and estimatr parity rows at TODO.md:L143-L144 are non-blocking documentation of audit decisions.
Concrete fix: None required, aside from the P2 location correction above.

Security

No findings. The diff adds no code, workflows, secrets, or external calls.

Documentation/Tests

Severity: P2
Impact: Same documentation-location issue as Maintainability: TODO.md:L144 should reference the estimatr parity test file, not the fixest TWFE parity file. No tests are needed for this docs-only correction.
Concrete fix: Update the Location column to tests/test_estimatr_iv_robust_parity.py.

Review P2 x2 (same issue): the classical-intercept O(1/n) waiver context
lives in tests/test_estimatr_iv_robust_parity.py (module docstring +
test_hc1_intercept_se_matches_estimatr), not the fixest TWFE parity
file. Verified against the tree this time, as were the other two rows'
paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lbd6nqWmg4snvvBmegwqiw
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 53a08474ce10daf14d6aa02e3faa380969a508a8


Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • Re-review scope is TODO.md only; no estimator, inference, weighting, variance, assumptions, or defaults changed.
  • Prior P2 is resolved: the estimatr waiver now points to tests/test_estimatr_iv_robust_parity.py at TODO.md:L144.
  • Methodology-adjacent prose touches PlaceboTests, StackedDiD, clustered CR1 df convention, and estimatr classical/intercept SE parity, but all changes are tracking/waiver documentation.
  • Clustered CR1 df convention remains actionable at TODO.md:L54 and is documented as a REGISTRY deviation at docs/methodology/REGISTRY.md:L362-L375.
  • No runtime, performance, security, or test-risk impact from this docs-only PR.

Methodology

  • Severity: P3-informational
    Impact: PlaceboTests boundary_gap is moved into Deferred / Documented at TODO.md:L66. This is explicitly framed as a new paper-gated feature/result field, not an existing computed statistic. Current PlaceboTests methodology and inference contracts are documented at docs/methodology/REGISTRY.md:L3470-L3521.
    Concrete fix: None required.

  • Severity: P3-informational
    Impact: estimatr classical intercept SE exclusion is now tracked at TODO.md:L144 and matches the documented O(1/n) classical deviation / intercept-SE note at docs/methodology/REGISTRY.md:L3229-L3231. This is documented and not a methodology defect.
    Concrete fix: None required.

  • Severity: P3-informational
    Impact: StackedDiD nuisance intercept SE waiver is documented at TODO.md:L143; no public estimator surface or variance computation changes. Existing StackedDiD R-parity tests target event-study and aggregate CR1/CR2 inference, not a new intercept-SE field.
    Concrete fix: None required.

Code Quality

  • Severity: None
    Impact: No code changed.
    Concrete fix: None.

Performance

  • Severity: None
    Impact: No runtime paths changed.
    Concrete fix: None.

Maintainability

  • Severity: None
    Impact: The previous maintainability issue is fixed: TODO.md:L144 now references the estimatr parity file, consistent with tests/test_estimatr_iv_robust_parity.py:L1-L7.
    Concrete fix: None.

Tech Debt

  • Severity: P3-informational
    Impact: Deferred work is now split into actionable, paper-gated, and waived buckets. Clustered CR1 df remains actionable at TODO.md:L54; PlaceboTests boundary_gap is tracked under Deferred at TODO.md:L66; waived SE-audit decisions are recorded at TODO.md:L143-L144.
    Concrete fix: None required.

Security

  • Severity: None
    Impact: No code, workflows, credentials, or external calls changed.
    Concrete fix: None.

Documentation/Tests

  • Severity: None
    Impact: Docs-only bookkeeping change; no tests are needed for runtime behavior. Prior docs-location issue is resolved at TODO.md:L144.
    Concrete fix: None.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 9, 2026
@igerber igerber merged commit 370af8c into main Jul 9, 2026
5 of 6 checks passed
@igerber igerber deleted the chore/se-audit-row-restructure branch July 9, 2026 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant