feat(pattern): griff-pattern — the std-only structural algebra (S16 Phase 1)

[PhysShell]

Summary

The first implementation slice of S16 (ADR-0029): a new workspace member griff-pattern holding the pure structural pattern algebra — Kernel, bounded fractalize, path-addressed pruning (swang-prune-hash-v1), row_major/snake traversals, and ActivitySequence. Everything lowers-to-music lives elsewhere; this crate knows no MIDI, no griff-core, no serde, no floats.

Commit sequence (TDD per AGENTS.md)

1. Red (540dde9) — 21 failing tests plus the API skeleton, all behavioral bodies unimplemented!() (trivial accessors like NodePath::as_slice were plumbing, not algorithm). The nine swang-prune-hash-v1 golden vectors were computed by an independent BigInteger implementation of spec §1.8 before the crate compiled.
2. Red fix (e2f7ba6) — the first red's depth arithmetic disagreed with itself (81 cells at depth 2 in one test, depth 1 in another) and mis-addressed a pruned block. With depth 0 = the kernel (spec §1.7), a depth-d grid carries d+1 kernel factors per axis.
3. Green (ab108bd) — the minimal implementation; all 21 tests pass, including the bit-for-bit match against the independent golden vectors.

What the tests pin

• kernel laws: rectangular X/., ragged/invalid/empty are typed errors naming the offending cell (spec §1.6);
• expansion laws: active parent → kernel replica, empty parent → entirely empty block, depth 0 = kernel (§1.7);
• budgets: required, no library defaults, breach fires before allocation carrying the offending NodePath (§1.4);
• pruning: the exact normative algorithm of §1.8 — mix64/DOMAIN/GAMMA fold, constant threshold floor(bps·2^64/10000), edge laws at 0 and 10000 bps, pruned parent → silent 9×9 subtree two levels down, generation-seed independence by construction;
• traversals: the spec's §1.9 worked example verbatim (row_major 0 2 3 4 7 8 vs snake 0 2 4 5 7 8 from one kernel), and linearize preserves all 81 cells because silence is a slot (§1.10);
• two property tests over random kernels: grid dimensions, cell preservation, the empty-parent law.

Notes for review

• cargo clippy --all-targets -- -D warnings is clean; the single arithmetic allow carries its reason.
• cargo test --workspace is green except the pre-existing missing_file_golden, which compares the OS strerror text and fails on any non-English Windows locale regardless of branch — Linux CI is unaffected. Worth a follow-up normalization, tracked separately.
• thin is deliberately absent: spec §1.10 fixes its type contract but not its selection rule; it gets its own red once the spec section lands (flagged during the design review).

Refs #108, ADR-0029, S16 Phase 1.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NkqJUU6d1sW1RAfvyHrqVM

Summary by CodeRabbit

• New Features
  • Added structural pattern generation from 2D occupancy grids.
  • Added configurable expansion depth, cell budgets, and deterministic density-based pruning.
  • Added row-major and snake traversal options for converting patterns into activity sequences.
  • Added helpers for inspecting active cells and identifying sequence onsets.
  • Added validation with clear errors for malformed patterns, invalid density, and exceeded limits.
• Tests
  • Added comprehensive coverage for pattern parsing, expansion, pruning, traversal, and budget enforcement.

Review round 2 (comment 4972142215)

• Blocker 1 (90d6828 red → 69f5b69 green): two-pass kernel validation — the rectangle is judged in a pass that stores nothing, so a ragged-and-foreign row fails as RaggedKernel and nothing allocates for a broken shape. Codex's thread resolved.
• Blocker 2 (same red/green): MaxDepthExceeded now carries the NodePath every budget breach owes; the up-front check names the root.
• Contract blocker (d823a7a, docs): ADR-0029 §2 and the Phase 1 primitive list now name the real types (Kernel/Expansion/NodePath/PruneSpec) and record that no materialized PatternTree exists — the coordinate digits are the tree (decisions log). thin moves out of Phase 1 and the specified v0.1 roster: its type contract stays in spec §1.10, its selection rule is deliberately unspecified; spec acceptance test 6 now guards the artifact's bar geometry instead.
• Minor: prune-hash doc says evaluation-order-independent.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a workspace-integrated griff-pattern crate implementing kernel parsing, bounded fractal expansion, deterministic pruning, traversal linearization, structured errors, and comprehensive unit and property tests.

Changes

Pattern crate

Layer / File(s)	Summary
Crate setup and regression fixtures Cargo.toml, pattern/Cargo.toml, pattern/proptest-regressions/lib.txt	Registers the new workspace member, defines the std-only crate manifest and workspace linting, and adds saved proptest regression seeds.
Pattern data model and traversal pattern/src/lib.rs	Defines Kernel, expansion configuration and data types, ActivitySequence, traversal behavior, and structured error handling.
Expansion, pruning, and validation pattern/src/lib.rs	Implements budget-checked expansion, path-addressed hash pruning, exact density thresholds, and tests for parsing, expansion, pruning, traversal, hashing, and invariants.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Kernel
  participant fractalize
  participant Expander
  participant linearize
  participant ActivitySequence
  Kernel->>fractalize: kernel, depth, prune spec, budget
  fractalize->>Expander: expand candidate cells
  Expander->>Expander: evaluate activity and prune paths
  fractalize-->>linearize: Expansion
  linearize->>ActivitySequence: traversal-ordered cells

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding the std-only griff-pattern crate for S16 Phase 1.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/s16-pattern-core

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab108bdd23

