jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(ci): add GDScript->Rust cross-encoder fixtures (#475) #31

Closed
jpmschweitzer wants to merge 0 commits from ci into main
pull from: ci
merge into: :main
Conversation 2 Commits - Files Changed -
jpmschweitzer commented 2026-02-18 02:21:32 +01:00
Owner
Copy Link
Copy Source

Summary

  • Add make fixtures-client target: GDScript encoder generates 20 .msgpack fixtures, Rust decoder verifies them in one step (#475)
  • Closes the bidirectional cross-encoder compatibility loop (D-030 Layer 1): Rust encodes/GDScript decodes (#474, done) + GDScript encodes/Rust decodes (this PR)
  • 20 fixtures cover unit variants, data variants, struct variants, batch encoding, and 10 boundary tick values exercising the int_16/int_32 encoding asymmetry

Details

  • client/tests/gen_client_fixtures.gd — standalone SceneTree script, loads Messagepack encoder directly (bypasses class_name registry issues with -s scripts)
  • server/tests/serialization.rs — gdscript_generated_fixtures_deserialize test with value assertions on key fixtures (boundary ticks, action variants)
  • docs/DEVOPS.md — documented cross-encoder fixtures workflow
  • Fixtures committed to server/tests/fixtures/gdscript/ (same pattern as Rust→GDScript fixtures)

Test plan

  • make fixtures-client generates 20 fixtures and Rust verification passes
  • cargo test --test serialization — all 34 tests pass (including new test)
  • Boundary tick values 256, 32767, 65536, 2147483647 verify int_16/int_32 asymmetry acceptance
## Summary - Add `make fixtures-client` target: GDScript encoder generates 20 `.msgpack` fixtures, Rust decoder verifies them in one step (#475) - Closes the bidirectional cross-encoder compatibility loop (D-030 Layer 1): Rust encodes/GDScript decodes (#474, done) + GDScript encodes/Rust decodes (this PR) - 20 fixtures cover unit variants, data variants, struct variants, batch encoding, and 10 boundary tick values exercising the int_16/int_32 encoding asymmetry ## Details - `client/tests/gen_client_fixtures.gd` — standalone SceneTree script, loads Messagepack encoder directly (bypasses class_name registry issues with `-s` scripts) - `server/tests/serialization.rs` — `gdscript_generated_fixtures_deserialize` test with value assertions on key fixtures (boundary ticks, action variants) - `docs/DEVOPS.md` — documented cross-encoder fixtures workflow - Fixtures committed to `server/tests/fixtures/gdscript/` (same pattern as Rust→GDScript fixtures) ## Test plan - [x] `make fixtures-client` generates 20 fixtures and Rust verification passes - [x] `cargo test --test serialization` — all 34 tests pass (including new test) - [x] Boundary tick values 256, 32767, 65536, 2147483647 verify int_16/int_32 asymmetry acceptance
jpmschweitzer added 1 commit 2026-02-18 02:21:32 +01:00
feat(ci): add make fixtures-client target for GDScript->Rust cross-encoder validation (#475) da63aa580e 
Closes the bidirectional protocol compatibility loop (D-030 Layer 1):
- GDScript fixture generator (20 fixtures: inputs, boundary ticks, batch)
- Rust decoder test verifying all GDScript-encoded fixtures deserialize
- Makefile target with generation + verification in one step

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-18 09:02:28 +01:00
Author
Owner
Copy Link
Copy Source

Review: ci -> main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Cross-encoder fixture infrastructure is well-structured. Boundary tick analysis is exactly the defensive work needed.

# File Severity Issue
1 gen_client_fixtures.gd critical Encode failures silently swallowed — empty .msgpack written, quit(0) exits success
2 serialization.rs critical count == 0 silently skips — empty fixture dir = false pass
3 gen_client_fixtures.gd warning Missing action variants: MoveSouth, MoveEast, MoveWest, Unpause
4 Makefile warning make pre-pr does not call fixtures-client — GDScript fixtures can go stale
5 gen_client_fixtures.gd suggestion No file.flush() before close() in headless mode
6 serialization.rs suggestion Relative fixture path inconsistent — add CWD comment
7 docs/DEVOPS.md suggestion No documented recovery procedure for failures

Tyre (Architecture): APPROVE

Correctly closes D-030 Layer 1 cross-encoder loop. Encoding asymmetry tests are architecturally the most valuable piece.

# File Severity Issue
1 serialization.rs warning Silent skip on missing fixtures masks CI gaps
2 gen_client_fixtures.gd warning Output path traverses outside client/ — fragile assumption
3 fixtures/gdscript/ suggestion No staleness check for GDScript fixtures in pre-pr
4 gen_client_fixtures.gd suggestion Missing cardinal direction variants for completeness

Verdict: CHANGES REQUESTED

## Review: ci -> main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Cross-encoder fixture infrastructure is well-structured. Boundary tick analysis is exactly the defensive work needed. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | gen_client_fixtures.gd | critical | Encode failures silently swallowed — empty .msgpack written, quit(0) exits success | | 2 | serialization.rs | critical | `count == 0` silently skips — empty fixture dir = false pass | | 3 | gen_client_fixtures.gd | warning | Missing action variants: MoveSouth, MoveEast, MoveWest, Unpause | | 4 | Makefile | warning | `make pre-pr` does not call `fixtures-client` — GDScript fixtures can go stale | | 5 | gen_client_fixtures.gd | suggestion | No `file.flush()` before `close()` in headless mode | | 6 | serialization.rs | suggestion | Relative fixture path inconsistent — add CWD comment | | 7 | docs/DEVOPS.md | suggestion | No documented recovery procedure for failures | ### Tyre (Architecture): APPROVE Correctly closes D-030 Layer 1 cross-encoder loop. Encoding asymmetry tests are architecturally the most valuable piece. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | serialization.rs | warning | Silent skip on missing fixtures masks CI gaps | | 2 | gen_client_fixtures.gd | warning | Output path traverses outside client/ — fragile assumption | | 3 | fixtures/gdscript/ | suggestion | No staleness check for GDScript fixtures in pre-pr | | 4 | gen_client_fixtures.gd | suggestion | Missing cardinal direction variants for completeness | ### Verdict: CHANGES REQUESTED
jpmschweitzer added 2 commits 2026-02-18 09:54:08 +01:00
fix(ci): address PR #31 review — harden cross-encoder fixture pipeline 3ccac5432b 
- Fail on encode errors instead of silently writing empty .msgpack files
- Fail test on missing/empty fixture dir instead of silent skip
- Add all missing action variants (MoveSouth, MoveEast, MoveWest,
  Unpause, ToggleStanceDown, WalkAway) to GDScript fixture generator
- Add GDScript fixture staleness check to make pre-pr
- Validate repo root detection before writing outside client/
- Add file.flush() before close in headless mode
- Document fixture failure recovery in DEVOPS.md

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog f8db996eb9 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-18 10:22:54 +01:00
Author
Owner
Copy Link
Copy Source

Re-Review: ci -> main (type: code) — Post-Fix

Hoshe (Code Quality): APPROVE

All 7 original items verified as FIXED — including both criticals. Pipeline now fails loudly on encode errors and empty fixture dirs.

# Original Item Status
1 Silent encode failures (critical) FIXED — error tracking + quit(1)
2 Empty fixture dir false pass (critical) FIXED — hard assert!
3 Missing action variants (warning) FIXED — all 8 directions + Unpause
4 pre-pr missing fixture check (warning) FIXED — bidirectional staleness
5 No file.flush() (suggestion) FIXED
6 CWD comment (suggestion) FIXED
7 DEVOPS recovery docs (suggestion) FIXED

Tyre (Architecture): APPROVE

All 4 original items verified as FIXED. Notes SetTickRate fixture is missing but pre-existing, not a regression.

# Original Item Status
1 Silent skip masks CI gaps (warning) FIXED — hard assert
2 Output path traversal (warning) FIXED — validation gate
3 No GDScript staleness in pre-pr (suggestion) FIXED
4 Missing cardinal directions (suggestion) FIXED — all 8 + extras

Verdict: APPROVED

## Re-Review: ci -> main (type: code) — Post-Fix ### Hoshe (Code Quality): APPROVE All 7 original items verified as FIXED — including both criticals. Pipeline now fails loudly on encode errors and empty fixture dirs. | # | Original Item | Status | |---|--------------|--------| | 1 | Silent encode failures (critical) | FIXED — error tracking + `quit(1)` | | 2 | Empty fixture dir false pass (critical) | FIXED — hard `assert!` | | 3 | Missing action variants (warning) | FIXED — all 8 directions + Unpause | | 4 | `pre-pr` missing fixture check (warning) | FIXED — bidirectional staleness | | 5 | No `file.flush()` (suggestion) | FIXED | | 6 | CWD comment (suggestion) | FIXED | | 7 | DEVOPS recovery docs (suggestion) | FIXED | ### Tyre (Architecture): APPROVE All 4 original items verified as FIXED. Notes `SetTickRate` fixture is missing but pre-existing, not a regression. | # | Original Item | Status | |---|--------------|--------| | 1 | Silent skip masks CI gaps (warning) | FIXED — hard assert | | 2 | Output path traversal (warning) | FIXED — validation gate | | 3 | No GDScript staleness in pre-pr (suggestion) | FIXED | | 4 | Missing cardinal directions (suggestion) | FIXED — all 8 + extras | ### Verdict: APPROVED
jpmschweitzer closed this pull request 2026-02-18 10:24:55 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#31