docs: architecture review, tech-debt register, P-022 review notes#167
Conversation
External-style review of the whole repo: core pipeline, OwnIR seam, Roslyn extractor, codegen/buffers/lifetimes/di, tests, CI, audit/. Prioritized risks (OwnIR contract formalization, god-files, DI spec drift) and recommendations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X
Follow-up to the July 2026 architecture review. Records: formalize OwnIR instead of replacing it (fail-loud unknown ops, golden facts snapshots, spec + JSON Schema, evolution policy); the extractor/bridge mirror is producer/consumer, pin it rather than remove it; rewrite is parked behind explicit triggers with the C#-vs-Rust target decision left open; now / with-next-touch / on-trigger / rejected buckets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X
…into review branch
…ollisions Reviews the merged P-022 + strictness notes: the ownir.py bridge (~900 lines of verdict-determining lowering/inference) has no crate in the topology; the IDE-extension goal is the legitimate Gate-B trigger and forces panic=unwind for the LSP binary; indexing_slicing=deny collides with the dense-Vec hot path; residual cheap fixes (stale own-ir label, byte-offset spans, NonZeroU32 ids, release profile TOML, golden-snapshot oracle caching, error-text parity fixtures). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X
|
Warning Review limit reached
Next review available in: 52 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)
📝 WalkthroughWalkthroughThis PR adds three new non-normative Markdown documentation files under docs/notes/: an architecture review (July 2026) enumerating strengths and prioritized risks R1–R6, follow-up review notes for proposal P-022, and a tech-debt register with bucketed action items. No code or public API changes are included. ChangesArchitecture and Tech-Debt Documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/notes/p022-review-notes.md`:
- Around line 154-157: The strictness note in the review docs currently
contradicts the workspace’s actual Ruff configuration by stating select =
["ALL"] without reflecting the deliberate curated selection in pyproject.toml.
Reword the note in the section describing Ruff strictness so it clearly
distinguishes the current workspace config from the proposed stricter variant,
and use the ruff configuration entry and pyproject.toml as the anchors for the
wording.
In `@docs/notes/tech-debt-register.md`:
- Around line 29-34: Reword the disclaimer in the tech-debt register so it no
longer conflicts with §1 and the closing notes about OwnIR formalization and
bridge relocation. Update the wording around the “no item below is justified by
prepares the rewrite” statement to clearly separate standalone debt items from
any future rewrite-related considerations, and make sure the language in the
surrounding section stays consistent with the same framing.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 6b29d387-98e2-4401-ae35-018cfe656e72
📒 Files selected for processing (3)
docs/notes/architecture-review-2026-07.mddocs/notes/p022-review-notes.mddocs/notes/tech-debt-register.md
Register: the standalone-justification disclaimer no longer contradicts the §1 P-022 update note and the §3 rewrite prerequisites. P-022 notes: distinguish the proposed ruff select=ALL variant from the current curated pyproject.toml selection instead of calling it a contradiction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X
…collisions Folds the accepted findings from docs/notes/p022-review-notes.md (PR #167): - own-bridge crate (highest cost): today's ownir.py beyond the schema (~2k lines of verdict-determining logic — facts->core lowering, the interprocedural MOS inference, BCL fresh-factory table, check_facts verdict mapping) had no named home in the DAG. Added the crate, its edges (ir+cfg+analysis+diagnostics -> bridge; only own-cli may depend on it), a migration step 6 with the normative-semantics prerequisite, and the fitness rule. - IDE extension as the recorded Why (Gate B of incremental-computation): reorders perf priorities to latency/incrementality over batch (the 10-100x figures are batch-CLI numbers), makes panic=abort per-binary (LSP needs unwind — salsa cancels via unwinding), promotes error-tolerant parsing to a requirement, and documents the hybrid extension shape (in-process Roslyn extraction + Rust core over the OwnIR seam). salsa open question upgraded from maybe to Gate-B substance. - indexing_slicing deny vs dense-Vec state: pre-resolved — justified module-scoped allow on the arena-indexed state container, deny stays elsewhere. - Cheap residuals: own-ir relabeled fact contract (verdicts live in own-diagnostics), span primitives pinned to the leaf (not own-syntax), byte-offsets-internally row, NonZeroU32 IDs, [profile.release] as TOML, oracle diffs golden snapshots keyed by (corpus hash, py commit) instead of live Python per CI pass, error-text parity fixtures, #[cold] on diagnostic paths, SmallVec measured-not-sprinkled, enum-size ritual.
2 participants
Что и зачем
Три ревью-документа в
docs/notes/: (1) архитектурное ревью всего репозитория (ядро, OwnIR-шов, экстрактор, codegen, тесты, CI) с приоритизированными рисками; (2) техдолг-реестр с bucket'ами now / with-next-touch / on-trigger / rejected и планом формализации OwnIR (fail-loud unknown ops → golden facts → schema → evolution policy); (3) ревью влитого P-022 — главное: в топологии крейтов нет дома для OwnIR-моста (~900 строк verdict-determining логики изownir.py), IDE-цель — легитимный Gate-B-триггер и требуетpanic = "unwind"для LSP-бинаря,indexing_slicing = "deny"конфликтует с dense-Vecна горячем пути.Тип изменения
Как проверено
Docs-only: код не менялся, тесты/линтеры не затронуты. Ссылки
file:lineв документах сверены с состоянием дерева на момент коммитов (включая mergeorigin/mainс P-022).python tests/run_tests.pyruff check .иmypypython scripts/<...>.py --selftest)Связанные issue
Нет связанных issue. Дополняет P-022 (PR #166):
p022-review-notes.md— правки к пропозалу до первого коммита скелета workspace.Чеклист
feat:,fix:,docs:…)🤖 Generated with Claude Code
https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X
Generated by Claude Code
Summary by CodeRabbit