Skip to content

Commit f9b6cd8

Browse files
mbreiserclaude
mbreiser
and
claude
committed
fix: YAML import bug + protocol roundtrip test infrastructure
Fix simpleYAMLParse() comment-handling bug that caused multi-condition YAML imports to show only 1 trial (comments/blank lines between conditions were breaking child-block detection). Add CI-ready roundtrip testing: - test-protocol-roundtrip.js: 49 checks across 5 suites - generate-roundtrip-protocol.js: generates V1 YAML + manifest for MATLAB-side validation - validate-protocol-roundtrip.yml: GitHub Actions workflow - protocol-roundtrip-testing.md: architecture + usage docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1b6cd3a commit f9b6cd8

5 files changed

Lines changed: 1349 additions & 18 deletions

File tree

‎.github/workflows/validate-protocol-roundtrip.yml‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
name: Validate Protocol YAML Roundtrip
2+
3+
on:
4+
push:
5+
paths:
6+
- 'experiment_designer.html'
7+
- 'tests/test-protocol-roundtrip.js'
8+
- 'tests/generate-roundtrip-protocol.js'
9+
- '.github/workflows/validate-protocol-roundtrip.yml'
10+
pull_request:
11+
paths:
12+
- 'experiment_designer.html'
13+
- 'tests/test-protocol-roundtrip.js'
14+
- 'tests/generate-roundtrip-protocol.js'
15+
workflow_dispatch:
16+
17+
jobs:
18+
validate:
19+
runs-on: ubuntu-latest
20+
21+
steps:
22+
- name: Checkout repository
23+
uses: actions/checkout@v4
24+
25+
- name: Set up Node.js
26+
uses: actions/setup-node@v4
27+
with:
28+
node-version: '24'
29+
30+
- name: Run protocol roundtrip tests
31+
run: node tests/test-protocol-roundtrip.js
32+
33+
- name: Generate roundtrip YAML (smoke test)
34+
run: node tests/generate-roundtrip-protocol.js --outdir /tmp/roundtrip-output
35+
36+
- name: Verify generated files exist
37+
run: |
38+
echo "Checking generated YAML files..."
39+
ls -la /tmp/roundtrip-output/
40+
test -f /tmp/roundtrip-output/test_protocol_v1.yaml || { echo "FAIL: test_protocol_v1.yaml not generated"; exit 1; }
41+
test -f /tmp/roundtrip-output/test_protocol_manifest.json || { echo "FAIL: manifest not generated"; exit 1; }
42+
echo "All generated files present."
43+
44+
- name: Summary
45+
if: always()
46+
run: |
47+
echo "## Protocol YAML Roundtrip Validation" >> $GITHUB_STEP_SUMMARY
48+
echo "" >> $GITHUB_STEP_SUMMARY
49+
echo "### Test Suites:" >> $GITHUB_STEP_SUMMARY
50+
echo "- V1 Generate → Parse Roundtrip (33 checks)" >> $GITHUB_STEP_SUMMARY
51+
echo "- Comment-Handling Regression (5 checks)" >> $GITHUB_STEP_SUMMARY
52+
echo "- Excluded Phases (3 checks)" >> $GITHUB_STEP_SUMMARY
53+
echo "- Numeric Type Preservation (6 checks)" >> $GITHUB_STEP_SUMMARY
54+
echo "- Inline Comments (2 checks)" >> $GITHUB_STEP_SUMMARY
55+
echo "" >> $GITHUB_STEP_SUMMARY
56+
echo "### Generated Files:" >> $GITHUB_STEP_SUMMARY
57+
echo "- test_protocol_v1.yaml (for MATLAB validation)" >> $GITHUB_STEP_SUMMARY
58+
echo "- test_protocol_manifest.json (field expectations)" >> $GITHUB_STEP_SUMMARY
59+
echo "" >> $GITHUB_STEP_SUMMARY
60+
echo "> **Note**: Full MATLAB-side validation requires local MATLAB." >> $GITHUB_STEP_SUMMARY
61+
echo "> Run \`validate_web_protocol_roundtrip.m\` in maDisplayTools for end-to-end verification." >> $GITHUB_STEP_SUMMARY

