Complete BAST Phase 3: Fix Repository/Schema tag collision

- Add NODE_SCHEMA tag (35) for Schema nodes to resolve tag collision
- Repository now writes without subtype byte
- Schema uses NODE_SCHEMA with schemaKind subtype
- Split reader into separate readRepositoryNode() and readSchemaNode()
- All 14 tests passing, ShopifyCart 20x faster than parsing

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: reidspencer

bast/KNOWN_ISSUES.md

@@ -1,180 +1,92 @@
 # BAST Known Issues
 
-## Multiple Contents Fields Serialization Bug
+## Current Status (January 14, 2026)
 
-**Status**: CRITICAL - Affects serialization/deserialization of larger files
+**All known issues have been fixed. BAST serialization is working correctly.**
 
-**Discovered**: January 11, 2026
-
-### Symptoms
-
-- Small files (ToDoodles: 12 nodes) serialize/deserialize correctly
-- Larger files (ShopifyCart: 510 nodes) fail during deserialization
-- Error: "Invalid string table index: 1000019 (table size: 649)"
-- The error occurs because the reader/writer become out of sync
+---
 
-### Root Cause
+## Fixed Issues
 
-Some AST nodes have **multiple Contents fields**:
+### 1. Repository/Schema Tag Collision - FIXED (Jan 14, 2026)
 
-1. **`SagaStep`** (will not be removed)
-   - `doStatements: Contents[Statements]`
-   - `undoStatements: Contents[Statements]`
+**Problem**: Both Repository and Schema were using the same `NODE_REPOSITORY` tag (12), with a subtype byte to distinguish them. This caused the reader to misinterpret data when reading Repository nodes.
 
-2. **`IfThenElseStatement`** (may be removed in future revision)
-   - `thens: Contents[Statements]`
-   - `elses: Contents[Statements]`
+**Solution**:
+- Added new `NODE_SCHEMA` tag (35) for Schema nodes
+- Repository now writes: `NODE_REPOSITORY, loc, id, contents, metadata`
+- Schema now writes: `NODE_SCHEMA, schemaKind, loc, id, data, links, indices, metadata`
+- Reader dispatch handles both tags separately
 
-**The Problem**:
+**Files Modified**:
+- `package.scala`: Added `NODE_SCHEMA: Byte = 35`
+- `BASTWriter.scala`: Updated `writeRepository()` to not write subtype, updated `writeSchema()` to use `NODE_SCHEMA`
+- `BASTReader.scala`: Split `readRepositoryOrSchemaNode()` into `readRepositoryNode()` and `readSchemaNode()`
 
-The current serialization architecture has a design flaw:
+### 2. Multiple Contents Fields Serialization - FIXED (Jan 13, 2026)
 
-```scala
-// BASTWriter.scala:658-660
-private def writeSagaStep(ss: SagaStep): Unit = {
-  writer.writeU8(NODE_HANDLER)
-  writeLocation(ss.loc)
-  writeIdentifier(ss.id)
-  writeContents(ss.doStatements)    // Writes count immediately
-  writeContents(ss.undoStatements)  // Writes count immediately
-}
-```
+**Problem**: Nodes with multiple Contents fields (SagaStep, IfThenElseStatement, ForEachStatement) were not serializing correctly.
 
-The `writeContents()` method writes the count:
+**Solution**:
+- Added special cases in `traverse()` override for multi-Contents nodes
+- Each Contents field now writes: count, items (interleaved)
+- Added `NODE_SAGA_STEP` tag (34) to distinguish SagaStep from Handler
 
-```scala
-private def writeContents[T <: RiddlValue](contents: Contents[T]): Unit = {
-  writer.writeVarInt(contents.length)
-  // Note: Individual elements are written by the main process() method during traversal
-}
-```
-
-But the `traverse()` override only processes the main `.contents` field:
-
-```scala
-override protected def traverse(definition: RiddlValue, parents: ParentStack): Unit = {
-  definition match {
-    case branch: Branch[?] with WithMetaData =>
-      process(branch, parents)
-      parents.push(branch)
-      branch.contents.foreach { value => traverse(value, parents) }  // Only .contents!
-      parents.pop()
-      writeMetadataCount(branch.metadata)
-    case _ =>
-      super.traverse(definition, parents)
-  }
-}
-```
+### 3. Statement/Handler Disambiguation - FIXED (Jan 13, 2026)
 
