Skip to content

docs: correct Dube et al. (2025) JAE citation to issue 7#610

Open
igerber wants to merge 1 commit into
mainfrom
fix/dube-2025-jae-citation
Open

docs: correct Dube et al. (2025) JAE citation to issue 7#610
igerber wants to merge 1 commit into
mainfrom
fix/dube-2025-jae-citation

Conversation

@igerber

@igerber igerber commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Corrects the LP-DiD primary-source citation from Journal of Applied Econometrics 40(5):741-758 to the published 40(7):741-758 across all five live sites: docs/references.rst, docs/methodology/REGISTRY.md, docs/methodology/papers/dube-2025-review.md, docs/api/lpdid.rst, and carousel/generate_lpdid_carousel.py. The 3.6.0 release notes retain the original attribution per the repo's historical-CHANGELOG convention; a [Unreleased] → Fixed entry supersedes it.

Methodology references (required if estimator / math changes)

  • Method name(s): LP-DiD (citation metadata only — no estimator or math changes)
  • Paper / source link(s): Dube, Girardi, Jordà & Taylor (2025), J. Applied Econometrics 40(7):741–758, https://doi.org/10.1002/jae.70000 — issue number verified against the published record (IDEAS/RePEc wly/japmet/v40y2025i7p741-758)
  • Any intentional deviations from the source (and why): none

Validation

  • Tests added/updated: none needed (docs/comment-string change only; no code paths touched)
  • Backtest / simulation / notebook evidence (if applicable): grep -rn '40(5)' docs/ carousel/ diff_diff/ returns zero hits post-change; the only remaining 40(5) is the intentionally retained 3.6.0 CHANGELOG history

Security / privacy

  • Confirm no secrets/PII in this PR: confirmed — citation text only

🤖 Generated with Claude Code

https://claude.ai/code/session_012YazJXUeSs5dtLWR5DHpBr

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

Overall Assessment

⚠️ Needs changes — no methodology blocker, but the PR introduces a P1 release/version metadata regression.

Executive Summary

  • LP-DiD is the only affected method, and the changed estimator content is citation metadata only.
  • The 40(7):741-758 citation is consistently reflected in the Methodology Registry and live LP-DiD docs.
  • No estimator math, weighting, variance/SE, assumptions, or default behavior changed.
  • P1: the PR downgrades package metadata from 3.6.2 to 3.6.1 across Python, Rust, citation metadata, and generated LLM guide files.
  • The remaining 40(5) occurrence is in historical CHANGELOG.md release notes and is acceptable per the stated historical-entry convention.

Methodology

Finding: P3 informational — LP-DiD citation metadata corrected
Impact: The Methodology Registry now cites Dube, Girardi, Jordà & Taylor (2025) as Journal of Applied Econometrics 40(7):741-758, matching the updated LP-DiD API/reference docs. No methodology deviation is introduced.
Concrete fix: None required. References: docs/methodology/REGISTRY.md:L1801, docs/api/lpdid.rst:L64, docs/references.rst:L262, docs/methodology/papers/dube-2025-review.md:L4-L16.

Code Quality

Finding: No code-quality issue in estimator logic
Impact: The only Python source change is __version__; no inference, weighting, aggregation, or estimator code path is modified. Known anti-patterns such as inline inference computation and incomplete estimator parameter propagation are not introduced.
Concrete fix: None.

Performance

Finding: No performance issue
Impact: Citation and metadata changes do not affect runtime behavior.
Concrete fix: None.

Maintainability

Finding: P1 — version metadata is downgraded from 3.6.2 to 3.6.1
Impact: A build from this PR would report/package itself as an older already-released version, which can create duplicate release artifacts, package-index upload failures, incorrect diff_diff.__version__, and stale citation metadata. This is outside the stated citation-only scope.
Concrete fix: Restore the intended current version metadata, or split the release-state rollback into a separate explicit release-maintenance PR. Affected locations: pyproject.toml:L7, diff_diff/__init__.py:L304, rust/Cargo.toml:L3, CITATION.cff:L10-L11, diff_diff/guides/llms-full.txt:L5, CHANGELOG.md:L8, CHANGELOG.md:L2093.

Tech Debt

Finding: No untracked deferred-work issue
Impact: No new deferrable implementation limitation is introduced.
Concrete fix: None.

Security

Finding: No security issue
Impact: No secrets, credentials, PII, or unsafe code paths are introduced by the changed files.
Concrete fix: None.

Documentation/Tests

