Repository navigation
docs: add engineer-facing TDD testing playbook - #6411
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
📝 WalkthroughWalkthroughAdds an Ironclaw TDD playbook covering test selection, placement, workflow, repository examples, and PR checklists, with a new reference from ChangesTesting guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 0 | 0 | 4ecae9dee343 |
Head: 4ecae9dee34313460e009d7973f3787e22e9cb35
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Reviewed the complete normal-shape documentation diff (2 files, 460 added lines). The playbook link, repository paths, test commands, and cited examples align with current repository guidance and implementations.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/internal/testing-playbook.md`:
- Around line 338-340: Update the persistence guidance near the statement about
multiple backends to remove the exception that allows production behavior to
omit libSQL or PostgreSQL. Require both backends for new persistence features,
or reference the repository’s documented approved-exception policy if one
exists.
- Around line 447-448: Update the final test rule in the testing playbook to
require both unit or contract coverage and a hermetic feature test only for
production-wired Reborn or cross-component changes. Align the wording with the
behavior- and risk-based layer selection described in the referenced workflow
sections, without imposing both test types on all production changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d70ec8f1-fcf4-44a4-aeee-82787933d24c
📒 Files selected for processing (2)
CONTRIBUTING.mddocs/internal/testing-playbook.md
There was a problem hiding this comment.
Code Review
This pull request introduces the "Ironclaw TDD Playbook for Engineers" (docs/internal/testing-playbook.md) and updates CONTRIBUTING.md to reference it. The playbook outlines the testing strategy, detailing the four test types (unit/contract, hermetic feature, surface, and live canary), a step-by-step workflow, test locations, and a pull request test card template. The reviewer suggested referring to the root configuration file as the "workspace Cargo.toml" instead of "root Cargo.toml" for consistency with other documentation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| For a distinct scenario, add `tests/integration/<scenario>.rs` and register the | ||
| flat test binary in the root `Cargo.toml` as `reborn_integration_<scenario>`. |
4ecae9d to
3dd6659
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/internal/testing-playbook.md`:
- Around line 194-195: Update the production-facing persistence guidance in the
testing playbook to explicitly require coverage for both supported libSQL and
PostgreSQL paths, removing the conditional wording; only reference an
approved-exception policy if one already exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fc904783-fc6e-460d-8277-41cd20e66ce1
📒 Files selected for processing (2)
CONTRIBUTING.mddocs/internal/testing-playbook.md
| - For production-facing persistence, cover supported libSQL and PostgreSQL | ||
| paths as required by the owning contract. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Require both persistence backends explicitly.
CONTRIBUTING.md Lines 133-135 requires new persistence features to support both PostgreSQL and libSQL. “As required by the owning contract” weakens that invariant and can legitimize incomplete backend coverage. Require both paths, or link an explicit approved-exception policy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/internal/testing-playbook.md` around lines 194 - 195, Update the
production-facing persistence guidance in the testing playbook to explicitly
require coverage for both supported libSQL and PostgreSQL paths, removing the
conditional wording; only reference an approved-exception policy if one already
exists.
|
🚅 Deployed to the ironclaw-pr-6411 environment in ironclaw-ci-preview
|
3dd6659 to
fc18769
Compare
Summary
CONTRIBUTING.mdso contributors can find it during developmentChange Type
Linked Issue
None.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildcargo test --features integrationif database-backed or integration behavior changedgit diff --checkand the trailing-whitespace check passedreview-prorpr-shepherd --fixwas run before requesting reviewSecurity Impact
None. Documentation-only change.
Reborn Trust-Boundary Checklist
N/A. This documents existing testing practices and does not change Reborn behavior, types, or trust boundaries.
Database Impact
None.
Blast Radius
Documentation only:
docs/internal/testing-playbook.mdand a discoverability link inCONTRIBUTING.md.Rollback Plan
Revert commit
fc187695d.Review Follow-Through
The complete original playbook was restored after feedback that the shortened maintainability rewrite removed too much practical integration and live-canary guidance.
Review track: A (docs/tests/chore)