-**Result**:
-- Count for `doStatements` is written
-- Count for `undoStatements` is written
-- Items for `doStatements` are NEVER written (not in `.contents`)
-- Items for `undoStatements` are NEVER written (not in `.contents`)
-- Reader expects items after the count, reads garbage data
-- Deserialization fails with "Invalid string table index"
+**Problem**: Reader couldn't reliably distinguish statements from handlers.
 
-### Affected Files
+**Solution**:
+- Added `STATEMENT_MARKER` (0xFF = 255) byte after NODE_HANDLER for statements
+- Reader checks for marker before interpreting statement type
 
-**Writer**: `bast/shared/src/main/scala/com/ossuminc/riddl/bast/BASTWriter.scala`
-- Lines 655-660: `writeSagaStep()`
-- Lines 913-920: `writeIfThenElseStatement()`
-- Line 1682-1685: `writeContents()`
+### 4. Branch Types without WithMetaData - FIXED (Jan 13, 2026)
 
-**Reader**: `bast/shared/src/main/scala/com/ossuminc/riddl/bast/BASTReader.scala`
-- Lines 1660-1673: `readContentsDeferred()`
+**Problem**: Several types extend `Branch[T]` but NOT `WithMetaData`.
 
-### Impact
+**Affected types**: Handler, OnClauses, Type, UseCase, Group, Output, Input
 
-- ✅ **Works**: Files without SagaStep or IfThenElseStatement (e.g., ToDoodles)
-- ❌ **Fails**: Files containing SagaStep or IfThenElseStatement (e.g., ShopifyCart)
+**Solution**: Added explicit traverse() cases for each of these types.
 
-### Performance Impact
-
-When working correctly:
-- **Small files (12 nodes)**: 1.8x speedup over parsing
-- **Large files (510 nodes)**: 24.6x speedup over parsing (observed before deserialization failed)
+---
 
-### Solution Options
+## Performance Results (All Tests Passing)
 
-#### Option 1: Special-case traverse() for multi-Contents nodes
+| File | Nodes | Parse Time | BAST Read | Speedup | Status |
+|------|-------|------------|-----------|---------|--------|
+| ToDoodles | 12 | ~6ms | ~6ms | ~1x | ✅ Working |
+| ShopifyCart | 667 | ~77ms | ~4ms | **20.1x** | ✅ Working |
 
-Extend the `traverse()` override to detect and handle nodes with multiple Contents fields:
+---
 
-```scala
-override protected def traverse(definition: RiddlValue, parents: ParentStack): Unit = {
-  definition match {
-    case ss: SagaStep =>
-      process(ss, parents)
-      parents.push(ss)
-      ss.doStatements.foreach { value => traverse(value, parents) }
-      ss.undoStatements.foreach { value => traverse(value, parents) }
-      parents.pop()
-      writeMetadataCount(ss.metadata)
+## Technical Notes
 
-    case ite: IfThenElseStatement =>
-      process(ite, parents)
-      parents.push(ite)
-      ite.thens.foreach { value => traverse(value, parents) }
-      ite.elses.foreach { value => traverse(value, parents) }
-      parents.pop()
-      // No metadata for statements
+### Write/Read Order
 
-    case branch: Branch[?] with WithMetaData =>
-      // ... existing code
-  }
-}
 ```
+Writer produces for Branch nodes:
+  tag, nodeSpecificFields..., contentsCount, contentItems..., metadataCount, metadataItems...
 
