Skip to content

Commit a430842

Browse files
reid-spencerclaude
andcommitted
Fix BAST deserialization bugs for LetStatement and MatchStatement
Two reader-side fixes that caused byte misalignment during BAST deserialization, manifesting as failures on large models like reactive-bbq (4428 nodes): 1. LetStatement: Added missing readU8() to consume the NODE_PATH_IDENTIFIER tag before readPathIdentifierNode() when reading the optional TypeRef added in 1.16.0. The writer uses writePathIdentifier() which includes the tag byte, but the reader was not consuming it — causing 1-byte drift. 2. MatchStatement: Changed reader to match the writer's byte layout where all case headers (loc + pattern + count) are written first, followed by all case items sequentially, then default items. The reader was incorrectly interleaving each case's header with its items. Also removes pre-populated keywords from StringTable (90 entries that were always serialized regardless of usage, ~5-85% size reduction) and adds regression tests for epics and typed let statements. Removes the reactive-bbq pending marker — all 187 riddl-models now pass round-trip testing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f56999e commit a430842

5 files changed

Lines changed: 67 additions & 66 deletions

File tree

commands/jvm/src/test/scala/com/ossuminc/riddl/commands/RiddlModelsRoundTripTest.scala

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,7 @@ class RiddlModelsRoundTripTest
7979
"--show-usage-warnings=false"
8080
)
8181

82-
// Models with pre-existing validation errors in riddl-models
83-
// that are unrelated to BAST round-trip correctness. These
84-
// need fixes in the riddl-models repo, not here.
85-
private val pendingModels: Set[String] = Set(
86-
"hospitality/food-service/reactive-bbq" // validation errors in command handlers (missing event sends)
87-
)
82+
private val pendingModels: Set[String] = Set.empty
8883

