Preserve checked constants and exact array indexes

.agents/skills/peeper-code-review/SKILL.md

@@ -0,0 +1,83 @@
+---
+name: peeper-code-review
+description: Review Peeper compiler changes for correctness and simple, maintainable code. Use for this repository's uncommitted changes, branches or PRs, especially requests to avoid overengineering, simplify code, reduce redundant variables/helpers/state, or find clearer equivalent solutions. Report concrete findings and smallest safe remedies; review does not imply permission to apply fixes.
+---
+
+# Peeper code review
+
+Optimize for fewer concepts to maintain and code a new contributor can read cold.
+Deletion and direct reuse beat new layers. Clear, equivalent code beats clever brevity.
+
+## Pin scope
+
+- Read repo `RULES.md`, `AGENTS.md`, `go-style.md` and relevant active local plan.
+  Consult specs/architecture for changed behavior, then verify claims against source.
+  Reference existing rules instead of creating another policy or precedence hierarchy.
+- Inspect status and resolve requested baseline. For **current uncommitted changes**,
+  default to `git diff HEAD -- <scope>` for final worktree state; inspect staged diff
+  separately when index differs from worktree. Read untracked source/tests/fixtures.
+  An empty committed range is not an empty worktree. For branch/PR review, use requested
+  base/merge-base and included commits.
+- Identify task work and pre-existing user edits. Cover requested source/documentation;
+  do not treat unrelated edits as scope creep or overwrite them. Exclude generated
+  artifacts and operational journals/local plans from implementation findings.
+- Review source read-only unless user requested fixes. Follow existing agent edit gates
+  and delivery authorization when implementation is requested.
+
+## Standards and simplicity
+
+Trace changed symbols to real callers and existing canonical owners before suggesting
+abstraction, extraction or replacement. Review tests and docs as well as production.
+
+For each changed concept, ask:
+
+1. Does it need to exist? Recommend deletion of speculative state and stale layers.
+2. Can stdlib, existing dependency or current owner do it directly? Prefer reuse.
+3. Can deletion, inlining or a plain guard/loop/switch make the same behavior clearer?
+4. Does a variable name a useful domain fact, prevent repeated/effectful evaluation or
+   clarify an invariant? Keep it. If it only renames a clear expression, suggest removal.
+5. Is a helper/type/cache/boundary justified under `RULES.md`? Keep real reuse and
+   ownership/invariant boundaries; eliminate pass-through or single-use simple layers.
+6. Is identical logic repeated? Use its canonical owner. Similar-looking operations
+   with different semantic or validation responsibilities should remain distinct.
+
+Recommend the smallest equivalent change. State exactly what disappears and why the
+replacement is easier to understand. Fewer lines alone do not justify nested expressions,
+bit tricks, generic frameworks, hidden mutation or erasing validation. Do not manufacture
+cleanup findings merely to hit a quota, rename good domain variables or chase unrelated code.
+
+## Spec and correctness
+
+- Trace changed producers and consumers through actual compiler phases. Check source
+  acceptance, lowering/materialization, backend output and runtime meaning together.
+- Before deleting behavior, inventory validation/diagnostics and locations, mutation,
+  caching, normalization/fallbacks, locking, ownership/lifetimes and phase invariants.
+- Numeric simplification must retain intermediate truncation, signed extension and
+  float rounding. Bounds must preserve exact values and trap-before-access ordering.
+- Verify semantic/nominal identity, defining-owner publication and incomplete-type
+  construction where affected. Do not repeat decisions already owned by checked evidence.
+- Check new symbols/fields have current production consumers. Tests should prove
+  behavior or invariants, not require production APIs/state only for observation.
+
+## Evidence and validation
+
+Use the smallest focused source/test probe needed to confirm a suspected problem.
+Inspect baseline behavior when distinguishing a regression from an existing gap.
+Keep temporary probes outside tracked source; add no production instrumentation.
+Respect active plan's validation environment. Do not rerun unchanged expensive suites
+without new edits, failures or unresolved evidence. Clearly attribute inherited results.
+For language behavior, require appropriate `x_test/` coverage and bundled compiler;
+for ABI/backend changes, distinguish target object validation from native execution.
+Verify docs/skill edits with links, discovery/ignore rules and diff checks as relevant.
+
+## Report
+
+Findings first, under **Standards** and **Spec**. For each finding give:
+
+- severity and whether correctness defect, documented breach or optional cleanup;
+- file and precise lines, actual consequence and source/rule/test evidence;
+- smallest safe remedy, what it removes, and behavior that must be preserved.
+
+Distinguish introduced problems from existing gaps. Report zero findings when no
+worthwhile improvement is justified. End with validation actually run, remaining
+limitations and short Rules check. Keep review concise; no generic refactor wishlist.

