Repository navigation
example cleanup - #12
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (14)
💤 Files with no reviewable changes (4)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe pull request removes the legacy pre-cutover capture from the repository and contract artifacts. It updates conformance and project documentation, restricts push-triggered CI to ChangesLegacy archive and conformance cleanup
CI trigger scope
Priority: ⬇️ Low — Defer this cleanup because it removes legacy documentation and conformance artifacts while adding narrow replay-boundary tests, with no stated customer or production impact. Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change removes the legacy archive from contract and documentation scope, adds replay-boundary coverage, and limits push CI to main. No merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Full details: Title checkExplanation The title is related to the removal and revision of example artifacts, but "example cleanup" is too broad and does not identify the main changes, including removal of the legacy archive and addition of replay-boundary tests. Full details: Docstring CoverageExplanation Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 5 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
The capture deleted in the previous commit was still wired into four code paths, so its removal broke the build rather than completing it. paa_contracts lists every corpus directory in _REQUIRED_ROOTS and refuses to import over a partial set, which took down the conformance suite, the contract package tests and the wheel verification together. Drop the directory from _REQUIRED_ROOTS, the build hook, the wheel verifier and the roots test, and delete the conformance test that replayed it. The docs claimed more for that artifact than it could carry. It was generated from the same author's earlier implementation, and recorded production_event_count_at_cutover: 0 -- a cross-implementation continuity proof where both implementations are ours and the source system never ran a transition. PAA.md and README.md no longer cite it, and the scope-of-claim sentence drops the clause it was supporting. import_events stays. It is exported from paa_runtime and the replayed capture was its only test, so removing the fixture would have shipped a public API with no coverage. tests/test_replay.py now covers it directly -- fields preserved verbatim, one atomic transaction, empty input a no-op -- on events it builds itself, since what is worth pinning is that the fields survive the call and not that a particular file exists. Also scope the push trigger to main. Unfiltered, it fired alongside pull_request on every branch push, building each PR commit twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KF3srA4ZPg86FbETY3ZA2E
One move, repeated: state a fact, then add a clause about how disciplined it was to state it. "Honest non-matches", "required, not decoration", "the trade-off is named rather than hidden", "that is the part worth stating", "the one failure mode it must not have", "the ratchet that keeps this honest", "implementation-neutrality made mechanical instead of asserted". Each one asks the reader to admire the rigor instead of just showing it. Every fact survives; only the self-assessment is gone. "Scope of the claim" loses the pull-quote staging and says the same thing in prose. The conftest docstring drops a paragraph narrating its own development history, which no reader of the fixture needs. Left alone: the "deliberately"/"on purpose" uses that carry real information -- a deliberately tampered fixture, stages deliberately not checked, structural rules deliberately absent. Those distinguish intent from accident, which is worth saying in a spec. Docstrings under src/paa_runtime were already dry and are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KF3srA4ZPg86FbETY3ZA2E
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Chores
Documentation
Tests