Finding: P3 informational — tests are not necessary for the citation-only docs edits
Impact: The citation correction is covered by direct textual consistency. The only remaining 40(5) occurrence is in historical CHANGELOG.md:L282, which is documented as intentionally retained.
Concrete fix: None for tests. Keep the historical changelog entry if that is the repo convention.

Path to Approval

  1. Restore 3.6.2 metadata in pyproject.toml, diff_diff/__init__.py, rust/Cargo.toml, CITATION.cff, and diff_diff/guides/llms-full.txt, unless there is an explicit release-management reason handled in a separate PR.
  2. Restore the CHANGELOG.md 3.6.2 heading/link, or split the new citation fix into an [Unreleased] section without deleting/downgrading the existing release metadata.

The LP-DiD primary source was cited as J. Applied Econometrics 40(5):741-758
in five doc/code sites; the published record is 40(7):741-758 (DOI
10.1002/jae.70000). Fixed in references.rst, REGISTRY.md, dube-2025-review.md,
docs/api/lpdid.rst, and the carousel generator; CHANGELOG [Unreleased] Fixed
entry supersedes the 3.6.0 release-note attribution per the repo's
historical-entry convention.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@igerber igerber force-pushed the fix/dube-2025-jae-citation branch from 4bf7d0f to 9a4ed4a Compare July 3, 2026 23:55
@igerber

igerber commented Jul 4, 2026

Copy link
Copy Markdown
Owner Author

/ai-review

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 9a4ed4a4428466307a2fdfc10f29b2e791d9ac8c


Overall Assessment

Looks good — no unmitigated P0 or P1 findings.

Executive Summary

  • Affected method: LPDiD; the PR changes citation metadata only.
  • No estimator math, weighting, variance/SE, identification assumptions, defaults, or inference paths changed.
  • The prior P1 version-metadata regression is resolved: project metadata remains 3.6.2.
  • The remaining 40(5) occurrence is only in the historical 3.6.0 changelog entry and is explicitly superseded by the new [Unreleased] note.
  • One minor documentation-formatting nit is noted below; it is not blocking.

Methodology

  • Finding: LP-DiD citation metadata is consistently updated.
    • Severity: P3 informational
    • Impact: docs/methodology/REGISTRY.md:L1801, docs/methodology/papers/dube-2025-review.md:L4-L16, docs/api/lpdid.rst:L62-L64, and docs/references.rst:L262 now agree on Journal of Applied Econometrics, 40(7):741-758. No methodology deviation is introduced.
    • Concrete fix: None required.

Code Quality

  • Finding: No estimator-code quality issue.
    • Severity: P3 informational
    • Impact: The only Python change is a carousel citation string at carousel/generate_lpdid_carousel.py:L816-L820; no inline inference, parameter propagation, control-group logic, or aggregation path changed.
    • Concrete fix: None required.

Performance

  • Finding: No performance issue.
    • Severity: P3 informational
    • Impact: Citation text changes do not affect runtime performance.
    • Concrete fix: None required.

Maintainability

  • Finding: Prior version-metadata regression is resolved.
    • Severity: P3 informational
    • Impact: Current metadata remains 3.6.2 in pyproject.toml:L7, diff_diff/__init__.py:L304, rust/Cargo.toml:L3, CITATION.cff:L10, diff_diff/guides/llms-full.txt:L5, and CHANGELOG.md:L20.
    • Concrete fix: None required.

Tech Debt

  • Finding: No untracked deferred-work issue.
    • Severity: P3 informational
    • Impact: The PR does not add implementation limitations or deferred estimator behavior.
    • Concrete fix: None required.

Security

  • Finding: No security issue.
    • Severity: P3 informational
    • Impact: The diff contains citation/documentation text only; no secrets, PII, credentials, or unsafe execution paths are introduced.
    • Concrete fix: None required.

Documentation/Tests

  • Finding: Minor Markdown emphasis nesting in the new changelog entry.

    • Severity: P3
    • Impact: CHANGELOG.md:L11-L12 wraps the whole citation in bold and also uses **7** inside it, which may render the issue number inconsistently depending on the Markdown renderer. This is cosmetic only.
    • Concrete fix: Optional: change 40(**7**):741-758** to 40(7):741-758**, or remove the outer bolding and keep only the issue-number emphasis.
  • Finding: Tests are not needed for this docs-only correction.

    • Severity: P3 informational
    • Impact: The retained historical citation at CHANGELOG.md:L283-L284 is mitigated by the new superseding note at CHANGELOG.md:L8-L18.
    • Concrete fix: None required.

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