ℹ️ 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".

[PhysShell]

Architecture/code review — changes required before merge

The hard part is sound: I independently compared the coordinate-digit evaluator with a recursive substitution model over small rectangular kernels, depths 0–2, and several seed/density combinations. The resulting grids match, and the hash/threshold implementation agrees with the normative §1.8 vectors. The red → red-fix → green sequence is also materially honest.

There are two code blockers and one phase-contract blocker.

1. Validate the complete kernel before allocating

Codex has already opened the correct inline thread. Kernel::from_rows calls Vec::with_capacity before validating later row widths, then pushes cells before discovering a ragged row. This violates the explicit S16 law that ragged kernels are rejected before allocation and can change a typed error into an allocation failure.

Please use a two-pass implementation:

1. validate non-empty shape, all row lengths, and allowed characters without building cells;
2. only after successful validation, allocate and populate.

Add a red test pinning shape-validation precedence for an input that is both ragged and contains a foreign character on the ragged row. The result should be RaggedKernel, proving the shape pass completes before cell decoding. Then land the green implementation separately.

2. Every structural budget breach must carry NodePath

PatternError::MaxCellsExceeded carries a path, but MaxDepthExceeded does not. The normative spec and Phase-1 acceptance say a budget breach carries the offending NodePath, covering both max_depth and max_cells.

Add a red test requiring:

PatternError::MaxDepthExceeded {
    path: NodePath::default(),
    depth: 3,
    max_depth: 2,
}

Then add the field in a separate green commit. The root is the correct offending path for the up-front global depth check.

3. Do not declare Phase 1 closed or start Phase 2 with the current contract drift

The implementation is a good Phase-1 core slice, but the accepted stage contract currently also requires thin, and the documented primitive roster names Pattern, PatternTree, and FractalSpec. This PR intentionally ships none of those exact contracts: it uses Kernel, a flattened Expansion, and direct depth/prune/budget arguments. That may be the better API, but documents and implementation cannot quietly diverge.

Before moving to Phase 2, choose one honest path:

• define the missing thin selection semantics and land it red→green, while explaining how Expansion satisfies or replaces PatternTree/FractalSpec; or
• amend the still-Proposed ADR/S16/spec contract now: defer thin until its selector is specified, and replace the planned type roster with the actual v0.1 representation (Kernel, Expansion, NodePath, PruneSpec, ExpansionBudget). Record why the direct coordinate evaluator avoids a materialized tree.

I prefer the second option. Inventing thin merely to satisfy a stale bullet would be governance by checkbox, one of humanity’s more persistent recreational errors.

Minor cleanup

Change prune_hash_v1’s documentation from “order-independent” to “evaluation-order-independent”; path order is intentionally significant.

CI is green on ab108bdd23d03dc23c6e4b326c6cba6bdf00e0ce. After the two red/green fixes and the contract alignment, this is mergeable. The pre-existing localized missing_file_golden is unrelated and not a blocker here.

[coderabbitai]

🧹 Nitpick comments (1)

pattern/src/lib.rs (1)

109-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate is_active/active_count logic between Kernel and Expansion.

Kernel::is_active/Kernel::active_count (Lines 109-125) and Expansion::is_active/Expansion::active_count (Lines 219-235) are byte-for-byte identical implementations operating on the same {width, cells: Vec<bool>} shape. Consider extracting a small shared internal grid type (or a private trait with default methods given width()/cells() accessors) that both Kernel and Expansion delegate to, so the bounds-check logic (including the important col >= width guard that prevents flat-index wraparound into the next row) only exists once and can't silently drift between the two copies in a future edit.

♻️ Sketch of a shared grid helper

struct Grid {
    width: usize,
    height: usize,
    cells: Vec<bool>,
}

impl Grid {
    fn is_active(&self, row: usize, col: usize) -> bool {
        if col >= self.width {
            return false;
        }
        cell_index(self.width, row, col)
            .and_then(|index| self.cells.get(index))
            .copied()
            .unwrap_or(false)
    }

    fn active_count(&self) -> usize {
        self.cells.iter().filter(|&&cell| cell).count()
    }
}

Kernel and Expansion can then hold a Grid field (or newtype-wrap it) and forward their public is_active/active_count to it.

Also applies to: 219-235

🤖 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 `@pattern/src/lib.rs` around lines 109 - 125, Extract the duplicated grid
behavior from Kernel and Expansion into one private shared grid abstraction,
such as Grid, containing the width and cells data and implementing is_active and
active_count. Update both types’ public methods to delegate to this shared
implementation while preserving the col >= width guard and existing out-of-range
behavior; avoid maintaining separate copies of the bounds-check and counting
logic.

🤖 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.

Nitpick comments:
In `@pattern/src/lib.rs`:
- Around line 109-125: Extract the duplicated grid behavior from Kernel and
Expansion into one private shared grid abstraction, such as Grid, containing the
width and cells data and implementing is_active and active_count. Update both
types’ public methods to delegate to this shared implementation while preserving
the col >= width guard and existing out-of-range behavior; avoid maintaining
separate copies of the bounds-check and counting logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b3e7ed5-4362-4931-8e8d-a9e28659d166

📥 Commits

Reviewing files that changed from the base of the PR and between 8e7c2ac and ab108bd.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

• Cargo.toml
• pattern/Cargo.toml
• pattern/proptest-regressions/lib.txt
• pattern/src/lib.rs