docs(agents,edit_workflow): capture session-learned anti-patterns (2026-06-07)
Captures the 5 patterns that burned the most time in the startup_speedup_20260606 sub-track 4 work: 1. ALWAYS use manual-slop_edit_file, not custom scripts (custom scripts fail silently on indent/EOL/whitespace drift) 2. The decorator-orphan pitfall (inserting before 'def foo' leaves @property decorating YOUR new method) 3. ast.parse() is not enough (semantic errors aren't caught; import + instantiate + call after every edit) 4. The git restore trap (don't run git status/restore while a user is mid-conversation) 5. Small verified edits beat big scripts (edit_workflow says 3-10 lines; if you write 200 lines of script, wrong tool) Also adds 2 new anti-patterns to the Critical list in AGENTS.md and 3 new sections to conductor/edit_workflow.md (decorator-orphan, ast.parse-not-enough, set_file_slice-is-literal).
This commit is contained in:
@@ -12,6 +12,7 @@ All AI agents consuming this project must read `./conductor/workflow.md` and tre
|
||||
|
||||
Detailed agent guidance lives in the following locations — read these directly, do not duplicate content here:
|
||||
|
||||
- **MUST READ TO DE-RETARD EDIT WORKFLOW** `conductor/edit_workflow.md`
|
||||
- **Operational workflow:** `conductor/workflow.md`
|
||||
- **Code style and process:** `conductor/product-guidelines.md`
|
||||
- **Tech stack and constraints:** `conductor/tech-stack.md`
|
||||
@@ -33,6 +34,39 @@ For understanding, using, and maintaining the tool, see `docs/Readme.md` and the
|
||||
- Do not skip TDD - write failing tests before implementation
|
||||
- Do not batch commits - commit per-task for atomic rollback
|
||||
- Do not add comments to source code; documentation lives in `/docs`
|
||||
- Do not use `set_file_slice` for multi-line content; it's literal line replacement by design (see `conductor/edit_workflow.md`)
|
||||
- Do not use `git restore` while a user is mid-conversation without first confirming the desired state
|
||||
|
||||
## Session-Learned Anti-Patterns (Added 2026-06-07)
|
||||
|
||||
These burned the most time in a recent startup_speedup session. The rules below are short because the rules above (and `conductor/edit_workflow.md`) are the source of truth.
|
||||
|
||||
### 1. ALWAYS use the proper edit tool, not a custom script
|
||||
|
||||
- For Python source edits, use `manual-slop_edit_file` with `old_string`/`new_string`. **Do NOT** write a standalone Python script that does file-level replacements.
|
||||
- Custom scripts fail silently on: wrong indent in `new_content`, wrong EOL (CRLF vs LF) in `old_string` searches, wrong exact-string match (whitespace drift).
|
||||
- When a script fails, debug the actual error message. Do not dismiss it and try a different approach.
|
||||
|
||||
### 2. The decorator-orphan pitfall
|
||||
|
||||
When inserting new methods **before an existing `@property` def**, your script will leave the `@property` decorator on the line above your new methods. The decorator then accidentally decorates YOUR new method (which is no longer a property, breaking any subsequent `@your_method.setter` calls). The file passes `ast.parse()` but blows up at import time.
|
||||
|
||||
The fix: anchor on the **def line that has the `@property` ABOVE it**, and replace the pair `@property\n def foo(...)` with `@property\n def your_new(...)\n ...\n def foo(...)` — keeping the decorator attached to its original method. Or anchor on a different non-decorated landmark (e.g. `self._init_actions()`).
|
||||
|
||||
### 3. `ast.parse()` "Syntax OK" is not enough
|
||||
|
||||
`ast.parse()` only catches syntax errors. Semantic errors (wrong decorator targets, wrong class attribute, missing `self`, etc.) are NOT caught. After a multi-line edit, ALWAYS:
|
||||
- Import the module
|
||||
- Instantiate the class
|
||||
- Call the new method in the way it's expected to be called (e.g. `ctrl.foo_ts` vs `ctrl.foo_ts()` for properties vs methods)
|
||||
|
||||
### 4. The "I'll just check git status" trap
|
||||
|
||||
If you suspect you might have lost work, the worst move is to run `git status` / `git restore` while a frantic user is watching. Pause, read the actual file, and admit what state you're in. The user knows their state better than you do.
|
||||
|
||||
### 5. Small, verified edits beat big scripts
|
||||
|
||||
`conductor/edit_workflow.md` says it explicitly: 3-10 lines at a time, verify after each, repeat. If you find yourself writing a 200-line Python script to do an edit, you're doing it wrong. Use the MCP tools.
|
||||
|
||||
## Compaction Recovery
|
||||
|
||||
|
||||
@@ -38,6 +38,33 @@ Before ANY edit to a function you haven't touched recently:
|
||||
- Nested blocks: ` ` (3 spaces total)
|
||||
- NO 4-space indentation anywhere in this file
|
||||
|
||||
### 6. The Decorator-Orphan Pitfall (Added 2026-06-07)
|
||||
|
||||
When inserting new methods **before an existing `@property` def**:
|
||||
```
|
||||
@property
|
||||
def perf_profiling_enabled(self) -> bool:
|
||||
...
|
||||
```
|
||||
If you anchor on `def perf_profiling_enabled` and insert before it, the `@property` decorator on the line above is left orphaned on the line right before YOUR new method. Now `@property` decorates your method (which is no longer a property), and the original setter `@perf_profiling_enabled.setter` blows up at import with `'function' object has no attribute 'setter'`.
|
||||
|
||||
**Fix:** Anchor on a non-decorated landmark, or include the decorator in the replacement:
|
||||
- `old_string` = ` self._init_actions()\n\n @property\n def perf_profiling_enabled`
|
||||
- `new_string` = ` self._init_actions()\n\n def your_new(...)\n ...\n\n @property\n def perf_profiling_enabled`
|
||||
|
||||
This keeps the `@property` attached to its original method.
|
||||
|
||||
### 7. ast.parse() Is Not Enough (Added 2026-06-07)
|
||||
|
||||
`py_check_syntax` only confirms `ast.parse()` succeeds. Semantic errors (wrong decorator targets, wrong base class, wrong attribute, missing `self`) are NOT caught. After any multi-line edit, ALWAYS:
|
||||
1. Import the module: `python -c "from src.app_controller import AppController"`
|
||||
2. Instantiate the class
|
||||
3. Call the new method in the way it's expected to be called (`ctrl.foo_ts` for a property, `ctrl.foo_ts()` for a method)
|
||||
|
||||
### 8. Do Not Use `set_file_slice` For Multi-Line Content (Added 2026-06-07)
|
||||
|
||||
`set_file_slice` does literal line replacement by design. It does not reindent, does not normalize EOL, does not parse decorators. Use it for surgical line-level edits (3-10 lines). If you need to insert or replace a multi-method block, use `manual-slop_edit_file` with verified exact-text old_string/new_string, or use `py_add_def` / `py_update_definition` for class/method-level work.
|
||||
|
||||
## Step-by-Step Workflow for gui_2.py
|
||||
|
||||
### Before ANY edit:
|
||||
|
||||
Reference in New Issue
Block a user