Skip to content

Add development checklists and workflow improvements to CLAUDE.md#92

Merged
igerber merged 1 commit intomainfrom
docs/pr-review-workflow-improvements
Jan 20, 2026
Merged

Add development checklists and workflow improvements to CLAUDE.md#92
igerber merged 1 commit intomainfrom
docs/pr-review-workflow-improvements

Conversation

@igerber
Copy link
Copy Markdown
Owner

@igerber igerber commented Jan 20, 2026

Summary

  • Add estimator inheritance map to Key Design Patterns section
  • Add test writing guidelines under Test Structure section
  • Add development checklists for adding parameters and warning/error handling

Based on lessons learned from PR #91 review process (3+ rounds to merge due to missing get_params() param, warning message mismatch, and tests that checked invariants but not behavior).

Methodology references (required if estimator / math changes)

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

Validation

  • Tests added/updated: No test changes (documentation-only)
  • Backtest / simulation / notebook evidence (if applicable): N/A

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

Add estimator inheritance map, test writing guidelines, and development
checklists based on lessons learned from PR #91 review process. These
additions help prevent common issues:

- Estimator inheritance map: Shows which classes inherit get_params/set_params
  to prevent missing parameters in subclasses
- Test writing guidelines: Emphasizes testing behavior (not just no-crash)
  for fallback paths, new parameters, and warnings
- Development checklists: Actionable steps for adding parameters and
  warning/error handling with verification steps

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Overall assessment: ✅ Looks good

Executive summary

  • Documentation-only change; no estimator, math, or default behavior modifications.
  • Adds clear inheritance map and checklists that should reduce param/warning inconsistencies.
  • No methodology registry impact; no paper cross-check needed.
  • Minor suggestion to tie checklist to registry/assumption checks (low severity).

Methodology

  • Severity: P3 | Impact: No methodology impact; PR only updates documentation (CLAUDE.md). Concrete fix: None required.

Code Quality

  • Severity: P3 | Impact: Guidance is helpful but could be more actionable by referencing where get_params/set_params live. Concrete fix: Consider adding a short pointer to the relevant base class in CLAUDE.md near the inheritance map.

Performance

  • Severity: P3 | Impact: No runtime changes. Concrete fix: None.

Maintainability

  • Severity: P3 | Impact: Checklists improve maintainability; small gap is lack of explicit linkage to methodology/assumption checks. Concrete fix: Add a checklist item to verify any new param doesn’t alter identification assumptions unless documented in docs/methodology/REGISTRY.md.

Tech Debt

  • Severity: P3 | Impact: No new debt introduced. Concrete fix: None.

Security

  • Severity: P3 | Impact: No code changes or secrets. Concrete fix: None.

Documentation/Tests

  • Severity: P3 | Impact: Documentation improved; no tests affected. Concrete fix: None.

References

  • CLAUDE.md

@igerber igerber merged commit f838bef into main Jan 20, 2026
1 check passed
@igerber igerber deleted the docs/pr-review-workflow-improvements branch January 20, 2026 23:36
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