-**Pros**: Minimal changes, surgical fix
-**Cons**: Must remember to update for any future multi-Contents nodes
-
-#### Option 2: Refactor writeContents() to defer writing
-
-Change `writeContents()` to not write anything immediately. Instead, track pending Contents writes and emit them during traversal.
-
-**Pros**: More robust, handles any future cases
-**Cons**: Significant refactoring, more complex state management
-
-#### Option 3: Two-phase serialization
-
-Separate count-writing from item-writing phases.
-
-**Pros**: Clean separation of concerns
-**Cons**: Requires complete redesign of serialization
-
-### Recommended Fix
-
-**Option 1** (special-case traverse) is recommended because:
-1. Minimal code changes
-2. Easy to understand and verify
-3. Only 2 node types affected (possibly only 1 if IfThenElseStatement is removed)
-4. Fast to implement and test
-
-### Test Cases Needed
-
-After fix:
-1. ✅ Verify ToDoodles still works (regression test)
-2. ✅ Verify ShopifyCart serializes/deserializes correctly
-3. ✅ Create test specifically for SagaStep round-trip
-4. ✅ Create test for IfThenElseStatement round-trip (if not removed)
-5. ✅ Verify performance benchmarks still show speedup
+Reader expects (after tag dispatch):
+  nodeSpecificFields..., contentsCount+Items (via readContentsDeferred), metadataCount+Items
+```
 
-### Related Files
+### Statement Format
 
-- `bast/jvm/src/test/scala/com/ossuminc/riddl/bast/BenchmarkRunner.scala` - Performance benchmark
-- `bast/jvm/src/test/scala/com/ossuminc/riddl/bast/TestRunner.scala` - Round-trip test
-- `bast/shared/src/test/scala/com/ossuminc/riddl/bast/DeepASTComparison.scala` - Deep comparison utility
+```
+Statement: NODE_HANDLER, STATEMENT_MARKER(255), stmtType(0-16), loc, stmtSpecificData...
+Handler:   NODE_HANDLER, loc, id, contentsCount, contentItems..., metadataCount, metadataItems...
+```
 
-### Notes
+### Location Encoding
 
-- The issue does NOT affect metadata serialization (fixed in earlier session)
-- The issue does NOT affect Include node preservation (working correctly)
-- The issue does NOT affect location delta encoding (working correctly)
-- The issue ONLY affects nodes with multiple Contents fields
+```
+First location:  originString, offset, line, col
+Delta location:  sameSourceFlag(0/1), [originString if flag=1], offsetDelta+1M, lineDelta+1K, colDelta+1K
+```
 
 ---
 
-**Last Updated**: January 11, 2026
-**Severity**: High
-**Priority**: Must fix before production use
+**Last Updated**: January 14, 2026
+**Severity**: None (all issues resolved)
+**Priority**: N/A

bast/SESSION_HANDOFF.md

@@ -0,0 +1,151 @@
+# BAST Session Handoff Document
+
+**Date**: January 14, 2026
+**Branch**: `development`
+**Previous Session**: January 13, 2026
+
+---
+
+## Overview
+
+**BAST (Binary AST) serialization is now fully working.** All known issues have been resolved. Both small (ToDoodles) and large (ShopifyCart) files serialize and deserialize correctly with excellent performance characteristics.
+
+---
+
+## Session Progress (January 14, 2026)
+
+### Issue Fixed
+
+**Repository/Schema Tag Collision** - FIXED
+
+**Problem**: Both Repository and Schema were using the same `NODE_REPOSITORY` tag (12), with a subtype byte to distinguish them. This caused the reader to misinterpret data when reading Repository nodes, leading to "Invalid string table index: 1000019" errors.
+
+**Solution**:
+- Added new `NODE_SCHEMA` tag (35) for Schema nodes
+- Repository now writes: `NODE_REPOSITORY, loc, id, contents, metadata` (no subtype)
+- Schema now writes: `NODE_SCHEMA, schemaKind, loc, id, data, links, indices, metadata`
+- Reader dispatch handles both tags separately with dedicated `readRepositoryNode()` and `readSchemaNode()` methods
+
+**Files Modified**:
+- `bast/shared/src/main/scala/com/ossuminc/riddl/bast/package.scala`: Added `NODE_SCHEMA: Byte = 35`
+- `bast/shared/src/main/scala/com/ossuminc/riddl/bast/BASTWriter.scala`:
+  - `writeRepository()`: Removed subtype byte (255)
+  - `writeSchema()`: Changed from `NODE_REPOSITORY` to `NODE_SCHEMA`
+- `bast/shared/src/main/scala/com/ossuminc/riddl/bast/BASTReader.scala`:
+  - Added `NODE_SCHEMA` case to dispatch
+  - Split `readRepositoryOrSchemaNode()` into `readRepositoryNode()` and `readSchemaNode()`
+
+---
+
+## Current Status
+
+### All Tests Passing
+
+| Test Suite | Tests | Status |
+|------------|-------|--------|
+| BASTWriterSpec | 7 | ✅ All passing |
+| BASTRoundTripTest | 3 | ✅ All passing |
+| BASTPerformanceTest | 4 | ✅ All passing |
+| **Total** | **14** | ✅ **All passing** |
+
+### Performance Results
+
+| File | Nodes | Parse Time | BAST Read | Speedup | Status |
+|------|-------|------------|-----------|---------|--------|
+| ToDoodles | 12 | 5.95 ms | 6.16 ms | ~1x | ✅ Working |
+| ShopifyCart | 667 | 77.01 ms | 3.83 ms | **20.1x** | ✅ Working |
+
+### Deep Comparison Results (ToDoodles)
+
+- Total comparisons: 8
+- Successes: 8 (100%)
+- Failures: 0
+- All structural relationships preserved
+
+---
+
+## Previously Fixed Issues (Jan 13, 2026)
+
+1. **Multi-Contents Nodes** - SagaStep, IfThenElseStatement, ForEachStatement
+2. **Statement/Handler Disambiguation** - Added STATEMENT_MARKER (255)
+3. **Branch Types without WithMetaData** - Handler, OnClauses, Type, UseCase, Group, Output, Input
+
+---
+
+## Remaining Work (Future Sessions)
+
+### Phase 4: Import Integration (PENDING)
+- [ ] Update import syntax parser in `CommonParser.scala`
+- [ ] Implement `doImport()` in `ParsingContext.scala`
+- [ ] Implement namespace resolution in path resolution
+- [ ] Add `.bast` file detection to `TopLevelParser`
+- [ ] Implement BAST cache invalidation
+
+### Phase 5: CLI & Testing (PENDING)
+- [ ] Add `riddlc bast-gen` command
+- [ ] Add command-line flags (`--use-bast-cache`, `--bast-dir`)
+- [ ] Additional edge case tests
+- [ ] Cross-platform testing (JS, Native)
+
+### Phase 6: Documentation (PENDING)
+- [ ] Write BAST format specification document
+- [ ] Document serialization/deserialization API
+- [ ] Add ScalaDoc to all public APIs
+- [ ] Create usage examples
+
+---
+
+## How to Test
+
+```bash
+# Unit tests (14 tests)
+sbt "project bast" test
+
+# Deep comparison test (ToDoodles)
+sbt "bast/Test/runMain com.ossuminc.riddl.bast.TestRunner"
+
+# Performance benchmark (ToDoodles + ShopifyCart)
+sbt "bast/Test/runMain com.ossuminc.riddl.bast.BenchmarkRunner"
+```
+
+---
+
+## Key Code Locations
+
+**Core Serialization**:
+- `BASTWriter.scala` - Serialization logic
+- `BASTReader.scala` - Deserialization logic
+- `package.scala` - Node type tags and constants
+
+**Test Utilities**:
+- `BenchmarkRunner.scala` - Performance testing
+- `TestRunner.scala` - Deep comparison testing
+- `DeepASTComparison.scala` - Structural comparison utility
+
+---
+
+## Git Information
+
+**Branch**: `development`
+
+**Files Modified This Session**:
+- `bast/shared/src/main/scala/com/ossuminc/riddl/bast/package.scala`
+- `bast/shared/src/main/scala/com/ossuminc/riddl/bast/BASTWriter.scala`
+- `bast/shared/src/main/scala/com/ossuminc/riddl/bast/BASTReader.scala`
+- `bast/KNOWN_ISSUES.md`
+- `bast/SESSION_HANDOFF.md`
+
+---
+
+## Summary
+
+✅ **Phase 2 Complete**: Core serialization working for all node types
+✅ **Phase 3 Complete**: Deserialization working with full round-trip verification
+✅ **Performance**: 20x speedup for large files confirmed
+⏳ **Next**: Import integration (Phase 4)
+
+The BAST foundation is solid and production-ready. The next major milestone is implementing the `import "file.bast" as namespace` functionality.
+
+---
+
+**End of Handoff Document**