File size: 5,708 Bytes
dda4e9e
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
# SessionState Implementation Plan

## Status: 🟑 IN PROGRESS (Design Complete, Implementation Pending)

Branch: `feature/remove-global-state`

## Completed βœ…

1. **Analysis** - Identified all global variable usage (20+ functions affected)
2. **Design** - Created `SessionState` class with all required fields
3. **Documentation** - Wrote comprehensive analysis and risk assessment

## Next Steps (Implementation)

### Phase 1: Refactor Core Wrapper Functions (2-3 hours)

**Functions to Update (in order):**

1. `get_current_sheet(session: SessionState)` ← reads 4 globals
2. `load_character_wrapper(char_choice, session)` ← writes 3 globals
3. `load_character_with_debug_wrapper(char_choice, scenario, session)` ← writes 3 globals
4. `chat_wrapper(message, history, session)` ← reads 4 globals
5. `clear_history_wrapper(session)` ← writes 1 global
6. `load_party_mode_wrapper(session)` ← writes 4 globals
7. `add_to_party_wrapper(choices, session)` ← writes 2 globals
8. `remove_from_party_wrapper(name, session)` ← writes 2 globals

**Pattern for Each Function:**

```python
# BEFORE
def function_wrapper(arg1, arg2):
    global var1, var2
    # ... logic ...
    var1 = new_value
    return result

# AFTER
def function_wrapper(arg1, arg2, session: SessionState):
    # ... logic ...
    session.var1 = new_value
    return result, session  # CRITICAL: Return session!
```

**Testing After Each Function:**
- Add type hints to catch missing parameters
- Test manually with single user
- Verify state flows correctly

### Phase 2: Update Gradio Event Handlers (1-2 hours)

**Event Handlers to Update:**

1. Load character button (play tab)
2. Load party button (play tab)
3. Delete character button (play tab)
4. Chat submit button + Enter key (play tab)
5. Combat control buttons (next turn, end combat) (play tab)
6. Clear history button (play tab)
7. Quick action buttons (attack, cast, use item, help) (play tab)
8. Create character button (create tab)
9. Add/remove party buttons (party tab)

**Pattern for Event Handlers:**

```python
# BEFORE
component.click(
    wrapper_function,
    inputs=[arg1, arg2],
    outputs=[output1, output2]
)

# AFTER
component.click(
    wrapper_function,
    inputs=[arg1, arg2, session_state],  # Add session
    outputs=[output1, output2, session_state]  # Return session
)
```

**Special Cases:**
- `.then()` chains need session passed through
- Lambda functions need session parameter
- Multiple outputs need session appended

### Phase 3: Create gr.State Component (30 min)

**In app_gradio.py, after demo creation:**

```python
with demo:
    # Create session state component (ONE per user session)
    session_state = gr.State(create_session_state())

    # ... rest of UI setup ...
```

**Key Points:**
- Only ONE `gr.State` component created
- Passed to ALL event handlers
- Gradio automatically manages per-session instances

### Phase 4: Remove Global Variables (15 min)

**Delete from app_gradio.py (lines 75-80):**
```python
# DELETE THESE:
current_character = None
conversation_history = []
party = PartyState(...)
party_characters = {}
gameplay_mode = "character"
```

**Delete global gm (line 69):**
```python
# DELETE THIS:
gm = GameMaster(db)
```

**Search for remaining global statements:**
```bash
grep "global " web/app_gradio.py
# Should return NO results
```

### Phase 5: Integration Testing (1 hour)

**Test Scenarios:**

1. **Single User Flow:**
   - Load character βœ“
   - Chat with GM βœ“
   - Start combat βœ“
   - Use spells/items βœ“
   - Clear history βœ“

2. **Party Mode Flow:**
   - Create party βœ“
   - Add characters βœ“
   - Start combat βœ“
   - Remove characters βœ“

3. **Multi-User Test (CRITICAL):**
   - Open app in 2 browser windows (different sessions)
   - Window 1: Load Character A
   - Window 2: Load Character B
   - Window 1: Chat "Hello"
   - Window 2: Chat "Goodbye"
   - Verify: Each window shows only its own character/chat
   - Verify: No state leakage between sessions

4. **Edge Cases:**
   - Reload page (state should reset)
   - Switch between character/party mode
   - Rapid clicking (race conditions)

**Automated Test (Create Later):**
```python
# tests/test_session_state.py
def test_session_isolation():
    """Test that two SessionState instances are independent."""
    session1 = create_session_state()
    session2 = create_session_state()

    session1.gameplay_mode = "party"
    assert session2.gameplay_mode == "character"

    session1.conversation_history.append({"role": "user", "content": "test"})
    assert len(session2.conversation_history) == 0
```

## Success Criteria

- [ ] No `global` statements in app_gradio.py
- [ ] All event handlers pass/return `session_state`
- [ ] App works with single user (smoke test)
- [ ] App works with multiple users simultaneously (isolation test)
- [ ] No race conditions or state leakage
- [ ] All existing tests still pass

## Rollback Plan

If issues arise:
1. Discard changes: `git checkout main`
2. Delete feature branch: `git branch -D feature/remove-global-state`
3. Start over with smaller scope (e.g., only character mode first)

## Estimated Time

- Phase 1: 2-3 hours (function refactoring)
- Phase 2: 1-2 hours (event handler updates)
- Phase 3: 30 minutes (gr.State setup)
- Phase 4: 15 minutes (cleanup)
- Phase 5: 1 hour (testing)

**Total: 5-7 hours of focused work**

## Current Status

**Completed:**
- βœ… Analysis (30 min)
- βœ… Design (30 min)
- βœ… Documentation (30 min)

**Remaining:**
- ⏳ Implementation (5-7 hours)

**Branch:** `feature/remove-global-state`
**Ready to merge:** NO (implementation not started)
**Blocking issues:** None
**Next action:** Begin Phase 1 (refactor wrapper functions)