docs(P-035): project-declared weak-subscription conventions + corpus case#283
Conversation
…case Motivated by a real conversion: a net472 WPF codebase's static-publisher subscription leak could NOT be fixed with the BCL WeakEventManager / PropertyChangedEventManager -- they are thread-affine (the objects are built on background threads) and did not resolve in the data-layer's WPF markup-compile pass. The correct fix was a small project-owned thread-agnostic weak forwarder. Lesson: the accepted weak release is project-specific. P-004 Open Question #4 assumed it is the BCL manager; this generalises it -- a repo declares its own weak-subscribe API (a [weak-subscription] table in P-015's config), and own-check (a) recognises that wrapper as an accepted release (subscriber-side sibling of the #223 curated allowlist, but config-extensible) and (b) names it in the OWN001 fix text / hands it to the 007 fix agent instead of a WeakEventManager that may not compile or work there. Adds: - docs/proposals/P-035-custom-weak-subscription.md (+ README index row) - corpus/wpf/custom-weak-wrapper/ (before.cs +=, after.cs weak wrapper, case.own -> OWN001, expected-diagnostics.txt, notes.md). The .own reduction exercises the leak with today's checker; recognising the fixed form as silent is the P-035 recognition half (regression fixture for it). NB: the .own fixture is a faithful clone of corpus/wpf/zombie-viewmodel (same grammar; acquire Subscribe / release Unsubscribe already used elsewhere) but was not run here (ownlang needs the nix/WSL devShell). Confirm with pytest tests/test_wpf.py before relying on it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a WPF custom weak-subscription corpus fixture, expected OWN001 diagnostics, project-owned weak wrapper examples, and draft P-035 documentation for configuring recognition and fix behavior. ChangesWeak subscription fixture
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ed9093371
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,24 @@ | |||
| module WpfCustomWeakWrapper | |||
There was a problem hiding this comment.
Adding this .own file under corpus/ makes it part of the fixture sweeps in tests/test_cfg_fixtures.py and tests/test_diag_fixtures.py, but the generated JSON fixtures were not updated. I checked those tests and both now fail with the fixture-stale message because tests/fixtures/cfg_parity.json and tests/fixtures/diag_parity.json do not include corpus/wpf/custom-weak-wrapper/case.own; since tests/run_tests.py auto-discovers these tests, the zero-dependency regression suite is left red until the fixtures are regenerated.
Useful? React with 👍 / 👎.
… case
Adding corpus/wpf/custom-weak-wrapper/ made the shared parity goldens stale
(run_tests.py: "cfg_parity.json / diag_parity.json is stale"). Regenerated with
`python tests/test_{cfg,diag}_fixtures.py --write`; the diff only adds the new
case's CFG + diagnostics (DocumentViewModel: one acquire, no release -> OWN001),
nothing else changed. run_tests.py is green again.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
1 participant
What
WeakEventManager. A repo declares its own weak-subscribe API (a[weak-subscription]table in P-015's config); own-check then (a) recognises that wrapper as an accepted release (subscriber-side sibling of the#223curated allowlist, but config-extensible) and (b) names it in the OWN001 fix text / hands it to the 007 fix agent, instead of aWeakEventManagerthat may not fit.corpus/wpf/custom-weak-wrapper/— the regression fixture:before.cs(+=leak),after.cs(project weak wrapper),case.own→ OWN001,expected-diagnostics.txt,notes.md.Why
Motivated by a real conversion: a net472 WPF codebase's static-publisher subscription leak could not be fixed with the BCL
WeakEventManager/PropertyChangedEventManager— they are thread-affine (the objects are built on background threads) and did not resolve in the data-layer's WPF markup-compile pass. The correct fix was a small project-owned thread-agnostic weak forwarder. Lesson: a tool that only knowsWeakEventManagerwill mis-suggest a fix that doesn't compile there, and (once method-call subscriptions are seen) re-flag correctly-fixed code.Generalises P-004 Open Question #4 (recognise weak subscription as an accepted release), which had assumed the BCL manager.
Verified
python -m ownlang check corpus/wpf/custom-weak-wrapper/case.own→OWN001;tests/test_wpf.py→ 10/10 cases match (the new case included).Follow-ups (not in this PR): the extractor seam (config
--weak-subscription+ method-call subscription detection + release marking) and the 007 fix-agent wiring.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests