From 20077b3733417c55588210a7d8d2987a4b6f4696 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Jul 2026 02:55:12 +0000 Subject: [PATCH 1/4] docs: architecture, libraries and approaches review (July 2026) 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 Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X --- docs/notes/architecture-review-2026-07.md | 260 ++++++++++++++++++++++ 1 file changed, 260 insertions(+) create mode 100644 docs/notes/architecture-review-2026-07.md diff --git a/docs/notes/architecture-review-2026-07.md b/docs/notes/architecture-review-2026-07.md new file mode 100644 index 00000000..2156a042 --- /dev/null +++ b/docs/notes/architecture-review-2026-07.md @@ -0,0 +1,260 @@ +# Архитектурное ревью Own.NET — июль 2026 + +> Статус: внешнее ревью (не нормативный документ). Охват: ядро `ownlang/`, +> OwnIR-мост и Roslyn-экстрактор, codegen/buffers/lifetimes/di, тестовая +> стратегия, CI, `audit/`, документационный процесс. Смотрели по коду с +> точными ссылками `file:line` на момент коммита ревью. + +## Вердикт одним абзацем + +Это необычно дисциплинированный PoC. Архитектурный скелет — слоёное ядро +(parser → CFG → worklist-fixpoint dataflow → diagnostics) с фронтендами, +которые *только извлекают факты*, — концептуально верный и, что важнее, +реально соблюдается: OwnIR-мост делегирует вердикты ядру, а не дублирует его. +Тестовая и eval-инфраструктура (дифференциальный фаззер с независимым +оракулом, метаморфические тесты, spec-conformance, cross-tool oracle против +Infer#/CodeQL, реальная компиляция golden-примера в CI) — сильнее, чем у +большинства зрелых анализаторов. Главные долги: **контракт OwnIR не имеет +формальной спецификации** (при том что это несущая балка всей +мульти-фронтенд-архитектуры), два бога-файла (`ownir.py` 2430 строк, +`Program.cs` 4392 строки), DI-подсистема выпала из spec-дисциплины, и +рукописная тестовая/CI-обвязка начала обгонять свою управляемость +(900-строчный CI-job, 25-членный `or` в test-раннере). + +--- + +## 1. Что архитектурно сильно + +### 1.1 Ядро: слоистость и корректность dataflow + +- Импорт-граф `ownlang/` — чистый DAG без циклов: листья + `lexer`/`ast_nodes`/`diagnostics`/`evidence`, дальше `parser` → `cfg` → + `analysis` → `__main__`. Учебная слоистость. +- Решётка анализа корректна: состояние переменной — множество над + `{OWNED, MOVED, RELEASED, ESCAPED}`, merge = union (конечная + join-полурешётка), transfer монотонен, worklist сходится без widening + (`analysis.py:192-208`, `348-371`). Разделение фаз «тихий fixpoint → эмиссия + диагностик по сошедшимся in-state» (`analysis.py:379-397`) закрывает тонкий + баг дублирования диагностик в циклах by design. +- Займы (loans) — first-class `Loan` рядом с состоянием владельца, а не вместо + него; permissions выводятся на лету. Разделение definite/maybe + (OWN002/005 vs OWN009/010) естественно падает из решётки множеств. +- `typing.assert_never` на каждом exhaustive-диспатче (`cfg.py:384`, + `analysis.py:622`) — новый вариант AST/instr становится type-ошибкой. +- `ownership.py` — итеративная конденсация Тарьяна + least fixpoint по SCC на + 4-точечной решётке, `unknown` fail-closed — один из самых сильных модулей. + +### 1.2 «Один чекер» — принцип, который реально работает + +- `check_facts()` → `to_module()` → `check_module()` (`ownir.py:1895-1896`); + DI/effects делегируются `di.py`/`effects.py`. Логика OWN001/OWN014 не + переписана вторым чекером. +- Само-детекция дрейфа: если ядро выдало диагностику, которую мост не может + привязать к handle, мост **падает**, а не молча теряет находку + (`ownir.py:1927-1934`). Отличный предохранитель для лик-чекера. +- Roslyn-экстрактор действительно type-aware (SemanticModel, биндинг `+=` к + `IEventSymbol`), а не синтаксический grep. + +### 1.3 Верификация и eval — лучшая часть репозитория + +- `tests/test_codegen_props.py` — настоящий дифференциальный фаззер: + инварианты пересчитываются независимым оракулом прямо по AST, плюс + assert на невакуумность покрытия (прогон не может пройти «впустую»). + В CI — 50k прогонов с ротацией seed. +- Метаморфические тесты (`scripts/metamorphic*.py`) на ядре и на fact-мосте. +- `tests/test_spec.py`: каждое нормативное правило `spec/` обязано выстрелить + на каноническом примере — живая спека, а не витрина. +- Двухслойный корпус: reduced `.own` (точные коды) + реальные `before.cs`/ + `after.cs` с измерением recall/specificity (`scripts/benchmark.py`); чужой + код не коммитится. +- Cross-tool oracle (Own.NET vs Infer# vs CodeQL, единый SARIF-парсер, + FP-baseline без привязки к номерам строк) — использование зрелых тулов как + планки recall и как оракула. Честно и грамотно. +- `dotnet-golden` job компилирует и запускает сгенерированный C# — lowering + доказан исполнением, а не «by construction». +- CI: least-privilege permissions, env-passing вместо строковой интерполяции, + allowlist-валидация входов. + +### 1.4 Процесс + +- Proposals P-001…P-021 с обязательными Non-goals; ADR → task-spec → test + прослеживается 1:1 (`AGENTS.execution-surfaces.md` → + `docs/tasks/evidence-coverage.md` → `tests/test_evidence_coverage.py`). +- Культура honest-skip / `NO-TOOL` / OWN050 последовательно проведена через + чекер, oracle и audit. ADR об отказе от Forth-VM как внутреннего IR — + правильное решение с правильной аргументацией (graph/fixpoint-задача, а не + линейный поток; естественная будущая цель — Datalog, не стековый байткод). + +--- + +## 2. Главные архитектурные риски (по приоритету) + +### R1. Контракт OwnIR — «джентльменское соглашение», а не спецификация + +Вся мульти-фронтенд-архитектура держится на OwnIR, но: + +- **Версия схемы захардкожена в трёх местах на двух языках** без общего + источника: `ownlang/ownir.py:159`, `Program.cs:4360`, `ownts.py:580`. + Гарантия «fail loudly on mismatch» работает, только если каждый продюсер + вручную не забыл поднять литерал. Забыли поднять в `Program.cs` — ядро + молча неверно прочитает факты. Это ровно тот failure mode, от которого + гейт должен защищать. +- **Нет `spec/OwnIR.md` и нет JSON Schema.** Контракт живёт в 90-строчном + docstring (`ownir.py:1-97`) + рукописном валидаторе `load()`. Каждый новый + фронтенд ре-деривит формат из прозы; словарь resource-kind продублирован + руками в `_RESOURCES` (`ownir.py:208-215`), в emit-сайтах `Program.cs` и в + `ACQUIRES` (`ownts.py:60-66`). +- Валидация размазана между `load()` и `to_module()` (`ownir.py:760-775`) — + два разных набора гарантий. + +### R2. Бога-файлы и дрейф зеркальной логики Python↔C# + +- `ownir.py` (2430 строк) совмещает пять работ: рендеринг находок + (human/GitHub/MSBuild/SARIF), загрузку/валидацию, lowering фактов в AST, + **межпроцедурный inference** (~900 строк: `solve`, `_infer_return_skeleton`, + `_infer_param_effect`, BCL-таблицы, `_lower_flow`) и оркестрацию проверок. + `check_facts` — ~280 строк (`ownir.py:1879-2157`), каждый новый resource + kind добавляет ветку именно сюда. +- `Program.cs` (4392 строки) — процедурный top-level + ~130 плоских статиков + без типовой декомпозиции, глобальный мутабельный флаг `BodyThrowEdges` + (`Program.cs:4389`), `LowerFlowStmt` на 417 строк (`Program.cs:1384-1801`). +- **`LowerFlowStmt` (C#) и `_lower_flow` (Python) — зеркала, которые должны + совпадать поведенчески вручную**, без общего тестового оракула. Это + структурный риск дрейфа между двумя языками. +- Inference-логика живёт в мосте — ни во фронтенде (где SemanticModel), ни в + ядре (как переиспользуемый pass). Любой будущий фронтенд, желающий + flow-locals, привязывается к внутренностям моста, а не к чистому IR. + +### R3. DI-подсистема — вне общей дисциплины + +- DI001–DI005 (и EFF) нет ни в `spec/`, ни в `Diagnostics.md`, ни в + `test_spec.py` — второй по размеру анализатор (485 строк, 5 диагностик, + уровень warning для пользователей) живёт без нормативного управления, + которое проект держит для всего остального. +- Пять DFS-обходов графа регистраций — копипаста одного скелета + (`di.py:128/199/267/345/438`) + пять почти одинаковых result-датаклассов; + напрашивается один параметризованный `_walk_registration_graph`. +- `Service` — frozen-датакласс на 13 полей, конструируемый позиционно; + комментарии «new fields LAST… positional-contract safe» показывают, что + авторы сами знают о хрупкости. `kw_only=True` снял бы риск целиком. + +### R4. Долги ядра (не критично, но копится) + +- `cfg.py` совмещает резолвер + сбор сигнатур + lowering + **тайпчек + return'ов** (`_Builder.lower_return`, `cfg.py:603-658` — семантический + анализ внутри построителя CFG). Чистое разбиение: resolver → typed AST → + lowering. +- Коды диагностик — голые строковые литералы без compile-time-связи с + каталогом `TITLES`; опечатка в коде обнаружится только в рантайме + (`diagnostics.py:243` молча вернёт `""`). Для проекта с `mypy --strict` это + самая большая дыра типовой дисциплины. Нужен `Code`-enum/`Final`-константы. +- Синтаксические ошибки маркируются как `OWN020` «unsupported construct» + (`__main__.py:80`) — неверная категория, вводит в заблуждение. +- `check_module(mod: object)` + шесть `# type: ignore` (`__main__.py:60-66`) — + дыра в строгой типизации ровно на швах сборки пайплайна. +- `Symbol` (cfg.py:45-73) — мутабельный после `declare` 12-польный + god-struct; безопасен только по конвенции. +- Инвариант блочной скоупированности займов держится на голом `assert` + в `join()` (`analysis.py:200-203`) — срезается `python -O`; при появлении + `break` из-под займа станет либо молчаливо неверным ответом, либо крэшем. +- `_sym_by_id` — рефлексия по строковым именам атрибутов + (`analysis.py:433`) — стрингли-типизированное дублирование знания о форме + инструкций, против которого в остальном коде стоит `assert_never`. + +### R5. Codegen: строковая шаблонизация с молчаливыми деградациями + +- Двухстратегийный дизайн (try/finally hoist vs faithful inline) с выбором в + одной точке `_is_simple()` (`codegen.py:77-95`) — сильная часть; но: +- `emit_*`-шаблоны подставляются verbatim без проверки наличия + placeholder'ов (`codegen.py:511/518/525`) — шаблон без `{0}` молча теряет + аргумент. +- `cond_text` — сырой прогон токенов; пустое условие эмитит литерал `cond` + (`codegen.py:478/488`) — некомпилируемый C#. +- `_arg` → `/* expr */` и `_size_expr` → `"0"` (`codegen.py:528-533`, + `398-403`) — молчаливые деградации, целиком полагающиеся на upstream. +- Диспатч mode→backend закодирован дважды: `BufferInfo.branches()` + (`buffers.py:116-130`) для отчёта и `codegen._buffer_lowering` + (`codegen.py:254-361`) для эмиссии — отчёт может разъехаться с кодом. + +### R6. Тестовая/CI-обвязка обгоняет свою управляемость + +- `tests/run_tests.py:1144-1149` — рукописный 25-членный `or`: добавил + тест-модуль, забыл вписать его `rc` — он **молча перестаёт гейтить**. +- Нет измерения покрытия вообще; кастомный раннер его не умеет. +- `tests/test_ownir.py` — 2292 строки одним модулем. +- `ci.yml:133-1049` — 900-строчный job `wpf-extractor` из десятков хрупких + `grep -q` по формулировкам сообщений; любое переформулирование — красный CI. + Это должен быть Python golden/snapshot-тест. +- Дублированный 15-строчный блок framework-refs в трёх workflow; + dev-branch-остатки с пометкой «remove before merging to main» + (`mine-on-push.yml:11-16`, `oracle.yml:45-49`) закоммичены в дерево; + actions не запинены по SHA (осознанно отложено, но exposure реальный). +- `audit/` — по собственному README «не источник истины», живая разработка в + отдельном репо OwnAudit: в дереве лежит устаревший форк. + +--- + +## 3. Оценка выбора библиотек и стека + +| Выбор | Оценка | +|---|---| +| Python 3.11+, stdlib-only ядро | ✅ Правильно для PoC: скорость итераций, нулевой порог входа, воспроизводимость. Компенсировано `mypy --strict` + ruff + `assert_never`. | +| Отказ от pytest в пользу самописного раннера | ⚠️ Цена уже видна (ручная агрегация, нет coverage, нет параметризации/изоляции). «Zero-dependency» осмыслен для *runtime*, но pytest/coverage как **dev-зависимости** ядро не загрязняют. Минимум — автодискавери модулей в раннере вместо ручного `or`. | +| Самописный фаззер вместо hypothesis | ✅/⚠️ Дифференциальный оракул — сильнее, чем типичное использование hypothesis; но shrinking и стратегия покрытия там бесплатны. Не срочно. | +| `Microsoft.CodeAnalysis.CSharp` 4.9.2, без MSBuildWorkspace/Buildalyzer | ✅ для v0: один self-contained `dotnet tool`, error-tolerant SemanticModel без билда — реальное преимущество перед Infer# (нужны бинарники). ⚠️ Ручной текст/XML-парсинг `.csproj`/`.sln` разойдётся с реальной семантикой MSBuild (Directory.Build.props, Compile Remove, multi-targeting). Buildalyzer/MSBuild.Locator стоит держать как opt-in Tier C, не по умолчанию. Версию Roslyn стоит поднимать по мере поддержки новых языковых конструкций. | +| SARIF как единственный нормализованный формат | ✅ Ключевое инфраструктурное решение; уже спасло от бага (regex-парсер молча терял находки). | +| PyYAML только в `audit/` | ✅ Правильная изоляция. | +| Python worklist сейчас, Datalog (Soufflé/Ascent) как горизонт | ✅ Согласен с ADR: fixpoint-по-CFG — не задача для стековой VM; Datalog — естественная цель, когда факты станут межпроцедурными всерьёз (Polonius-подобная формулировка уже совпадает со словарём OwnIR). | + +## 4. Оценка подходов (методологии) + +- **Fail-loud / honest-skip** — последовательно и всерьёз: `emit` отказывается + генерировать при ошибках, буферные политики не имеют молчаливых дефолтов, + OWN050 вместо угаданных ликов, `NO-TOOL` вместо имитации покрытия. Это + правильная культура для safety-чекера, и она реально в коде, а не в README. +- **Инверсия FP/FN-приоритета** (пропуск хуже ложного срабатывания) — + корректна для этого класса инструментов и явно задокументирована. +- **Bug-driven expansion** (фича фронтенда только под реальный баг) — лучший + антидот от «поддержали 40% языка, нашли ноль багов»; вынесенные исключения + (lambda-handler, модульная межпроцедурность по контракту сигнатуры) + аргументированы. +- **Spec-as-test** — работает для OwnCore/Lifetimes/Buffers; сломан для DI/EFF + (см. R3). Дисциплина хороша ровно настолько, насколько она тотальна. +- **Риск стратегического расползания:** 21 proposal при ~9 частично + построенных, горизонты OwnTS/OwnJVM/React/effects при незакрытом gap'е + packaging (собственный `alpha-readiness.md` честно это фиксирует: «capability + is past alpha; the gap is packaging»). Документы сами это знают — но + распределение усилий пока не следует их же выводу. +- **Двуязычие документации** (README 44KB + README.ru 58KB, несущий ADR + по-русски в англоязычном репо) — хазард дрейфа перевода. + +--- + +## 5. Рекомендации (приоритизировано) + +1. **Формализовать OwnIR** (закрывает R1): `spec/OwnIR.md` + JSON Schema; + единый источник версии (генерация константы или CI-assert, что три + литерала совпадают); валидация output всех фронтендов и всех фикстур по + схеме в CI. Это самая дешёвая страховка самого дорогого актива. +2. **Расщепить бога-файлы** (R2): `ownir/` → `{schema,load,lower,inference, + render,check}`; `Program.cs` → классы по доменам (EventSubscriptions, + DisposableFields, ArrayPool, DiGraph, FlowLowering, ProjectResolution, + Cli). Решить, где живёт flow-inference (ядро-pass или фронтенд), и + добавить общий golden-оракул для пары `LowerFlowStmt`/`_lower_flow`. +3. **Вернуть DI/EFF в дисциплину** (R3): спека + `Diagnostics.md` + + `test_spec.py`; один параметризованный обход графа вместо пяти копий; + `kw_only=True` для `Service`. +4. **Типизировать швы ядра** (R4): `Code`-enum для диагностик; отдельный код + синтаксической ошибки вместо OWN020; типизировать `check_module`; + поднять голый assert в `join()` до исключения. +5. **Оздоровить обвязку** (R6): автодискавери в `run_tests.py` (или pytest + dev-only); вынести 900-строчный CI-job в Python golden-тест; composite + action для дублированных блоков; убрать dev-branch-остатки; либо удалить + `audit/` из дерева, либо явно заморозить с указателем на OwnAudit; + coverage-метрика. +6. **Codegen-гигиена** (R5): валидация placeholder'ов в `emit_*`-шаблонах; + единая таблица mode→backend для отчёта и эмиссии; убрать `or 'cond'`. +7. **Горизонт**: Datalog-бэкенд рассматривать после того, как модульная + межпроцедурность упрётся в выразительность worklist'а — не раньше; фокус + ближайшего квартала по собственному же `alpha-readiness.md` — packaging + (`ownsharp check MyApp.sln`), а не новые профили. From c0df0a744ce13b7e128c491b45b6f94c581ae817 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Jul 2026 05:02:44 +0000 Subject: [PATCH 2/4] =?UTF-8?q?docs:=20tech-debt=20register=20=E2=80=94=20?= =?UTF-8?q?buckets,=20triggers,=20OwnIR=20formalization=20plan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X --- docs/notes/tech-debt-register.md | 233 +++++++++++++++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100644 docs/notes/tech-debt-register.md diff --git a/docs/notes/tech-debt-register.md b/docs/notes/tech-debt-register.md new file mode 100644 index 00000000..7b43333a --- /dev/null +++ b/docs/notes/tech-debt-register.md @@ -0,0 +1,233 @@ +# Tech-debt register — what to fix, when, and what triggers it + +> Status: **living register** (not normative, not a proposal). Derived from the +> July 2026 architecture review +> ([architecture-review-2026-07.md](architecture-review-2026-07.md)) and a +> follow-up investigation of the OwnIR seam, the extractor↔bridge "mirror", and +> the rewrite question. It **links, not duplicates**, +> [consolidation-and-positioning.md](consolidation-and-positioning.md) (which +> already tracks the OwnIR `subscriptions→resources` rename, the +> `Program.cs` split trigger, and the `WPFxxx` catalog rename) and the trigger +> discipline in `AGENTS.execution-surfaces.md` §7–8 +> (`trigger = цифры из профилировщика или реальная боль в коде`). +> +> Standing priority (unchanged): **this register is subordinate to shipping the +> alpha** ([alpha-readiness.md](alpha-readiness.md) item A — the +> `ownsharp check MyApp.sln` front door). Every "now" item below is days-scale +> hygiene that must not displace packaging work. + +## 0. How to read the buckets + +- **Now** — standalone value today; cheap; the cost of *not* doing it is paid on + every feature (the repo's fastest-churning surfaces). +- **With next touch** — fix when the surrounding code is next edited anyway; + not worth a dedicated PR. +- **On trigger** — parked behind an explicit condition, recorded here so the + condition (not aesthetics) pulls it off the shelf. +- **Rejected** — considered and refused, with the reason on record. + +A deliberate framing decision: **no item below is justified by "prepares the +rewrite."** A core rewrite is not on the record anywhere (the only mentions are +the hypothetical trigger rows in `AGENTS.execution-surfaces.md:317,327`), and +the repo's own rule says work needs profiler numbers or real pain, not a +speculative future. Every item here pays rent standalone. What a rewrite *would* +need is §1. + +## 1. The rewrite question (recorded, not scheduled) + +**Current position: no rewrite until a trigger fires.** Candidate triggers, in +the spirit of the existing trigger table: + +- measured performance pain at mining scale (profiler numbers on 50+-repo runs, + not vibes); +- Gate A/B of [incremental-computation.md](incremental-computation.md) + (interprocedural whole-program inference, or live in-IDE feedback); +- the imperative rule code becomes unmanageable (>30–50 interdependent rules — + the existing Datalog threshold). + +**If a trigger fires, the target language is an open decision, not a default.** +Both candidates must be argued in a P-NNN proposal before any code: + +- **C# core.** The strongest *packaging* case: extractor + core become one + self-contained `dotnet tool` / Roslyn analyzer; the two-runtime install + (.NET SDK **and** Python 3.11+) — which is exactly alpha-readiness gap A — + disappears; the in-IDE story unblocks (`docs/howto-visual-studio.md:7-13` + documents that the Python core is *why* there is no Roslyn analyzer today: + an in-process analyzer would be a second checker). Multi-stack neutrality is + **not** an argument against C#: neutrality lives in the OwnIR JSON seam, not + in the core's implementation language (the OwnTS frontend is itself Python + today; CodeQL's engine language is invisible to its users). +- **Rust core.** The case: Ascent/Soufflé (already the named Datalog + candidates) and Salsa (the named IDE-incrementality candidate) are + Rust-native; a single static binary distributes without any runtime; the + domain (borrow checking) has its reference implementations there. The cost: + a third language in the repo, and none of the packaging gaps close (the + extractor stays C#, users still install two things). + +**Non-negotiable guardrail either way** (from incremental-computation.md): the +new core is an *optimization, never a new decider* — it must produce +bit-identical verdicts to the Python core over the full corpus, proven by +differential testing, or "one checker" is broken. + +**What is actually rewrite-durable** (the oracle a port would be built +against): `spec/` + `tests/test_spec.py` conformance, the two-layer corpus +(`corpus/` + `scripts/benchmark.py` recall/specificity over real C#), the +differential codegen fuzzer, and end-to-end golden runs (sample `.cs` → +expected diagnostics). **Not** the OwnIR fact schema — it is at version 0, +churning weekly, and would be redrawn at any seam re-cut (§3). Sharpening the +durable set is ordinary test hygiene and is already mostly built. + +## 2. OwnIR: formalize, do not replace + +Investigated and settled: OwnIR is a *data schema for facts at rest*, not a +language. Config/scripting languages (Starlark, Lua, CUE, Dhall) solve a +different problem (executing logic to produce data); MLIR/LLVM are +instruction-level compiler frameworks (massive cost, wrong abstraction, no +fact-interchange precedent); SCIP/LSIF are code-navigation formats; SARIF is +for *results* (already used correctly on the output side); Joern's CPG is the +closest adoptable format but demands a whole-program graph — a frontend-scale +rewrite for no analysis gain. The facts-seam we have is the industry-standard +shape: CodeQL TRAP files, Doop/Soufflé `.facts`, Polonius input relations, +Glean's versioned JSON facts, Infer's Textual `.sil`. Datalog *relations* +become the right vocabulary only if the engine ever moves to a fixpoint/Datalog +core — derivable then as an export from validated OwnIR. + +The formalization stack, in order of actual protection delivered: + +1. **Fail-loud unknown ops** *(now)*. `_lower_flow`'s `if/elif` chain has no + `else` — an unknown `op` is silently dropped (`ownlang/ownir.py:1692-1836`), + and the five structural walkers that hardcode the `if`/`while` recursion + (`_collect_vars`, `_has_bare_return`, `_call_result_callees`, + `_param_signals`, `_forward_targets`) share the hole. A newer extractor + emitting a new compound op under an unbumped version would silently swallow + nested acquires/releases — fabricated OWN001s and missed leaks while every + hand-written fixture stays green. Add `else: raise OwnIRError(...)`. + This single guard is worth more than any schema file. +2. **Golden facts snapshots in CI** *(now)*. Today the extractor→bridge seam is + pinned only at the rendered-diagnostics level (~174 `grep -q` assertions); + the `facts.json` itself is `cat`'d, never diffed. CI already has the + `jq -S` diff machinery and its own rationale for why diagnostics-level + diffing is insufficient (`ci.yml:1002-1019`) — apply it to the seam: snapshot + normalized facts for the pinned samples, and feed the same goldens to + `test_ownir.py` so the Python suite also consumes *extractor-produced* + facts, not only hand-written ones. +3. **`spec/OwnIR.md` + `spec/ownir.schema.json`** (JSON Schema draft 2020-12, + `ownir_version` as a `const`) *(now/short)*. Validate all + `tests/fixtures/ownir/*.json` and the extractor's CI output against it. + Note: the schema must encode the *deliberate* open points — unknown + resource kinds coerce to `subscription` by documented design + (`ownir.py:66-70`), so the resource-kind enum is open, and the schema's job + is shape/type/enum guarantees, not vocabulary closure. +4. **A written evolution policy** *(now, one paragraph in the spec)*: additive + optional fields do not bump `OWNIR_VERSION`; a **new op or changed op + semantics does**. Today's policy only covers the first half, which is + exactly the gap item 1 exploits. +5. Version single-sourcing across the three producers (`ownir.py:159`, + `Program.cs:4360`, `ownts.py:580`) *(with next touch)* — worth doing, but + understand what it does not protect: the dangerous case is *nobody* bumping, + which only items 1–4 catch. +6. Reconcile or explicitly document the BCL fresh-factory asymmetry *(now, + small)*: the extractor's `IsOwningFactory` knows the ADO.NET family + (`ExecuteReader`/`CreateCommand`/`BeginTransaction`, + `Program.cs:2545-2568`) and crypto `Create*` statics; the bridge's + `_BCL_FRESH_BY_NS` (`ownir.py:1179-1199`) lists neither, despite "kept in + lockstep" comments on both sides. Precision-safe today (the Python table is + only consulted for first-party `call` ops), but the lockstep is + comment-enforced and has already quietly diverged. + +Later, if a Rust/C# core materializes: generate types from the same schema +(serde via typify / System.Text.Json source-gen). Protobuf/FlatBuffers only if +fact volume ever makes JSON parse time measurable — no evidence today. + +## 3. The extractor↔bridge "mirror" (LowerFlowStmt / _lower_flow) + +Settled by code reading: **this is not duplicated logic and does not have to be +removed.** The two sides are producer and consumer of one flow-op vocabulary — +C# lowers syntax→facts (throw-edge injection, `using`/`switch`/`do` +desugaring exist *only* there), Python lowers facts→core AST (handle minting, +MOS inference, hoisting exist *only* there). The genuine double-encodings are +three enumerable spots: the BCL fresh-factory tables (§2.6), the +`kind:"pool"` tag, and the op-vocabulary structure re-walked by the five +Python walkers (§2.1). + +- **Now:** pin the contract mechanically instead of by comment — §2.1 + §2.2 + + §2.3 cover it (~1–2 days total). +- **On trigger (P-017 multi-stack becomes real, or a core rewrite fires):** + re-cut the seam so the *core* owns control-flow desugaring (structured + `try`/`using`/`switch` ops with `may_throw` annotations on leaves) and the + ~900 lines of contract inference that today live in the bridge + (`solve`, `_infer_return_skeleton`, `_infer_param_effect`, hoisting). + Otherwise every new frontend reimplements the exception-edge machinery per + language. **What must stay frontend-side regardless:** everything that needs + the `SemanticModel` — pool/factory/adopt classification, may-throw + reasoning, escape shapes. "Dumb frontend, all semantics in core" is not + achievable and is not the goal; the goal is *one* implementation of path + enumeration and ownership inference. +- **Before any core rewrite (hard prerequisite):** the bridge's inference + layer is verdict-determining and has **no normative description** — it is + outside `spec/`, outside the schema, and pinned only by examples in + `test_ownir.py`. Either spec it (consume/borrow/fresh/alias/overwrite rules) + or relocate it into `ownlang/` proper first, so "the core" is coextensive + with "what must be ported." Discovering this mid-port is the expensive + surprise this register exists to prevent. + +## 4. Register + +### Now (standalone value; days-scale; ordered by protection-per-effort) + +| # | Item | Where | Why now | +|---|------|-------|---------| +| N1 | `else: raise` on unknown flow ops (+ the five walkers) | `ownlang/ownir.py:1692-1836` | Silent fact-swallowing → fabricated/missed verdicts (§2.1) | +| N2 | Auto-discovery in the test runner | `tests/run_tests.py:1144-1149` | 25-term hand-rolled `or`; a forgotten `rc` silently stops gating | +| N3 | Golden facts snapshots in CI + feed to `test_ownir.py` | `ci.yml` wpf-extractor job | The seam is never diffed at the facts level (§2.2) | +| N4 | `spec/OwnIR.md` + JSON Schema + evolution policy | new; `ownir.py:1-97` is the source | §2.3–2.4 | +| N5 | DI001–005 + EFF001 into `spec/` + `Diagnostics.md` + `test_spec.py` | `ownlang/di.py`, `effects.py` | Second-largest analyzer has zero normative governance | +| N6 | Diagnostic `Code` enum (replace bare string literals) | `ownlang/diagnostics.py:243` + emit sites | New codes land weekly; a typo'd code silently renders `""` | +| N7 | Split `ownir.py` → `ownir/{render,load,lower,inference,check}` | `ownlang/ownir.py` (2430 lines) | Fastest-accreting file in the repo; `check_facts` grows a branch per resource kind; deferring = paying the god-file tax on every feature | +| N8 | Dedicated syntax-error code (stop filing `ParseError` as OWN020) | `ownlang/__main__.py:80` | Miscategorized as "unsupported construct" | +| N9 | Bare `assert` on the loan invariant → raise | `ownlang/analysis.py:200-203` | Stripped under `python -O`; silent wrong answer if block-scoping is ever relaxed | +| N10 | Reconcile/document BCL table asymmetry | `Program.cs:2545-2568` vs `ownir.py:1179-1199` | Lockstep already quietly diverged (§2.6) | + +### With next touch + +| Item | Do it when | +|------|-----------| +| `di.py`: one parameterized graph walk instead of five copies; `kw_only=True` on `Service` | next DI feature (DI006+ or P-006 open questions) | +| `buffers.branches()` / `codegen._buffer_lowering` single mode→backend table | next buffer/codegen work | +| `emit_*` template placeholder validation; drop the `or 'cond'` literal | next codegen work | +| Version literal single-sourcing across three producers | next OwnIR-schema-touching PR (per consolidation note: fold with the `subscriptions→resources` rename) | +| Move the 900-line wpf-extractor grep job into a Python golden test | next time it breaks on a message rewording (it will) | +| CI: composite action for the 3× duplicated framework-refs block; remove dev-branch push sentinels | next workflow edit | + +### On trigger (parked; the trigger pulls it, not aesthetics) + +| Item | Trigger | Candidate | +|------|---------|-----------| +| Core rewrite (language decision per §1) | profiler numbers at mining scale, Gate A/B, or the >30–50-rule threshold | C# (packaging case) vs Rust (Ascent/Salsa case) — P-NNN required | +| Seam re-cut: control-flow desugaring + bridge inference → core | P-017 goes real, or the rewrite fires | §3 | +| `cfg.py` three-pass split (resolver / typecheck / lowering); `Symbol` freeze; typed `check_module` | `cfg.py` starts churning again, or the rewrite fires | low churn today; contained blast radius | +| `Program.cs` decomposition by concern | merge pain (already recorded in consolidation-and-positioning.md) | EventSubscriptions / DisposableFields / ArrayPool / DiGraph / FlowLowering / ProjectResolution / Cli | +| Datalog core | existing trigger-table row | Ascent / Soufflé | +| Incrementality | Gate B (IDE) | Salsa | +| Protobuf/FlatBuffers for OwnIR | measured JSON parse cost | typify/schemars codegen path first | + +### Rejected (on the record) + +- **Replace OwnIR with an existing language/format** — Starlark/CUE/Dhall + (config/scripting, wrong domain), MLIR/LLVM (instruction-level, massive + cost), SCIP/LSIF (code navigation), CPG/Joern (whole-program graph for no + analysis gain). The facts-seam shape is already the industry standard (§2). +- **Lowering fully in the extractor** — the "second checker" the charter + forbids (`ownir.py:5-7`, ROADMAP "One checker"). +- **Runtime `released?` flags in codegen** — already rejected in README; if we + don't trust the static result we don't ship it. + +## 5. What changed vs the July review's sequencing + +The review's recommendation list keyed several items to "during the rewrite." +This register corrects that: with no rewrite on the record, "during the +rewrite" resolves to "never" for the highest-churn surfaces. The `ownir.py` +split (N7) and the `Code` enum (N6) move to **now**; `cfg.py`/`Symbol`/ +`check_module` stay parked (genuinely low churn). The OwnIR formalization is +re-scoped from "schema + version single-sourcing" to the protection stack in +§2 (fail-loud ops first, schema third, version constant last). From 159b0848714551013ce2656e3707a27fa7ebfe53 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Jul 2026 05:59:33 +0000 Subject: [PATCH 3/4] =?UTF-8?q?docs:=20P-022=20review=20notes=20=E2=80=94?= =?UTF-8?q?=20bridge=20crate=20gap,=20IDE=20trigger,=20lint/perf=20collisi?= =?UTF-8?q?ons?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X --- docs/notes/p022-review-notes.md | 157 +++++++++++++++++++++++++++++++ docs/notes/tech-debt-register.md | 10 +- 2 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 docs/notes/p022-review-notes.md diff --git a/docs/notes/p022-review-notes.md b/docs/notes/p022-review-notes.md new file mode 100644 index 00000000..d3443ae1 --- /dev/null +++ b/docs/notes/p022-review-notes.md @@ -0,0 +1,157 @@ +# P-022 review notes — Rust core migration + the IDE-extension trigger + +> Status: **review notes** (follow-up to +> [architecture-review-2026-07.md](architecture-review-2026-07.md) and +> [tech-debt-register.md](tech-debt-register.md)). Reviewed: P-022 as merged in +> PR #166 (`docs/proposals/P-022-rust-core-migration.md`) and +> [strictness-and-fitness.md](strictness-and-fitness.md). + +## Verdict + +P-022 as merged is a strong proposal: the differential oracle as the spine +(exit/crash gate before any output diff; the explicit "do not reuse +`oracle_compare.py` as the parity oracle" fence; OWN050 pinned to the SARIF +seam; "CFG JSON seam is work to build, not an existing contract"), +strangler-fig bottom-up ordering, the `Diagnostic`-is-data-not-`Err` rule, and +the persistent-vs-arena question left open with a concrete bench protocol +(largest real corpus function, wall-clock **and** RSS). The perf principles +match the external playbook and the ruff/rustc precedent. The findings below +are ordered by cost-of-late-discovery. + +## 1. The OwnIR *bridge* is missing from the crate topology (highest cost) + +The crate map covers syntax → cfg → analysis / codegen / diagnostics / cli, +but ~900 lines of **verdict-determining** logic in today's `ownir.py` have no +named home: facts→core-AST lowering (`_lower_flow`, handle minting, localmap +kill-on-rebind), the interprocedural MOS solver (`_build_skeletons`, `solve`, +`_infer_return_skeleton`, `_infer_param_effect`), branch-local hoisting, the +BCL fresh-factory table, and `check_facts`' verdict mapping. `own-ir` is +declared "fact types + serde, dependency-light" — correct, and therefore this +logic cannot live there; `own-analysis` is the lattice/worklist — not this +either. The migration steps (§ strangler-fig) port parser, cfg, analysis, +diagnostics, codegen, cli — the bridge is never named as a port step, yet +without it the flagship path (real C# → facts → verdicts) has no Rust +implementation. + +**Fix:** add an `own-bridge` (or `own-ir-lower`) crate to the DAG +(`own-ir + own-cfg → own-bridge → own-analysis` consumers), a migration step +for it (between analysis and cli), and — per the tech-debt register — write +the normative description of the inference semantics +(consume/borrow/fresh/alias/overwrite rules) *before* porting, since today it +is pinned only by `test_ownir.py` examples. Discovering this mid-port is the +expensive surprise; the OwnIR fixtures in the oracle corpus would catch the +*behavior* but not tell anyone *where the code goes*. + +## 2. The IDE goal is the missing "Why" — and it changes two design points + +P-022's motivation is the maintenance tax; under the repo's own trigger +discipline (`AGENTS.execution-surfaces.md` §7–8: profiler numbers or real +pain) that is the weakest possible justification. The actual trigger now on +the table — **an IDE extension** — is Gate B of +[incremental-computation.md](incremental-computation.md), i.e. a *recorded, +legitimate* trigger. Writing it into P-022's Why both squares the proposal +with the house rules and reorders the perf priorities: an IDE cares about +**keystroke latency and incrementality**, not batch throughput — which is +exactly where Rust+salsa beats "fast Python" by construction, and where the +10–100× batch numbers are the wrong metric. + +Two concrete design consequences: + +- **`panic = "abort"` must be per-binary, not doctrine.** Fine for `own-cli`; + fatal for a long-lived LSP server, where one panic in one file's analysis + takes down the whole session — and salsa's *cancellation is implemented via + unwinding*, so an LSP binary needs `panic = "unwind"` + catch at the request + boundary (rust-analyzer's model). Record it now so "abort in release" + doesn't get baked into the workspace profile. +- **Error-tolerant parsing gets promoted** from nice-to-have to requirement: + an IDE analyzes broken code on every keystroke. This strengthens the + existing "hand-roll the parser" lean (rust-analyzer precedent) beyond the + error-message-parity argument. + +**Architecture reality-check for the IDE scenario.** For the flagship use +case (real C# in the editor), the latency chain is keystroke → Roslyn +semantic model (in-process in the IDE, already incremental) → fact extraction +→ core verdicts. The Rust core is the *cheap* half of that chain; the +extraction must run in-process on the .NET side regardless. So the realistic +extension shape is hybrid: the VS/VS Code extension hosts the (existing C#) +extraction and talks to the Rust core over the OwnIR seam (stdio or FFI) — +which is exactly the seam P-022 already freezes. Rust removes the Python +runtime from the distribution (alpha-readiness gap A closes); it does not +remove the .NET half, and no one should expect it to. For `.own` files a pure +Rust LSP is trivial and a good first slice. + +## 3. `indexing_slicing = "deny"` collides with the core data structure + +The perf doctrine's centerpiece is a dense `Vec` indexed by RID +(and flat backing arrays everywhere); the strictness block denies `v[i]` +(`indexing_slicing`), denies `unwrap_used`, and the hot path cannot afford +`.get().expect(...)` noise either. Left unresolved, the first hot loop starts +the suppress-without-looking reflex the ratchet doctrine explicitly warns +against. **Resolution to record:** arena/newtype-index access +(`la-arena`-style `arena[idx]` where the ID was minted by that arena) is +panic-free by construction — allow indexing there via a *justified, +module-scoped* `#[allow(clippy::indexing_slicing)]` on the state container, +keep the deny everywhere else. Decide it in the proposal, not in the first +PR's review thread. + +## 4. Residuals from the external review of the pre-merge draft + +The pasted external critique's blocker — the `own-diagnostics` arrow vs +fitness-function conflict — is **already fixed on main** (commit `60b45f9`), +and resolved the *opposite* way from the critique's suggestion: the +Diagnostic/Evidence model stays in `own-diagnostics` and `own-analysis` +depends on it. That resolution is sound (arguably better than moving verdicts +into `own-ir`, which stays pure facts). Still-standing residuals, all cheap: + +- **Stale label:** the diagram still calls `own-ir` the "fact/**verdict** + contract" while verdicts now live in `own-diagnostics`. One-word fix, + prevents the next reader re-importing the ambiguity. +- **Span placement:** `own-diagnostics` "depends only on span/location + primitives (`own-ir`/`own-syntax`)" — if `Span` lives in `own-syntax`, the + presentation crate pulls the parser. Put the span type in a leaf (`own-ir` + or a tiny `own-span`) instead. +- **Positions:** internal representation should be **byte offsets** + one + line-index per file, line/col only at the output seam (ruff / + rust-analyzer). Not fixed anywhere in P-022; cheap now, painful later. +- **IDs:** `Rid(u32)` → `Rid(NonZeroU32)` etc., so `Option` is free + (niche optimization). Day-1 decision. +- **`[profile.release]` block** (lto = "thin", codegen-units = 1, + opt-level = 3; panic per §2) is described in prose but absent as TOML. +- **Oracle cost:** don't run the Python core live on every CI pass — commit + its outputs as golden snapshots keyed by (corpus hash, Python-core commit) + and diff Rust against the snapshots; regenerate only when Python itself + changes (which the proposal already treats as the exception). Otherwise CI + time grows with corpus × Python and the ratchet gets disabled "temporarily". +- **Error-text parity:** extract parser/diagnostic message texts into shared + fixtures asserted from both sides, or parity rests on copy-paste. +- **Enum-size ritual:** `clippy::large_enum_variant` is already in the + default warn set (perf group) — the missing piece is the habit: audit + `size_of::()`/`()` (assert in a test) whenever IR types change. + +## 5. Notes on the performance playbook (the external document) + +Direction confirmed; the u8-`bitflags`-over-`fixedbitset` correction, RPO +worklist scheduling, and the serde_json-I/O caveat are all right and already +in the merged P-022. Additions/corrections worth keeping: + +- The 10–100× figure is a *batch CLI* number (ruff's). If the CLI on today's + corpus is not actually CPU-bound, the visible batch win may be modest — the + IDE latency budget (§2) is where the migration genuinely pays. Set + expectations accordingly in any public claim. +- `#[cold]` on the diagnostic-construction paths is a good, stable-Rust hint + (the playbook is right that `likely/unlikely` are unstable). +- `SmallVec` caveat is real; measure before sprinkling. Evidence steps and + loans-per-owner match the small-N profile, block instruction lists may not. +- Skip the HFT residue (busy-poll, thread-per-core, lock-free) — the playbook + itself says so; recorded here so it doesn't get cargo-culted later. + +## 6. strictness-and-fitness.md + +Sound doctrine (ratchet, baseline, justified allows). Two notes: (a) +`import-linter` for `ownlang` is the cheapest immediate win — the Python +import DAG is already clean (verified in the July review), so the contract +would pass on day one and prevent regressions while Python remains the +oracle; (b) ruff `select = ["ALL"]` contradicts the current deliberate +selection in `pyproject.toml` (SIM intentionally omitted because it fights +the commented branch structure) — keep the curated select, or carry the SIM +exclusion into the ALL-based config explicitly. diff --git a/docs/notes/tech-debt-register.md b/docs/notes/tech-debt-register.md index 7b43333a..01c9d3fc 100644 --- a/docs/notes/tech-debt-register.md +++ b/docs/notes/tech-debt-register.md @@ -35,7 +35,15 @@ need is §1. ## 1. The rewrite question (recorded, not scheduled) -**Current position: no rewrite until a trigger fires.** Candidate triggers, in +> **Update (2026-07-03):** the rewrite is now on the record as +> [P-022](../proposals/P-022-rust-core-migration.md) (draft, design-only, +> oracle-gated). The IDE-extension goal supplies the legitimate trigger +> (Gate B of incremental-computation.md); review notes and remaining gaps are +> in [p022-review-notes.md](p022-review-notes.md). The prerequisite below — +> spec or relocate the bridge inference before porting — still stands and is +> P-022's largest gap (no crate owns today's `ownir.py` bridge). + +**Prior position: no rewrite until a trigger fires.** Candidate triggers, in the spirit of the existing trigger table: - measured performance pain at mining scale (profiler numbers on 50+-repo runs, From 8dab13cd23231db219c325c6c4115f607261fbf4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Jul 2026 06:15:15 +0000 Subject: [PATCH 4/4] docs: fix internal wording conflicts flagged by CodeRabbit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01FJSfUpRbRXegrwJNV1rx1X --- docs/notes/p022-review-notes.md | 10 ++++++---- docs/notes/tech-debt-register.md | 15 +++++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/docs/notes/p022-review-notes.md b/docs/notes/p022-review-notes.md index d3443ae1..5c959365 100644 --- a/docs/notes/p022-review-notes.md +++ b/docs/notes/p022-review-notes.md @@ -151,7 +151,9 @@ Sound doctrine (ratchet, baseline, justified allows). Two notes: (a) `import-linter` for `ownlang` is the cheapest immediate win — the Python import DAG is already clean (verified in the July review), so the contract would pass on day one and prevent regressions while Python remains the -oracle; (b) ruff `select = ["ALL"]` contradicts the current deliberate -selection in `pyproject.toml` (SIM intentionally omitted because it fights -the commented branch structure) — keep the curated select, or carry the SIM -exclusion into the ALL-based config explicitly. +oracle; (b) the note *proposes* moving ruff to `select = ["ALL"]` with a +deliberate, commented ignore list, whereas the current `pyproject.toml` uses +a curated select (SIM intentionally omitted because it fights the commented +branch structure) — if the ALL-based variant is adopted, carry the SIM +exclusion into its ignore list explicitly with the same justification +comment; otherwise keep the curated select. diff --git a/docs/notes/tech-debt-register.md b/docs/notes/tech-debt-register.md index 01c9d3fc..d303ba57 100644 --- a/docs/notes/tech-debt-register.md +++ b/docs/notes/tech-debt-register.md @@ -26,12 +26,15 @@ condition (not aesthetics) pulls it off the shelf. - **Rejected** — considered and refused, with the reason on record. -A deliberate framing decision: **no item below is justified by "prepares the -rewrite."** A core rewrite is not on the record anywhere (the only mentions are -the hypothetical trigger rows in `AGENTS.execution-surfaces.md:317,327`), and -the repo's own rule says work needs profiler numbers or real pain, not a -speculative future. Every item here pays rent standalone. What a rewrite *would* -need is §1. +A deliberate framing decision: **every item below justifies itself standalone** +— none needs "prepares the rewrite" as its reason. When this register was first +written, no rewrite was on the record (only the hypothetical trigger rows in +`AGENTS.execution-surfaces.md:317,327`); P-022 has since landed as a draft (see +the §1 update note), but the framing survives: the repo's own rule says work +needs profiler numbers or real pain, not a speculative future, so each item +pays rent today either way. What a rewrite specifically *would* need — and its +hard prerequisites, such as the bridge-inference spec (§3) — is called out +separately in §1, not smuggled into the standalone items. ## 1. The rewrite question (recorded, not scheduled)