.gitignore

@@ -26,6 +26,13 @@ graphify-out
 *.patch
 clean.md
 .*/
+!.agents/
+.agents/*
+!.agents/skills/
+.agents/skills/*
+!.agents/skills/peeper-code-review/
+.agents/skills/peeper-code-review/*
+!.agents/skills/peeper-code-review/SKILL.md
 !.gitignore
 !.github/
 !.github/workflows/

AGENTS.md

@@ -6,6 +6,10 @@ This file defines how agents must work in this repository.
 
 When documents conflict, do not let a file's self-declared precedence settle technical correctness. Identify the conflict, inspect evidence, and ask for a decision when required. Human-facing engineering rules belong in `RULES.md`; agent-only gates, local plans, GitHub automation, and response style belong here.
 
+For code-review requests, read [Peeper code review](.agents/skills/peeper-code-review/SKILL.md).
+It captures the review workflow and user's simplicity preferences; use `RULES.md`
+for durable engineering requirements.
+
 ---
 
 ## 1) Required pre-change check

Code-tour.md

@@ -42,10 +42,11 @@ invoked as an external tool. Peeper does not link anything by hand.
 | `cmd/` | CLI commands: `build`, `run`, `check`, `dump`, `doctor` |
 | `internal/driver` | Thin wrapper that builds a `CompilerContext` and compiles one file |
 | `internal/pipeline` | Module loading and the phase ladder that drives everything |
-| `internal/project` | `Module`, `CompilerContext` — where every phase artifact is stored |
+| `internal/module` | `Module` and its phase artifacts |
+| `internal/project` | `CompilerContext`, module registry, imports, and fingerprints |
 | `internal/frontend` | `token`, `lexer`, `parser`, `ast` |
 | `internal/semantics` | Collector, binder, resolver, const eval, typechecker, flow facts, effects, definite init, ownership, usage, plus the artifact packages |
-| `internal/ir` | `cfg`, `hir`, `mir`, and the shared `ir` node/type model |
+| `internal/ir` | `thir`, `cfg`, `exprlower`, `mir`, and the shared `ir` node/type model |
 | `internal/backend/llvm` | MIR → LLVM IR text |
 | `internal/toolchain` | Finds `clang` and the sysroot; builds its command lines |
 | `internal/diagnostics` | Errors, warnings, source rendering, phase attribution |
@@ -86,7 +87,7 @@ Before any phase runs, the loader walks imports and builds a dependency graph.
 
 ```go
 // internal/pipeline/loader.go (simplified)
-func (l *moduleLoader) Load(entry *project.Module) error {
+func (l *moduleLoader) Load(entry *module.Module) error {
     l.enqueue(entry)
     for len(l.queue) > 0 {
         module := l.pop()
@@ -129,14 +130,14 @@ This is the spine of the compiler. Each module carries a `Phase`, and
 ```mermaid
 flowchart TD
     Setup --> Load --> Parsed --> Collected --> Bound --> Resolved
-    Resolved --> Typechecked --> CFG --> FlowTyped
-    FlowTyped --> Effects --> DefiniteInit --> Ownership --> Usage
+    Resolved --> Typechecked --> CFG --> Analyzed
+    Analyzed --> Usage
     Usage --> MIR --> Backend --> Finalize
 ```
 
 ```go
 // internal/pipeline/pipeline.go (heavily simplified)
-func advanceModulePhase(ctx *project.CompilerContext, module *project.Module) bool {
+func advanceModulePhase(ctx *project.CompilerContext, module *module.Module) bool {
     if module.Phase < phase.Collected { collector.Collect(ctx, module); module.Phase = phase.Collected; return true }
     if module.Phase < phase.Bound     { binder.Bind(ctx, module);       module.Phase = phase.Bound;     return true }
     if module.Phase < phase.Resolved  { resolver.Resolve(ctx, module);  module.Phase = phase.Resolved;  return true }
@@ -156,32 +157,34 @@ scheduler enforces that by advancing everyone one rung at a time.
 | `Collected` | top-level symbols, method sets | `SymbolIndex`, `ModuleScope` |
 | `Bound` | operator/interface bindings | `SymbolIndex` |
 | `Resolved` | every identifier → symbol | `SymbolIndex` occurrence lookup |
-| `Typechecked` | types, typing decisions, finalized constants, typed source IR | `Typechecking`, `Constants`, `THIR` |
+| `Typechecked` | types, typing decisions, finalized constants, typed source IR | `SymbolIndex`, `THIR`, `SemanticExportFingerprint` |
 | `CFG` | blocks, sites, edges | `CFG` |
-| `FlowTyped` | path-sensitive narrowing | `Analysis` |
-| `Effects` | ordered evaluation/storage events | transient during `Analysis.Run` |
-| `DefiniteInit` | *diagnostics only* | — |
-| `Ownership` | drop plan | `Ownership` |
-| `Usage` | *warnings only* | — |
+| `Analyzed` | flow, effect, definite-init, and ownership evidence | `Analysis` |
+| `Usage` | usage diagnostics at the project barrier | diagnostics; no durable module artifact |
 
 | `MIR` | flat, block-structured IR | `MIR` |
 | `Backend` | LLVM IR text | `LLVMIR` |
+| `Finalize` | cross-module runtime-symbol validation | project phase |
 
-**Phase artifacts are the central design idea.** Each fact is produced by exactly one
-phase, stored in exactly one place, and read by later phases. No phase reaches backwards
-to recompute something an earlier one already decided.
+**Phase artifacts are the central design idea.** Each durable artifact is produced by one
+owning phase and read by later phases. Flow, effects, definite initialization, and
+ownership evidence are published together in `Analysis`; `Usage` is a project barrier
+for diagnostics rather than another durable module artifact.
 
-`Module.resetToPhase` clears artifacts in phase order, so incremental rebuilds cannot
-leave stale evidence behind:
+`Module.ResetToPhase` clears downstream artifacts in phase order, so incremental rebuilds
+cannot leave stale evidence behind:
 
 ```go
-// internal/project/modules.go (simplified)
-func (m *Module) resetToPhase(retained phase.Phase) {
-    if retained < phase.Typechecked { m.Typechecking = nil; m.TypedASTNodes = nil }
-    if retained < phase.CFG         { m.CFG = nil }
-    if retained < phase.Effects     { m.Effects = nil }
-    if retained < phase.Ownership   { m.Ownership = nil }
-    // ...
+// internal/module/module.go (simplified)
+func (m *Module) ResetToPhase(retained phase.Phase) {
+    m.Phase = retained
+    if retained <= phase.Parsed { m.ModuleScope = nil; m.SymbolIndex = nil }
+    if retained < phase.Collected { m.typeDeclarations = nil }
+    if retained < phase.Typechecked { m.THIR = nil; m.SemanticExportFingerprint = "" }
+    if retained < phase.CFG { m.CFG = nil }
+    if retained < phase.Analyzed { m.Analysis = nil }
+    if retained < phase.MIR { m.MIR = nil }
+    if retained < phase.Backend { m.LLVMIR = "" }
 }
 ```
 
@@ -253,15 +256,13 @@ module.SymbolIndex.SetScope(block, scope)
 depend on, so nothing has to re-derive them:
 
 ```go
-// internal/semantics/typecheckresult/result.go (excerpt)
-result.RecordExprType(expr.ID(), typ)
-result.RecordImplicitConversion(expr.ID(), conversion)
-result.RecordForIteration(loop.ID(), iteration)
-
-// Later phases ask semantic questions; the backing indexes stay private.
-typ := result.ExprType(expr.ID())
-conversion, ok := result.ImplicitConversion(expr.ID())
-iteration, ok := result.ForIteration(loop.ID())
+// internal/semantics/typechecker/evidence.go (private during Check)
+c.evidence.RecordExprType(expr.ID(), typ)
+c.evidence.RecordImplicitConversion(expr.ID(), conversion)
+c.evidence.RecordForIteration(loop.ID(), iteration)
+
+// internal/semantics/typechecker/thir_build.go materializes those facts on THIR;
+// later phases query THIR instead of re-deriving source meaning.
 ```
 
 ---
@@ -406,15 +407,14 @@ They currently share **evidence, not solver machinery**. Each has distinct latti
 initialized on *every* path, so the join is intersection.
 
 ```go
-// internal/semantics/definiteinit (simplified)
-func apply(current state, op effect.Op) {
-    switch op := op.(type) {
-    case effect.Define:  if op.Initialized { current[op.Symbol.ID] = struct{}{} }
-    case effect.Write:   current[op.Place.Root.ID] = struct{}{}
-    case effect.Use:     // a read changes nothing
-    case effect.Borrow:  // nor does taking a reference
-    default:             panic("unhandled effect")   // a new op fails loudly here
+// internal/semantics/analysis/definite_init.go (simplified)
+func transfer(ops []effectOp, in initState) initState {
+    out := copyInitState(in)
+    visitor := &initializationVisitor{current: out, shouldApplyState: true}
+    for _, op := range ops {
+        visitEffect(op, visitor)
     }
+    return out
 }
 ```
 
@@ -423,22 +423,12 @@ It contains **no AST switch at all**; source identity comes from `source.NodeID`
 **Ownership** tracks moves, loans and liveness, then writes the drop plan:
 
 ```go
-// internal/semantics/analysis/effects.go (simplified)
-for _, op := range a.effects[site.ID] {
-    switch op := op.(type) {
-    case effect.CallBegin:
-        calls = append(calls, callFrame{                 // remember where this call's loans start
-            call: callFor(op), temporary: len(loans.temporary), reserved: len(loans.reserved),
-        })
-    case effect.CallEnd:
-        frame := pop(&calls)
-        a.activateCallReservations(frame.call, frame.reserved, loans)  // fire at call start
-        loans.temporary = loans.temporary[:frame.temporary]            // argument temporaries die
-        loans.reserved = loans.reserved[:frame.reserved]
-    case effect.Use:
-        a.applyUse(op, st, loans)                        // move state + storage access
-    case effect.Borrow:
-        a.applyBorrow(op, st, loans, calls)              // access + loan
+// internal/semantics/analysis/ownership_effects.go (simplified)
+func (a *analyzer) applyEffects(node *site, st ownershipState, loans *loanContext) {
+    ops := a.effects[node.cfgSite.ID]
+    visitor := &ownershipEffectVisitor{a: a, node: node, st: st, loans: loans}
+    for _, op := range ops {
+        visitEffect(op, visitor)
     }
 }
 ```
@@ -447,12 +437,12 @@ The result is the `CleanupPlan` — the **single source of drop obligations** ov
 values:
 
 ```go
-// internal/semantics/ownershipresult/result.go
-type CleanupPlan struct {
-    AfterScope     map[cfg.SiteID][]symbols.SymbolID  // scope exit
-    BeforeReturn   map[source.NodeID][]symbols.SymbolID // after the value is computed
-    BeforeAssign   map[source.NodeID]struct{}           // replacing a value drops the old
-    DiscardedValue map[source.NodeID]struct{}           // a temporary nobody owns
+// internal/semantics/analysis/cleanup.go
+type cleanupPlan struct {
+    AfterScope   map[cfg.SiteID][]symbols.SymbolID
+    BeforeReturn map[source.NodeID][]symbols.SymbolID
+    BeforeAssign map[source.NodeID]struct{}
+    DiscardedValue map[source.NodeID]struct{}
     // …
 }
 ```
@@ -543,18 +533,18 @@ The compiler is built so that *forgetting* something fails loudly.
 
 | Guard | Where | Catches |
 | --- | --- | --- |
-| AST traversal tests | `frontend/ast` tests + source fixtures | broken child traversal behavior |
+| AST traversal tests | `internal/frontend/ast` tests + source fixtures | broken child traversal behavior |
 | Sealed semantic type contract | Go type system | missing child/ownership behavior on a new semantic type |
 | THIR validation + required operation methods | `internal/ir/thir` | malformed evidence or missing consumer support |
-| MIR/backend rejecting dispatch | `internal/ir/mir`, `backend/llvm` | unsupported lowered nodes fail loudly |
+| MIR/backend rejecting dispatch | `internal/ir/mir`, `internal/backend/llvm` | unsupported lowered nodes fail loudly |
 | `cfg.Validate` | `internal/ir/cfg` | malformed topology |
-| `effect.Validate` | `internal/semantics/effect` | operations with no symbol, unbalanced calls |
-| `ownershipresult.Validate` | `internal/semantics/ownershipresult` | evidence that contradicts published types |
+| `Analysis` validation | `internal/semantics/analysis` | malformed effects or cleanup evidence |
 
 A contract failure reads like this:
 
 ```
-publishStmt makes no decision about ast.YieldStmt; add a case or declare why the kind is inert
+THIR operation contract has no implementation for a new node kind; add its case or
+declare why the kind is inert
 ```
 
 At true closed extension points, every omission must be handled or deliberately
@@ -573,7 +563,8 @@ For a new syntax construct, in order:
 4. **resolver** — scopes and bindings.
 5. **typechecker** — the type rule, and *publish* whatever later phases will need.
 6. **CFG** — only if the control-flow shape is genuinely new.
-7. **effects** — one case in `publishStmt`/`value` saying what it does to bindings.
+7. **analysis** — implement required `AnalyzeFlow` and `BuildEffects` methods when
+   the construct changes flow or storage behavior.
 8. **THIR/MIR** — only if no existing lowering shape can represent it.
 
 Steps 1–6 are unavoidable: where a name lives and what types are legal *is* the feature.
@@ -607,7 +598,7 @@ at each true extension point.
 | --- | --- |
 | Why is my program rejected? | `internal/diagnostics/codes.go`, then the phase that owns the code |
 | How does phase ordering work? | `internal/pipeline/pipeline.go`, `advanceModulePhase` |
-| What does the typechecker publish? | `internal/semantics/typecheckresult/result.go` |
-| Why is a value moved/dropped here? | `internal/semantics/effect`, then `internal/semantics/ownership` |
+| What does the typechecker publish? | `internal/semantics/typechecker/evidence.go`, then `internal/ir/thir` |
+| Why is a value moved/dropped here? | `internal/semantics/analysis`, then `internal/ir/mir` |
 | What does the backend emit for X? | `internal/backend/llvm/emitter.go` |
 | How do I add a node kind safely? | `docs/compiler-architecture.md`, then the owning syntax boundary |

docs/architecture/ir.md

@@ -130,7 +130,21 @@ rebuilt:
 - expression trees lower through `exprlower`, then existing shared-IR-to-MIR logic.
 
 External functions and body-backed functions share THIR signature lowering.
-Published module constants become MIR static data before function lowering.
+Published module constants become MIR static data before function lowering, using
+the declared semantic type's runtime representation. Lowering checks published
+value ABI identity against that type, including nominal enum identity. Missing
+values produce a compile-time evaluation diagnostic; mismatched published types
+produce an invalid-evidence diagnostic. Neither failure emits partial MIR.
+Discard (`_`) module bindings remain source-checked but emit no addressable static.
+
+LLVM fixed-array index lowering retains unsigned 64-bit lengths and compares
+constant indexes exactly; backend bounds diagnostics retain index source location.
+After exact logical bounds validation, physical offsets use the target pointer-index
+width for GEP; narrow unsigned values must not become negative through LLVM's
+implicit sign extension or wider-index truncation. Checked constant-index
+types/values remain intact in THIR/MIR.
+Runtime indexes continue through the shared bounds guard before element access;
+signed indexes also reject negative values independently of unsigned length.
 
 ## Validation boundaries