Skip to content

Commit 269c7f3

Browse files
skeptomaiclaude
andcommitted
docs: Major cleanup of ONGOING_TASKS.md and architectural knowledge preservation
CLEANUP ACHIEVEMENT: Reduced ONGOING_TASKS.md from 1549 lines to 128 lines (92% reduction) ONGOING_TASKS.md CHANGES: - Removed all completed/resolved items (infinite loop, climb tree, string-to-dictionary fixes, etc.) - Eliminated extensive debugging logs and outdated investigation details - Focused on active work: Localization Architecture with clear phase breakdown - Added current system status showing all major bugs resolved - Streamlined to actionable items only ARCHITECTURE.md ENHANCEMENTS: - Added comprehensive "Compiler Architecture Lessons Learned" section - Preserved critical architectural insights from resolved bugs: * Circular Reference Prevention patterns (object tree integrity) * HashMap→IndexMap Determinism Impact (function ordering effects) * String Parameter Passing constraints (two-phase processing) * Z-Machine Opcode Form Instability (form-sensitive conflicts) * Builtin Pseudo-Method Detection (property vs compiler builtin distinction) * Expression Control Flow Optimization (jump elimination techniques) * Development Methodology Insights (debugging and testing strategies) DOCUMENTATION ORGANIZATION: - Clean separation: Active tasks (ONGOING_TASKS.md) vs Architectural knowledge (ARCHITECTURE.md) - Historical context preserved in proper documentation structure - No valuable technical insights lost during cleanup IMMEDIATE NEXT STEPS: - Phase 1: AST Extensions for localization system ready to implement - Phase 5: Builtin Function Message Integration pending - All major bugs resolved, system in stable production state 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 7c0c37b commit 269c7f3

29 files changed

+767
-1485
lines changed

ONGOING_TASKS.md

Lines changed: 49 additions & 1471 deletions
Large diffs are not rendered by default.

docs/ARCHITECTURE.md

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,6 +1288,122 @@ Main Loop (ID 9000)
12881288

12891289
This modular architecture provides a robust, maintainable foundation for a complete Z-Machine interpreter with excellent game compatibility, clean code organization, and clear separation of concerns across functional domains.
12901290