‎docs/protocol-roundtrip-testing.md‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
# Protocol YAML Roundtrip Testing
2+
3+
Validates that YAML protocol files generated by the **web experiment designer** (`experiment_designer.html`) are fully compatible with the **MATLAB experiment runner** (`ProtocolParser` + `ProtocolRunner`).
4+
5+
## Architecture
6+
7+
```
8+
Web (JavaScript) MATLAB
9+
───────────────── ──────────────────────
10+
experiment_designer.html ProtocolParser.m
11+
└─ generateYAML() └─ parse()
12+
│ │
13+
▼ ▼
14+
V1 Protocol YAML ───────────────> Parsed protocol struct
15+
│ │
16+
│ ▼
17+
│ ProtocolRunner constructor
18+
│ (validates structure)
19+
▼
20+
simpleYAMLParse()
21+
(web-side parse-back)
22+
```
23+
24+
### Test Coverage
25+
26+
| Layer | Test | Runs in CI? |
27+
|-------|------|-------------|
28+
| Web YAML generation + parse | `test-protocol-roundtrip.js` | Yes (Node.js) |
29+
| Web YAML → file generation | `generate-roundtrip-protocol.js` | Yes (Node.js) |
30+
| MATLAB parse + validate | `validate_web_protocol_roundtrip.m` | No (needs MATLAB license) |
31+
| MATLAB ProtocolRunner construction | `validate_web_protocol_roundtrip.m` | No (needs MATLAB license) |
32+
33+
## Running Tests
34+
35+
### Web-side CI test (no dependencies)
36+
37+
```bash
38+
cd webDisplayTools
39+
node tests/test-protocol-roundtrip.js
40+
```
41+
42+
Tests 49 checks across 5 suites:
43+
1. **V1 Generate → Parse Roundtrip** — full field-by-field comparison
44+
2. **Comment-Handling Regression** — comments between conditions (bug fix validation)
45+
3. **Excluded Phases** — pretrial/intertrial/posttrial with `include: false`
46+
4. **Numeric Type Preservation** — ints, floats, booleans parse correctly
47+
5. **Inline Comments** — comments don't interfere with parsing
48+
49+
Exit code 0 = all passed, 1 = failures.
50+
51+
### Re-generate test YAML for MATLAB
52+
53+
```bash
54+
cd webDisplayTools
55+
node tests/generate-roundtrip-protocol.js --outdir ../maDisplayTools/tests/web_generated_patterns
56+
```
57+
58+
Generates:
59+
- `test_protocol_v1.yaml` — V1 protocol with 3 conditions
60+
- `test_protocol_manifest.json` — expected values for MATLAB validation
61+
62+
The generator also runs 27 self-verification checks.
63+
64+
### MATLAB validation (requires MATLAB license)
65+
66+
```matlab
67+
cd maDisplayTools
68+
results = validate_web_protocol_roundtrip('v1');
69+
```
70+
71+
Tests 28 checks:
72+
1. ProtocolParser can parse the YAML
73+
2. Version = 1
74+
3. Experiment info (name)
75+
4. Arena config (generation, rows, cols)
76+
5. Experiment structure (repetitions)
77+
6. Number of conditions = 3
78+
7. Per-condition: id, pattern, duration, frame_rate, mode (x3 conditions)
79+
8. Phase includes (pretrial, intertrial, posttrial)
80+
9. Pretrial command count = 4
81+
10. ProtocolRunner construction (validates full pipeline)
82+
83+
V2 test available but stubbed: `validate_web_protocol_roundtrip('v2')` returns early until V2 format is finalized.
84+
85+
## Files
86+
87+
### Web side (`webDisplayTools/`)
88+
89+
| File | Purpose |
90+
|------|---------|
91+
| `tests/test-protocol-roundtrip.js` | CI-ready regression test (49 checks) |
92+
| `tests/generate-roundtrip-protocol.js` | YAML + manifest generator for MATLAB validation |
93+
94+
### MATLAB side (`maDisplayTools/`)
95+
96+
| File | Purpose |
97+
|------|---------|
98+
| `tests/validate_web_protocol_roundtrip.m` | MATLAB validation (28 checks, parameterized V1/V2) |
99+
| `tests/web_generated_patterns/test_protocol_v1.yaml` | Generated test YAML |
100+
| `tests/web_generated_patterns/test_protocol_manifest.json` | Expected values manifest |
101+
102+
## What to Update When Changing YAML Format
103+
104+
### Adding new fields to V1
105+
106+
1. Update `generateV1Protocol()` in both:
107+
- `generate-roundtrip-protocol.js` (generates files)
108+
- `test-protocol-roundtrip.js` (in-memory test)
109+
2. Add expected values to the `testProtocol` definition
110+
3. Add check assertions
111+
4. Re-generate: `node tests/generate-roundtrip-protocol.js --outdir ...`
112+
5. Update `validate_web_protocol_roundtrip.m` with new field checks
113+
6. Update manifest expected values count
114+
115+
### Adding V2 support
116+
117+
1. Add `generateV2Protocol()` to generator and test scripts
118+
2. Remove the `skip` return from `validate_web_protocol_roundtrip('v2')`
119+
3. Add V2-specific checks (rig config, plugins, etc.)
120+
4. Generate V2 test YAML: update generator's `--version 2` support
121+
122+
### Changing `experiment_designer.html` YAML output
123+
124+
After modifying `generateYAML()` or `simpleYAMLParse()`:
125+
126+
1. Run `node tests/test-protocol-roundtrip.js` — catches web-side regressions
127+
2. Re-generate: `node tests/generate-roundtrip-protocol.js --outdir ...`
128+
3. Run MATLAB: `validate_web_protocol_roundtrip('v1')` — catches cross-tool regressions
129+
130+
## Known Constraints
131+
132+
- **ProtocolRunner dry run** tries to initialize hardware (PanelsController). The MATLAB test validates construction only (parse + validate), not `run()`.
133+
- **`ProtocolParser.m` and `CommandExecutor.m`** are Lisa's code — flagged as DO NOT MODIFY. Any incompatibilities found should be discussed before changing.
134+
- **simpleYAMLParse** is a minimal YAML parser. It handles the subset of YAML used by the experiment designer but is not a full YAML 1.2 parser.