8984
"BAST round-trip" should {
9085
val confFiles = discoverModels(riddlModelsDir)

language/shared/src/main/scala/com/ossuminc/riddl/language/bast/BASTReader.scala

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -824,20 +824,42 @@ class BASTReader(bytes: Array[Byte])(using pc: PlatformContext) {
824824

825825
case 11 => // Match
826826
val expression = readLiteralString()
827+
// Writer writes all case headers (loc + pattern + count) first,
828+
// then all case items sequentially, then default items.
827829
val numCases = reader.readVarInt()
828-
val cases = (0 until numCases).map { _ =>
830+
// Phase 1: Read all case headers (loc, pattern, count)
831+
val caseHeaders = (0 until numCases).map { _ =>
829832
val caseLoc = readLocation()
830833
val pattern = readLiteralString()
831-
val statements = readContentsDeferred[Statements]()
832-
MatchCase(caseLoc, pattern, statements)
834+
val count = reader.readVarInt()
835+
(caseLoc, pattern, count)
833836
}.toSeq
834-
val default = readContentsDeferred[Statements]()
835-
MatchStatement(loc, expression, cases, default)
837+
// Read default count
838+
val defaultCount = reader.readVarInt()
839+
// Phase 2: Read case items in order
840+
val cases = caseHeaders.map { case (caseLoc, pattern, count) =>
841+
val buffer = scala.collection.mutable.ArrayBuffer[Statements]()
842+
var i = 0
843+
while i < count do
844+
buffer += readNode().asInstanceOf[Statements]
845+
i += 1
846+
end while
847+
MatchCase(caseLoc, pattern, Contents(buffer.toSeq*))
848+
}
849+
// Phase 3: Read default items
850+
val defaultBuffer = scala.collection.mutable.ArrayBuffer[Statements]()
851+
var di = 0
852+
while di < defaultCount do
853+
defaultBuffer += readNode().asInstanceOf[Statements]
854+
di += 1
855+
end while
856+
MatchStatement(loc, expression, cases, Contents(defaultBuffer.toSeq*))
836857

837858
case 12 => // Let
838859
val identifier = readIdentifier()
839860
val hasTypeRef = reader.readU8() != 0
840861
val optTypeRef = if hasTypeRef then
862+
reader.readU8() // consume NODE_PATH_IDENTIFIER tag
841863
val pid = readPathIdentifierNode()
842864
Some(TypeRef(loc, "type", pid))
843865
else None

language/shared/src/main/scala/com/ossuminc/riddl/language/bast/BASTWriter.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -928,14 +928,14 @@ class BASTWriter(val writer: ByteBufferWriter, val stringTable: StringTable) {
928928
writer.writeU8(11) // Match statement
929929
writeLocation(s.loc)
930930
writeLiteralString(s.expression)
931-
// Write cases
931+
// Write all case headers (loc + pattern + count) first
932932
writer.writeVarInt(s.cases.size)
933933
s.cases.foreach { mc =>
934934
writeLocation(mc.loc)
935935
writeLiteralString(mc.pattern)
936936
writeContents(mc.statements)
937937
}
938-
// Write default
938+
// Write default count
939939
writeContents(s.default)
940940
// NOTE: case and default statement items are written by the Pass's traverse() override
941941
}

language/shared/src/main/scala/com/ossuminc/riddl/language/bast/StringTable.scala

Lines changed: 6 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -11,50 +11,13 @@ import scala.collection.mutable
1111
/** String interning table for BAST serialization
1212
*
1313
* Provides efficient string storage by deduplicating common strings
14-
* and allowing reference by index. Pre-populated with RIDDL keywords
15-
* and predefined types to minimize file size.
14+
* and allowing reference by index. Only strings actually used during
15+
* serialization are stored — no pre-populated keywords.
1616
*/
1717
class StringTable {
1818
private val strings = mutable.ArrayBuffer[String]()
1919
private val stringToIndex = mutable.HashMap[String, Int]()
2020

21-
// Pre-populate with RIDDL keywords for maximum compression
22-
private val keywords = Seq(
23-
// Structure keywords
24-
"domain", "context", "entity", "adaptor", "saga", "streamlet",
25-
"processor", "projector", "repository", "connector", "application",
26-
"epic", "story", "useCase", "function", "type", "author", "user",
27-
"term", "include", "import",
28-
29-
// Type keywords
30-
"String", "Number", "Integer", "Decimal", "Boolean", "Date", "Time",
31-
"DateTime", "TimeStamp", "URL", "UUID", "Current", "Id", "Abstract",
32-
"Pattern", "Enumeration", "Alternation", "Aggregation", "Mapping",
33-
"Graph", "Table", "Replica", "Reference", "Range", "Optional",
34-
"ZeroOrMore", "OneOrMore", "required",
35-
36-
// Common structural keywords
37-
"is", "as", "of", "from", "to", "by", "with", "for", "in", "on",
38-
"at", "and", "or", "not", "if", "then", "else", "briefly", "described",
39-
"explained", "contains", "yields", "returns", "requires", "ensures",
40-
"invariant", "options", "pipe", "source", "sink", "flow", "merge",
41-
"split", "void", "command", "event", "query", "result", "record",
42-
43-
// Handler keywords
44-
"handler", "on", "other", "arbitrary",
45-
46-
// Relationship keywords
47-
"sends", "acquires", "uses",
48-
49-
// Common names
50-
"id", "name", "description", "value", "data", "message", "request",
51-
"response", "error", "status", "code", "timestamp", "userId",
52-
"created", "updated", "deleted"
53-
)
54-
55-
// Initialize with keywords
56-
keywords.foreach(intern)
57-
5821
/** Intern a string and return its index
5922
*
6023
* If the string already exists in the table, returns the existing index.
@@ -119,25 +82,21 @@ class StringTable {
11982
}
12083
}
12184

122-
/** Clear the string table (except keywords) */
85+
/** Clear the string table */
12386
def reset(): Unit = {
12487
strings.clear()
12588
stringToIndex.clear()
126-
keywords.foreach(intern)
12789
}
12890

12991
/** Statistics for debugging and optimization */
13092
def stats: StringTableStats = {
13193
val totalChars = strings.map(_.length).sum
13294
val avgChars = if strings.nonEmpty then totalChars.toDouble / strings.length else 0.0
133-
val duplicatesAvoided = stringToIndex.size - keywords.size
134-
13595
StringTableStats(
13696
totalStrings = strings.length,
13797
uniqueStrings = stringToIndex.size,
13898
totalCharacters = totalChars,
139-
averageLength = avgChars,
140-
duplicatesAvoided = duplicatesAvoided
99+
averageLength = avgChars
141100
)
142101
}
143102
}
@@ -147,13 +106,12 @@ case class StringTableStats(
147106
totalStrings: Int,
148107
uniqueStrings: Int,
149108
totalCharacters: Int,
150-
averageLength: Double,
151-
duplicatesAvoided: Int
109+
averageLength: Double
152110
)
153111

154112
object StringTable {
155113

156-
/** Create a new empty string table with keywords pre-loaded */
114+
/** Create a new empty string table */
157115
def apply(): StringTable = new StringTable()
158116

159117
/** Deserialize a string table from a byte buffer
@@ -163,11 +121,6 @@ object StringTable {
163121
*/
164122
def readFrom(reader: ByteBufferReader): StringTable = {
165123
val table = new StringTable()
166-
167-
// Clear the pre-populated keywords since we'll load from file
168-
table.strings.clear()
169-
table.stringToIndex.clear()
170-
171124
val count = reader.readVarInt()
172125

173126
var i = 0

passes/jvm/src/test/scala/com/ossuminc/riddl/passes/BASTIncrementalTest.scala

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,37 @@ class BASTIncrementalTest extends AnyWordSpec with Matchers {
474474
}""")
475475
}
476476

477+
// Level 19.5: Test epic with user story and interactions
478+
"handle epic with user story and interactions" in {
479+
testRoundTrip("epic-test", """domain TestDomain is {
480+
user Customer is "a person who orders food" with { briefly "customer" }
481+
epic DineIn is {
482+
user Customer wants "to order food" so that "they can eat"
483+
case OrderFood is {
484+
user Customer wants "to select items from the menu" so that "they can build an order"
485+
step from user Customer "selects items from menu" to user Customer
486+
}
487+
} with { briefly "dining in" }
488+
}""")
489+
}
490+
491+
// Level 19.6: Test typed let statement (was the original LetStatement bug)
492+
"handle typed let statement" in {
493+
testRoundTrip("typed-let", """domain TestDomain is {
494+
context Reservations is {
495+
type ReservationBoardEntry is { name: String }
496+
entity ReservationBoard is {
497+
state main of ReservationBoard.ReservationBoardEntry
498+
handler Input is {
499+
on event TestDomain.Reservations.ReservationBoard.SomeEvent {
500+
let entry: ReservationBoardEntry = "new ReservationBoardEntry from event"
501+
}
502+
}
503+
}
504+
}
505+
}""")
506+
}
507+
477508
// Level 20: Test with rbbq-like structure
478509
"handle rbbq-like kitchen context" in {
479510
testRoundTrip("rbbq-kitchen", """domain ReactiveBBQ is {

0 commit comments

Comments
 (0)