docs: add P-014 (semantic resolution) and P-015 (config surface) prop…#12
Merged
Conversation
…osals P-014 — project-local SemanticModel for the C# extractor. Fixes the OWN001 event-leak false positives where a syntax-only `+=` match flags numeric accumulation as event subscriptions (32% FP on a real WPF codebase: `sum += value`, `x.Qty += y.Qty`, even `for(...; i += step)`). Tier A builds one CSharpCompilation over all inputs with TPA framework references (zero-config, no MSBuild), classifies a `+=` as a subscription only when the LHS binds to an IEventSymbol, and emits an honest OWN050 "leakage analysis skipped" warning for unresolved external types instead of guessing. Includes the incremental rollout (event fact first), versioning/anti-drift analysis, and a Tier A build plan. P-015 — stub for a linter-style configuration surface (check selection + per-category severity, e.g. own.toml). Generalises P-014's `--event-leaks` gate. Also fix the stale proposals index (docs/proposals/README.md), which was missing P-013 entirely; add P-013/P-014/P-015 rows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds three new documents to ChangesSemantic resolution and configuration proposals
Incremental computation planning
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/proposals/P-014-semantic-resolution.md (1)
206-207: ⚡ Quick winClarify whether unresolved-marker facts persist in OwnIR or are ephemeral intermediates.
Line 206–207 (T2) describes emitting an "unresolved marker fact (e.g.
resource: "unresolved-subscription")," while line 282–285 (Versioning) states the implementation "introduces no new OwnIR input fact at all." The tension suggests the marker is an intermediate artifact (emitted by the extractor, processed by the core to produce the OWN050 Finding), but this is not explicit. Clarify whether the marker is:
- Ephemeral (extractor → core only, not persisted), or
- Persisted in OwnIR but does not require a version bump.
💡 Suggested clarification for the T2 description
Revise line 206–207 to:
- - unresolved (`Symbol == null` / error-typed receiver) → emit an **unresolved - marker** fact (e.g. `resource: "unresolved-subscription"`) for the OWN050 - channel. **Do not** fall back to the old syntactic guess. + - unresolved (`Symbol == null` / error-typed receiver) → emit an **unresolved + marker** (e.g. as an intermediate signal tagged `resource: "unresolved-subscription"`) + to the core's OWN050 channel. This marker is not persisted in OwnIR; the core + converts it to a Finding in T4 (see *Versioning*). **Do not** fall back to the + old syntactic guess.Also applies to: 282-285
🤖 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/proposals/P-014-semantic-resolution.md` around lines 206 - 207, The documentation creates ambiguity about whether the unresolved marker fact described in lines 206–207 (T2 section) is an ephemeral intermediate or persisted in OwnIR, contradicting the statement in lines 282–285 (Versioning section) that no new OwnIR input fact is introduced. Explicitly clarify in the T2 section (lines 206–207) whether the unresolved marker fact is ephemeral (extracted, processed by the core to generate OWN050 Finding, then discarded) or persisted in OwnIR but does not require a version bump. Ensure this clarification is consistent with the Versioning section (lines 282–285) by either confirming no OwnIR persistence occurs or adjusting the Versioning language to acknowledge the marker's presence without triggering a version change.
🤖 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/proposals/P-014-semantic-resolution.md`:
- Around line 273-280: Update the cross-proposal reference citation in the text
discussing the P-013 "one checker" rule. Locate the phrase that currently reads
"P-013:19-21" within the context of "its governing constraint is the ROADMAP
'one checker' rule" and change the line range from "19-21" to "18-20" to
correctly point to where the "one checker" rule is actually defined in proposal
P-013.
---
Nitpick comments:
In `@docs/proposals/P-014-semantic-resolution.md`:
- Around line 206-207: The documentation creates ambiguity about whether the
unresolved marker fact described in lines 206–207 (T2 section) is an ephemeral
intermediate or persisted in OwnIR, contradicting the statement in lines 282–285
(Versioning section) that no new OwnIR input fact is introduced. Explicitly
clarify in the T2 section (lines 206–207) whether the unresolved marker fact is
ephemeral (extracted, processed by the core to generate OWN050 Finding, then
discarded) or persisted in OwnIR but does not require a version bump. Ensure
this clarification is consistent with the Versioning section (lines 282–285) by
either confirming no OwnIR persistence occurs or adjusting the Versioning
language to acknowledge the marker's presence without triggering a version
change.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b3b599c0-76b4-4b6f-bda6-1a9ba27f7dfe
📒 Files selected for processing (3)
docs/proposals/P-014-semantic-resolution.mddocs/proposals/P-015-configuration-surface.mddocs/proposals/README.md
Comment on lines
+273
to
+280
| `OWNIR_VERSION` bump. `OWNIR_VERSION = 0`, bumped only "whenever the fact | ||
| vocabulary changes incompatibly" (ownir.py:77-81); the existing additive precedent | ||
| is explicit — "the resource/type fields are additive and optional, so they do NOT | ||
| bump `ownir_version`" (ownir.py:48-51). `load()` rejects only a mismatched integer | ||
| and treats an absent field as current (ownir.py:200-208). This discipline | ||
| originates in **P-001:93-97** and the `ownir.py` docstring — *not* P-013 (an | ||
| earlier draft mis-cited P-013, which contains no version rule; its governing | ||
| constraint is the ROADMAP "one checker" rule, P-013:19-21). |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify P-013 file exists and contains the cited passage.
if [ -f docs/proposals/P-013-*.md ]; then
rg -n 'one checker' docs/proposals/P-013-*.md -B 2 -A 2 | head -20
else
echo "P-013 file not found in docs/proposals/"
fiRepository: PhysShell/Own.NET
Length of output: 563
🏁 Script executed:
#!/bin/bash
# Check P-013 for version-related content
echo "=== Checking for version/versioning content in P-013 ==="
rg -i 'version' docs/proposals/P-013-*.md -n | head -20
echo ""
echo "=== Full context around lines 18-21 in P-013 ==="
sed -n '15,25p' docs/proposals/P-013-*.md | cat -nRepository: PhysShell/Own.NET
Length of output: 682
Correct the line number citation for the P-013 "one checker" rule.
The cross-proposal reference is substantively correct: P-013 does contain the "one checker" rule that governs the versioning constraint, and P-013 contains no version rule. However, the rule spans lines 18–20, not 19–21. Update the citation to: "its governing constraint is the ROADMAP 'one checker' rule, P-013:18-20."
🤖 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/proposals/P-014-semantic-resolution.md` around lines 273 - 280, Update
the cross-proposal reference citation in the text discussing the P-013 "one
checker" rule. Locate the phrase that currently reads "P-013:19-21" within the
context of "its governing constraint is the ROADMAP 'one checker' rule" and
change the line range from "19-21" to "18-20" to correctly point to where the
"one checker" rule is actually defined in proposal P-013.
This was referenced Jun 16, 2026
This was referenced Jun 26, 2026
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.
…osals
P-014 — project-local SemanticModel for the C# extractor. Fixes the OWN001 event-leak false positives where a syntax-only
+=match flags numeric accumulation as event subscriptions (32% FP on a real WPF codebase:sum += value,x.Qty += y.Qty, evenfor(...; i += step)). Tier A builds one CSharpCompilation over all inputs with TPA framework references (zero-config, no MSBuild), classifies a+=as a subscription only when the LHS binds to an IEventSymbol, and emits an honest OWN050 "leakage analysis skipped" warning for unresolved external types instead of guessing. Includes the incremental rollout (event fact first), versioning/anti-drift analysis, and a Tier A build plan.P-015 — stub for a linter-style configuration surface (check selection + per-category severity, e.g. own.toml). Generalises P-014's
--event-leaksgate.Also fix the stale proposals index (docs/proposals/README.md), which was missing P-013 entirely; add P-013/P-014/P-015 rows.
Summary by CodeRabbit