1291+
## Compiler Architecture Lessons Learned
1292+
1293+
### Critical Bug Pattern: Circular Reference Prevention
1294+
1295+
**Date**: November 12-13, 2025
1296+
**Issue**: Container iteration infinite loops caused by circular sibling references
1297+
**Root Cause**: Phase 2 object tree processing created objects with `sibling=self` instead of `sibling=0`
1298+
1299+
**Key Insight**: Object containment systems require explicit circular reference detection:
1300+
```rust
1301+
// WRONG: Can create circular references
1302+
self.write_to_object_space(obj_offset + 5, parent_child)?;
1303+
1304+
// CORRECT: Check for existing correct placement first
1305+
if parent_child == obj_num {
1306+
// Object already correctly positioned, no updates needed
1307+
} else if parent_child == 0 {
1308+
// Parent has no children - make this object the first child
1309+
self.write_to_object_space(parent_offset + 6, obj_num)?;
1310+
} else {
1311+
// Insert at beginning of sibling chain with circularity prevention
1312+
self.write_to_object_space(obj_offset + 5, parent_child)?;
1313+
self.write_to_object_space(parent_offset + 6, obj_num)?;
1314+
}
1315+
```
1316+
1317+
**Prevention**: Always verify object tree integrity during Phase 2 processing and prevent self-referencing sibling chains.
1318+
1319+
### HashMap→IndexMap Determinism Impact
1320+
1321+
**Date**: November 12, 2025
1322+
**Issue**: Deterministic builds required HashMap→IndexMap migration, which unexpectedly affected function ordering
1323+
**Impact**: Identical source code produced different runtime behavior due to function address changes
1324+
1325+
**Key Insight**: Function ordering changes can affect Z-Machine branch offset calculations:
1326+
- Function addresses changed due to deterministic iteration order
1327+
- Branch offset calculations depend on absolute addresses in bytecode
1328+
- Identical IR logic can produce different runtime behavior when addresses shift
1329+
1330+
**Architectural Decision**: Function ordering stability is critical for reproducible builds. IndexMap provides deterministic ordering at the cost of requiring careful verification of address-dependent code.
1331+
1332+
### String Parameter Passing Architectural Constraint
1333+
1334+
**Date**: November 6, 2025
1335+
**Issue**: Z-Machine has no native string parameter support, but games need direction strings
1336+
**Challenge**: Original design constraint prohibited string literals as function parameters
1337+
1338+
**Solution Architecture**:
1339+
1. **Two-Phase Processing**: Collect string info during operand building, process after instruction emission
1340+
2. **Layout-Based Addressing**: Use `InstructionLayout.operand_location` for precise address calculation
1341+
3. **Type Differentiation**: Direction strings (dictionary addresses) vs display strings (packed addresses)
1342+
1343+
**Key Lesson**: When instruction emission affects addressing, always use layout information rather than manual calculation.
1344+
1345+
**Critical Bug Pattern**:
1346+
```rust
1347+
// WRONG: Manual calculation (off-by-one errors)
1348+
let operand_location = self.code_address + 1 + operands.len() * 2;
1349+
1350+
// CORRECT: Layout-based (exact addressing)
1351+
let layout = self.emit_instruction_typed(...);
1352+
let operand_location = layout.operand_location.unwrap() + (index * 2);
1353+
```
1354+
1355+
### Z-Machine Opcode Form Instability
1356+
1357+
**Date**: October 2025
1358+
**Issue**: Same opcode number means different instructions across forms (Long/Variable)
1359+
**Example**: Opcode 0x0D = `store` (Long form) vs `output_stream` (Variable form)
1360+
1361+
**Root Cause**: Form determination in `emit_instruction` automatically switches forms based on operand constraints, but doesn't account for semantic differences.
1362+
1363+
**Critical Pattern**:
1364+
```rust
1365+
// When LargeConstant > 255, form switches from Long to Variable
1366+
// but opcode 0x0D changes meaning from 'store' to 'output_stream'
1367+
emit_instruction(0x0D, [Variable(dest), LargeConstant(300)]) // BROKEN
1368+
```
1369+
1370+
**Prevention Strategy**: Add compile-time validation for form-sensitive opcodes to catch semantic conflicts before code generation.
1371+
1372+
### Builtin Pseudo-Method Detection
1373+
1374+
**Date**: October 9, 2025
1375+
**Issue**: Method calls with property checks incorrectly skipped builtin pseudo-methods
1376+
**Root Cause**: IR generator applied property check pattern to compiler-provided functions
1377+
1378+
**Key Insight**: Distinguish between property-based methods and compiler builtins:
1379+
```rust
1380+
// Property-based methods (check if object has custom implementation)
1381+
if property_exists { call_custom() } else { call_default() }
1382+
1383+
// Builtin pseudo-methods (direct compiler functions)
1384+
call_builtin() // No property check needed
1385+
```
1386+
1387+
**Detection Strategy**: Use pattern matching on method names to bypass property lookup for known compiler builtins (`get_exit`, `empty`, `none`).
1388+
1389+
### Complex Expression Control Flow Optimization
1390+
1391+
**Achievement**: 66% reduction in redundant jumps (32 → 11) through IR pattern optimization
1392+
**Technique**: Eliminate `Jump { end_label }` when else branch is empty
1393+
1394+
**Remaining Pattern**: LoadImmediate instructions don't emit Z-code, causing offset-2 jumps that convert to NOPs
1395+
**Decision**: Accept current state - remaining NOPs are harmless and rare
1396+
1397+
### Development Methodology Insights
1398+
1399+
**Debugging Lesson**: When debugging execution failures, instrument actual execution path first, not binary analysis
1400+
**Effective**: Trace instruction decode → execution flow to find missing execution
1401+
**Ineffective**: Examining bytecode with hex dumps and guessing intent
1402+
1403+
**Architecture Documentation**: Always document root causes and prevention strategies for systematic bugs - they tend to recur in similar forms.
1404+
1405+
**Testing Strategy**: Verify semantic correctness, not just syntactic validity. Complex bugs often produce valid bytecode that executes incorrectly.
1406+
12911407
## IR Generation for Builtin Pseudo-Methods (October 9, 2025)
12921408

12931409
### The Problem

killed-troll.sav

548 Bytes
Binary file not shown.

src/grue_compiler/ir.rs

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3247,23 +3247,24 @@ impl IrGenerator {
32473247
source_info
32483248
);
32493249

