conductor(audit): trace set_value('ai_input') flow to find routing bug
This commit is contained in:
@@ -0,0 +1,69 @@
|
||||
# set_value('ai_input') Audit
|
||||
|
||||
## Current Status (as of 2026-06-09)
|
||||
**Test `tests/test_gui2_parity.py::test_gui2_set_value_hook_works` PASSES in isolation** (4.50s).
|
||||
|
||||
Prior report (`rag_work_final_20260609_pm.md`, 2026-06-09) said it was a batch failure. This audit verifies the current state.
|
||||
|
||||
## Endpoint code path
|
||||
|
||||
### Routing map (src/app_controller.py:1052)
|
||||
```python
|
||||
self._settable_fields: Dict[str, str] = {
|
||||
'ai_input': 'ui_ai_input',
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
### Handler (src/app_controller.py:554-571)
|
||||
```python
|
||||
def _handle_set_value(controller: 'AppController', task: dict):
|
||||
item = task.get("item")
|
||||
value = task.get("value")
|
||||
if item in controller._settable_fields:
|
||||
attr_name = controller._settable_fields[item]
|
||||
setattr(controller, attr_name, value)
|
||||
...
|
||||
```
|
||||
|
||||
### Init state (src/app_controller.py:996)
|
||||
```python
|
||||
self.ui_ai_input: str = ""
|
||||
```
|
||||
|
||||
### __getattr__ allowlist (src/app_controller.py:1239)
|
||||
`ui_ai_input` IS in `_UI_FLAG_DEFAULTS` (so `hasattr()` returns True).
|
||||
|
||||
## Expected flow
|
||||
1. `client.set_value('ai_input', 'hello')` → POST /api/gui with `{"action": "set_value", "item": "ai_input", "value": "hello"}`
|
||||
2. Endpoint dispatches to `_handle_set_value` (via the action handler map at line 1190)
|
||||
3. `_handle_set_value` looks up `_settable_fields["ai_input"]` → `"ui_ai_input"`
|
||||
4. `setattr(controller, "ui_ai_input", "hello")` → `controller.ui_ai_input = "hello"`
|
||||
5. `client.get_value('ai_input')` → POST /api/gui with `{"action": "get_value", "item": "ai_input"}`
|
||||
6. Returns `controller.ui_ai_input` = `"hello"`
|
||||
|
||||
## Actual flow (verified 2026-06-09)
|
||||
Test PASSES in isolation. Both `set_value` and `get_value` work correctly.
|
||||
|
||||
## Prior failure (per rag_work_final_20260609_pm.md)
|
||||
The prior report (2026-06-09 PM) said:
|
||||
> `test_gui2_set_value_hook_works` batch failure — `set_value` hook returns `'queued'` but `get_value('ai_input')` returns `''` after 1.5s. Different code path from RAG, pre-existing, not investigated this session per the Deduction Loop rule (2-failure cap). Likely a `setattr` routing issue in `gui_2.py` (same class of bug as the earlier `_UI_FLAG_DEFAULTS` fix).
|
||||
|
||||
The commit `bcdc26d0` ("fix(gui): correct __getattr__ to not silently return None for missing ui_ attrs") from the prior session likely fixed the underlying `__getattr__` issue. The test now passes in isolation.
|
||||
|
||||
## Remaining risk: BATCH behavior
|
||||
The test passes in isolation but was reported as a BATCH failure. The batch-vs-isolation gap is the same pattern as the RAG test:
|
||||
- In isolation, the live_gui subprocess starts FRESH, controller state is clean.
|
||||
- In batch, state from prior tests may have left a different default for `ui_ai_input` (e.g., a prior test set it to a non-empty value, and the session-scoped fixture didn't reset between tests).
|
||||
|
||||
## Recommendation
|
||||
1. Run the test in the live_gui tier-3 batch to confirm the batch-vs-isolation gap.
|
||||
2. If batch still fails, the fix is to add `controller.ui_ai_input = ""` to the `_handle_reset_session` method (which is called by `client.reset_session()` in the conftest fixture's `finally` block).
|
||||
3. Alternatively, the test may need to call `client.reset_session()` at the start to ensure a clean state.
|
||||
|
||||
## Files affected
|
||||
- src/app_controller.py:554 (`_handle_set_value` handler)
|
||||
- src/app_controller.py:1052 (`_settable_fields` map — already has `ai_input`)
|
||||
- src/app_controller.py:1239 (`_UI_FLAG_DEFAULTS` — already has `ui_ai_input`)
|
||||
- src/app_controller.py:_handle_reset_session (potential fix for batch state pollution)
|
||||
- tests/test_gui2_parity.py:1-50 (the test that exposes the issue)
|
||||
Reference in New Issue
Block a user