Skip to content

Commit 00cd535

Browse files
reidspencerclaude
andcommitted
Fix CI build failures: test syntax, Hugo check, and parser enhancement
- Fix test input files with correct RIDDL statement syntax: - Added 'is' keyword to on clauses where needed - Fixed statement references with proper type keywords - Converted invalid string literals to comments in handler bodies - Add Hugo installation check in HugoPassTest: - Skip Hugo binary execution when not installed (CI environments) - Pass output generation is still validated - Extend pseudoCodeBlock parser to allow comments with ???: - Support { ??? // comment }, { // comment ??? }, etc. - Enables cleaner placeholder code with documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 088fd8c commit 00cd535

7 files changed

Lines changed: 183 additions & 16 deletions

File tree

NOTEBOOK.md

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,88 @@ Reasons:
253253

254254
---
255255

256+
## Future Considerations: Phase 8 Size Optimizations
257+
258+
Analysis performed January 18, 2026 on `large.riddl` (43KB source → 29KB BAST at 67.5%)
259+
260+
### Optimization 1: PathIdentifier Value Interning (Recommended)
261+
262+
**Current encoding** for PathIdentifier (e.g., `TenantId` or `Entity.StateRecord`):
263+
```
264+
Location (2-4 bytes) + Count (1 byte) + N × StringIndex (1-2 bytes each)
265+
```
266+
267+
**Observation**: Identifiers repeat frequently in RIDDL models:
268+
- `UserId`: 65 occurrences
269+
- `TenantId`: 52 occurrences
270+
- `String`: 181 occurrences
271+
- Total: 2,069 identifier references, only 529 unique
272+
- **Repetition rate: 74.4%**
273+
274+
**Proposed optimization**: Create a PathValueTable (similar to StringTable):
275+
```
276+
First occurrence: Location + PathValueIndex + [PathValue in table]
277+
Subsequent: Location + PathValueIndex
278+
```
279+
280+
**Estimated savings**:
281+
- Current path bytes: ~6,200 bytes (in large.riddl)
282+
- With interning: ~4,700 bytes
283+
- **Savings: ~1,500 bytes (5% of total BAST)**
284+
285+
**Implementation complexity**: Medium
286+
- Add PathValueTable to BASTWriter/BASTReader
287+
- Modify `writePathIdentifierInline()` to check table first
288+
- First bit of index indicates: 0 = table lookup, 1 = inline path
289+
290+
### Optimization 2: Location Delta Run-Length Encoding (Potential)
291+
292+
**Observation**: Many consecutive definitions share the same source file and have sequential line numbers.
293+
294+
**Current**: Each location is delta-encoded from the previous.
295+
296+
**Potential enhancement**: Use run-length encoding for sequences of "same file, next line":
297+
- Flag byte indicates: "increment line by 1, same file"
298+
- Saves offset/endOffset bytes for simple sequential definitions
299+
300+
**Estimated savings**: 500-1,000 bytes (2-3%)
301+
**Implementation complexity**: Medium-High
302+
303+
### Optimization 3: SagaStep Structure (Not Recommended)
304+
305+
**Observation**: SagaSteps always have exactly 2 Contents fields (doStatements, undoStatements).
306+
307+
**Current encoding**: Writes count for each Contents field.
308+
309+
**Potential**: Could eliminate counts since always 2 fields.
310+
311+
**Assessment**: **Not worth implementing**
312+
- Typical RIDDL models have very few SagaSteps (0-5)
313+
- Savings: ~5-10 bytes per SagaStep
314+
- Total savings: <50 bytes in typical models
315+
- Adds special-case complexity for minimal gain
316+
317+
### Optimization 4: Reference Kind Consolidation (Potential)
318+
319+
**Observation**: We have 20+ reference types (TypeRef, StateRef, EntityRef, etc.) each with their own tag.
320+
321+
**Current**: Each reference type has a dedicated NODE_* tag.
322+
323+
**Potential**: Could use a single REFERENCE tag + subtype byte for less common references, keeping dedicated tags only for the most frequent (TypeRef, FieldRef, StateRef).
324+
325+
**Assessment**: Marginal benefit, increases code complexity.
326+
327+
### Summary Recommendation
328+
329+
**Phase 8 should focus on PathIdentifier Value Interning**:
330+
- Clearest win with ~5% additional size reduction
331+
- Well-understood implementation pattern (mirrors StringTable)
332+
- Benefits grow with model size and reference density
333+
334+
**Target after Phase 8**: Large files at ~63-64% of source size (from current 67.5%)
335+
336+
---
337+
256338
## Planned: AsciiDoc Generation Module
257339

258340
### Overview
@@ -358,8 +440,75 @@ asciidoc/ # New module (or part of passes)
358440

359441
---
360442

443+
## Known Parser Issues
444+
445+
### PseudoCodeBlock with ??? and Comments
446+
447+
**Status**: ✅ FIXED January 19, 2026
448+
449+
The `pseudoCodeBlock` parser now allows comments before and/or after `???`:
450+
- `{ ??? }`
451+
- `{ ??? // comment }`
452+
- `{ // comment ??? }`
453+
- `{ // c1 ??? // c2 }`
454+
455+
**Fix applied** in `StatementParser.scala`:
456+
```scala
457+
(open ~ comment.rep(0) ~ undefined(Seq.empty[Statements]) ~ comment.rep(0) ~ close).map {
458+
case (before, _, after) => before ++ after
459+
}
460+
```
461+
462+
---
463+
361464
## Session Log
362465

466+
### January 19, 2026 (CI Build Fixes Complete)
467+
468+
**Focus**: Complete CI build fixes from previous session
469+
470+
**Tasks Completed**:
471+
1.**AdaptorWriterTest expected output update** - Updated byte positions and removed string literal from expected output
472+
2.**Hugo CI environment fix** - Added `isHugoInstalled` check to skip Hugo binary execution when not available
473+
3.**Parser fix for `{ ??? // comment }`** - Extended `pseudoCodeBlock` to allow comments before/after `???`
474+
475+
**Files Modified**:
476+
- `commands/jvm/src/test/scala/.../AdaptorWriterTest.scala` - Updated expected positions
477+
- `commands/jvm/src/test/scala/.../HugoPassTest.scala` - Added Hugo installation check
478+
- `language/shared/src/main/scala/.../StatementParser.scala` - Extended pseudoCodeBlock grammar
479+
480+
**Test Results**: All 75 commands tests pass, all 280 language tests pass
481+
482+
---
483+
484+
### January 18, 2026 (CI Build Failures - Initial Investigation)
485+
486+
**Focus**: Investigate and fix CI build failures
487+
488+
**Context**: CI builds started failing after statement syntax changes in the parser. 5 tests were failing due to test input files using old or invalid statement syntax.
489+
490+
**Tests Failing**:
491+
1. `RootOverviewDiagramTest` - context-relationships.riddl parse errors
492+
2. `ContextMapDiagramTest` - same file
493+
3. `ToDoPassListTest` - everything.riddl parse errors
494+
4. `AdaptorWriterTest` - adaptors.riddl parse errors
495+
5. `HugoPassTest` (example sources) - Hugo not installed in CI
496+
497+
**Root Causes Identified**:
498+
1. On clauses missing `is` keyword (e.g., `on command X {``on command X is {`)
499+
2. Statements missing type keywords (e.g., `tell X to Y``tell command X to entity Y`)
500+
3. Bare string literals not valid in handler bodies (pseudo-code strings)
501+
4. Hugo binary not available in CI (separate environmental issue)
502+
503+
**Completed**:
504+
- [x] Fixed `commands/input/hugo/context-relationships.riddl` - added `is` keyword to on clauses, fixed statement syntax
505+
- [x] Fixed `commands/input/everything.riddl` - added `is` keyword, fixed send/set statements
506+
- [x] Fixed `commands/input/adaptors.riddl` - converted string literal to comment
507+
508+
**Discovered Parser Issue**: `{ ??? // comment }` is not allowed by grammar - documented for next session.
509+
510+
---
511+
363512
### January 17, 2026 (Phase 7 Planning)
364513

365514
**Focus**: Plan further BAST size optimizations

commands/input/adaptors.riddl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ domain Adaptors is {
33
command TwoIsDone is { ??? }
44
adaptor FromTwo from context Two is {
55
handler Adaptation is {
6-
on event Adaptors.Two.DidIt from context Two {
7-
"convert Two.DidIt to One.TwoIsDone"
6+
on event Adaptors.Two.DidIt {
7+
// convert Two.DidIt to One.TwoIsDone
88
tell command Adaptors.One.TwoIsDone to context One
99
}
1010
}

commands/input/everything.riddl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@ domain Everything is {
6363
record fields is { field: SomeType }
6464
state someState of Something.fields
6565
handler foo is {
66-
on command ACommand {
67-
"if and(Something arrives, misc()) then"
68-
send event Inebriated to inlet Everything.full.Sink.input
69-
"end"
66+
on command ACommand is {
67+
// if and(Something arrives, misc()) then
68+
send event Inebriated to inlet Everything.full.Sink.input
69+
// end
7070
}
7171
}
7272

@@ -84,8 +84,8 @@ domain Everything is {
8484
record fields is { field: String }
8585
state otherThingState of SomeOtherThing.fields
8686
handler fee is {
87-
on event ItHappened {
88-
set field SomeOtherThing.otherThingState.field to "field ItHappened.field"
87+
on event ItHappened is {
88+
set field SomeOtherThing.otherThingState.field to "field ItHappened.field"
8989
}
9090
}
9191
}

commands/input/hugo/context-relationships.riddl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ domain foo {
99
record AData is { a01: Integer, a02: String }
1010
state AState of record AData
1111
handler AHandler is {
12-
on command ACommand {
12+
on command ACommand is {
1313
set field B.BEntity.BData.b01 to "initial value"
1414
send event A.AEvent1 to inlet B.Flow.bInlet
1515
tell command C.CCommand to entity C.CEntity
@@ -31,8 +31,8 @@ domain foo {
3131
record BData is { b01: Integer, b02: String }
3232
state BState of record BData
3333
handler BHandler is {
34-
on command BCommand {
35-
"do nothing for now"
34+
on command BCommand is {
35+
// do nothing for now
3636
}
3737
}
3838
}
@@ -47,7 +47,7 @@ domain foo {
4747
record CData is { c01: Integer, c02: String }
4848
state BState of record BData
4949
handler CHandler is {
50-
on command CCommand {
50+
on command CCommand is {
5151
send event C.CEvent to inlet A.Sink.aInlet
5252
}
5353
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,24 @@ class HugoPassTest extends RunCommandOnExamplesTest() {
4242
} else fail("wrong command!")
4343
}
4444

45+
private def isHugoInstalled: Boolean = {
46+
import scala.sys.process.*
47+
try {
48+
val result = "which hugo".!
49+
result == 0
50+
} catch {
51+
case _: Exception => false
52+
}
53+
}
54+
4555
def runHugo(outputDir: Path, tmpDir: Path): Assertion = {
4656
import scala.sys.process.*
57+
58+
if !isHugoInstalled then {
59+
info("Hugo not installed - skipping hugo binary execution (pass output was still validated)")
60+
return succeed
61+
}
62+
4763
val output = mutable.ArrayBuffer[String]()
4864
var hadErrorOutput: Boolean = output.nonEmpty
4965

commands/jvm/src/test/scala/com/ossuminc/riddl/commands/hugo/writers/AdaptorWriterTest.scala

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class AdaptorWriterTest extends WriterTest {
5151
|| _Briefly_ | No brief description. |
5252
|| _Authors_ | |
5353
|| _Definition Path_ | Root.Adaptors.One.FromTwo |
54-
|| _View Source Link_ | [commands/input/adaptors.riddl(77->375)]() |
54+
|| _View Source Link_ | [commands/input/adaptors.riddl(77->358)]() |
5555
|| _Used By_ | None |
5656
|| _Uses_ | None |
5757
|
@@ -69,15 +69,14 @@ class AdaptorWriterTest extends WriterTest {
6969
|| :---: | :--- |
7070
|| _Briefly_ | No brief description. |
7171
|| _Definition Path_ | FromTwo.Root.Adaptors.One.Adaptation |
72-
|| _View Source Link_ | [commands/input/adaptors.riddl(121->331)]() |
72+
|| _View Source Link_ | [commands/input/adaptors.riddl(121->314)]() |
7373
|| _Used By_ | None |
7474
|| _Uses_ | None |
7575
|
7676
|## *Description*
7777
|
7878
|#### On event Adaptors.Two.DidIt
7979
|```
80-
|"convert Two.DidIt to One.TwoIsDone"
8180
|tell command Adaptors.One.TwoIsDone to context One
8281
|```
8382
|""".stripMargin

language/shared/src/main/scala/com/ossuminc/riddl/language/parsing/StatementParser.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,10 @@ private[parsing] trait StatementParser {
153153
def pseudoCodeBlock[u: P](set: StatementsSet): P[Seq[Statements]] = {
154154
P(
155155
undefined(Seq.empty[Statements]) |
156-
(open ~ undefined(Seq.empty[Statements]) ~ close) |
156+
// Allow { ??? }, { // comment ??? }, { ??? // comment }, { // c1 ??? // c2 }
157+
(open ~ comment.rep(0) ~ undefined(Seq.empty[Statements]) ~ comment.rep(0) ~ close).map {
158+
case (before, _, after) => before ++ after
159+
} |
157160
(statement(set) | comment)./.rep(1) |
158161
(open ~ (statement(set) | comment)./.rep(1) ~ close)
159162
)

0 commit comments

Comments
 (0)