3250-
let container_object = self.variable_sources.get(&iterable_temp).and_then(
3251-
|source| {
3252-
if let VariableSource::ObjectTreeRoot(container_id) = source {
3253-
log::error!(
3250+
let container_object =
3251+
self.variable_sources
3252+
.get(&iterable_temp)
3253+
.and_then(|source| {
3254+
if let VariableSource::ObjectTreeRoot(container_id) = source {
3255+
log::error!(
32543256
"🔍 FOR_LOOP_DEBUG: Found ObjectTreeRoot source! Container ID = {}",
32553257
container_id
32563258
);
3257-
Some(*container_id)
3258-
} else {
3259-
log::error!(
3260-
"🔍 FOR_LOOP_DEBUG: Found non-ObjectTreeRoot source: {:?}",
3261-
source
3262-
);
3263-
None
3264-
}
3265-
},
3266-
);
3259+
Some(*container_id)
3260+
} else {
3261+
log::error!(
3262+
"🔍 FOR_LOOP_DEBUG: Found non-ObjectTreeRoot source: {:?}",
3263+
source
3264+
);
3265+
None
3266+
}
3267+
});
32673268

32683269
if let Some(container_id) = container_object {
32693270
log::error!(
8.94 KB
Binary file not shown.
8.94 KB
Binary file not shown.

tests/mini_zork_test.z3

8.94 KB
Binary file not shown.
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# Mini Zork Comprehensive Test Protocol Report
2+
3+
**Generated:** Wed Nov 12 16:19:45 PST 2025
4+
**Project:** infocom
5+
**Git Commit:** 7c0c37b
6+
7+
## Test Configuration
8+
9+
- **Debug Game:**
10+
- **Release Game:**
11+
- **Game Commands:** 11 commands (open mailbox → quit)
12+
- **Expected Final Score:** 7
13+
- **Expected Final Moves:** 4
14+
15+
## Test Results
16+
17+
### udebug interpreter debug game
18+
19+
**Status:** ✅ PASSED
20+
- **Final Score:** 7
21+
- **Final Moves:** 4
22+
- **Success Indicators:** 4/4
23+
24+
### udebug interpreter release game
25+
26+
**Status:** ✅ PASSED
27+
- **Final Score:** 7
28+
- **Final Moves:** 4
29+
- **Success Indicators:** 4/4
30+
31+
### urelease interpreter debug game
32+
33+
**Status:** ✅ PASSED
34+
- **Final Score:** 7
35+
- **Final Moves:** 4
36+
- **Success Indicators:** 4/4
37+
38+
### urelease interpreter release game
39+
40+
**Status:** ✅ PASSED
41+
- **Final Score:** 7
42+
- **Final Moves:** 4
43+
- **Success Indicators:** 4/4
44+
45+
## Overall Results
46+
47+
**Tests Passed:** 4/4
48+
**Overall Status:** ✅ ALL TESTS PASSED
49+
50+
🎉 **COMPREHENSIVE TEST PROTOCOL SUCCESSFUL**
51+
52+
All build combinations (debug/release compiler × debug/release interpreter) completed
53+
successfully with expected gameplay outcomes. The codebase is functioning correctly
54+
across all tested configurations.
55+
56+
## Files Generated
57+
58+
- **Raw Outputs:** `*_output.txt` files with complete game session logs
59+
- **Clean Outputs:** `*_clean.txt` files with ANSI codes stripped
60+
- **Test Summaries:** `*_summary.txt` files with key metrics
61+
- **Game Files:** Debug and release compiled game files
62+
63+
All files are located in: `/Users/cb/Projects/infocom-testing-old/infocom/tests/protocol_results_20251112_161909`
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/Users/cb/Projects/infocom-testing-old/infocom/tests/mini_zork_debug_20251112_161909.z3
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
DORK I: The Last Great Empire
2+
Copyright (c) 2025 Grue Games. All rights reserved.
3+
ZORK is a registered trademark of Infocom, Inc.
4+
DORK is .... not
5+
Revision 1 / Serial number 8675309
6+
7+
You are standing in an open field west of a white house, with a boarded front door.
8+
There is a small mailbox here.
9+
> 7 Score: 0 Moves: 08The a small mailbox contains:
10+
leaflet
11+
> 7 Score: 0 Moves: 08Took leaflet.
12+
> 7 Score: 2 Moves: 08"WELCOME TO DORK!
13+
14+
DORK is a game of adventure, danger, and low cunning. In it you will explore some of the most amazing territory ever seen by mortals. No computer should be without one!"
15+
> 7 Score: 2 Moves: 08You are facing the north side of a white house. There is no door here, and all the windows are boarded up. To the north a narrow path winds through the trees.
16+
> 7 Score: 2 Moves: 18This is a path winding through a dimly lit forest. The path heads north-south here. One particularly large tree with some low branches stands at the edge of the path.
17+
There is tree here.
18+
> 7 Score: 2 Moves: 28You are about 10 feet above the ground nestled among some large branches. The nearest branch above you is above your reach.
19+
There is a bird's nest here with a jewel-encrusted egg visible inside.
20+
> 7 Score: 2 Moves: 38You are the proud owner of a very special egg.. Is it ticking?
21+
> 7 Score: 7 Moves: 38This is a path winding through a dimly lit forest. The path heads north-south here. One particularly large tree with some low branches stands at the edge of the path.
22+
There is tree here.
23+
> 7 Score: 7 Moves: 48Your score is 7
24+
> 7 Score: 7 Moves: 48You are carrying:
25+
jewel-encrusted egg
26+
leaflet
27+
> 7 Score: 7 Moves: 48Are you sure you want to quit? (y/n) > 7 Score: 7 Moves: 48

0 commit comments

Comments
 (0)