‎experiment_designer.html‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,19 +1828,20 @@ <h1>Experiment Designer</h1>
18281828

18291829
if (valRaw === '' || valRaw === undefined) {
18301830
i++;
1831+
// Skip blank lines and comments to find actual child content
1832+
while (i < lines.length && (lines[i].trim() === '' || lines[i].trim().startsWith('#'))) {
1833+
i++;
1834+
}
18311835
if (i < lines.length) {
18321836
var nextLine = lines[i];
1833-
if (nextLine && nextLine.trim() !== '' && !nextLine.trim().startsWith('#')) {
1834-
var nextIndent = getIndent(nextLine);
1835-
if (nextIndent > indent) {
1836-
if (nextLine.trim().startsWith('- ')) {
1837-
obj[key] = parseList(nextIndent);
1838-
} else {
1839-
obj[key] = parseBlock(nextIndent);
1840-
}
1837+
var nextIndent = getIndent(nextLine);
1838+
if (nextIndent > indent) {
1839+
if (nextLine.trim().startsWith('- ')) {
1840+
obj[key] = parseList(nextIndent);
1841+
} else {
1842+
obj[key] = parseBlock(nextIndent);
18411843
}
18421844
}
1843-
}
18441845
} else {
18451846
obj[key] = parseValue(valRaw);
18461847
i++;
@@ -1880,19 +1881,20 @@ <h1>Experiment Designer</h1>
18801881
var subVal = (subKv[2] || '').trim();
18811882
if (subVal === '') {
18821883
i++;
1884+
// Skip blank lines and comments to find actual child content
1885+
while (i < lines.length && (lines[i].trim() === '' || lines[i].trim().startsWith('#'))) {
1886+
i++;
1887+
}
18831888
if (i < lines.length) {
18841889
var nLine = lines[i];
1885-
if (nLine && nLine.trim() !== '') {
1886-
var nIndent = getIndent(nLine);
1887-
if (nIndent > subIndent) {
1888-
if (nLine.trim().startsWith('- ')) {
1889-
item[subKey] = parseList(nIndent);
1890-
} else {
1891-
item[subKey] = parseBlock(nIndent);
1892-
}
1890+
var nIndent = getIndent(nLine);
1891+
if (nIndent > subIndent) {
1892+
if (nLine.trim().startsWith('- ')) {
1893+
item[subKey] = parseList(nIndent);
1894+
} else {
1895+
item[subKey] = parseBlock(nIndent);
18931896
}
18941897
}
1895-
}
18961898
} else {
18971899
item[subKey] = parseValue(subVal);
18981900
i++;

0 commit comments

Comments
 (0)