yakilee Claude Opus 4.6 commited on
Commit
51220b7
Β·
1 Parent(s): 008813e

docs: add lessons learned and cognitive notes to CLAUDE.md

Browse files

Capture recurring error patterns and cognitive lessons from past
sessions to prevent repeating mistakes across conversations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Files changed (1) hide show
  1. CLAUDE.md +67 -3
CLAUDE.md CHANGED
@@ -75,6 +75,70 @@ docs/ # Design docs and TDD guides
75
  Always commit atomically to build a clear git history for the larger dev team
76
 
77
  ## ALWAYS run scripts (bash/tests) in the background
78
- - you MUST always run the scripts in background to unblock the main context window;
79
- - When using timeout, it must be under 1 minute.
80
-
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
75
  Always commit atomically to build a clear git history for the larger dev team
76
 
77
  ## ALWAYS run scripts (bash/tests) in the background
78
+ - you MUST always run the scripts in background to unblock the main context window;
79
+ - When using timeout, it must be under 1 minute.
80
+
81
+ ## Lessons Learned (from past errors)
82
+
83
+ ### Async/Sync: never use asyncio.run() in Streamlit
84
+ - Streamlit has its own event loop; `asyncio.run()` will raise `RuntimeError: This event loop is already running`
85
+ - Use `ThreadPoolExecutor` + `asyncio.run` in a background thread as sync bridge
86
+ - If a method is declared `async`, verify the body actually awaits async I/O β€” don't wrap sync blocking calls in `async def` without `asyncio.to_thread`
87
+
88
+ ### Mocks must match real implementation
89
+ - Before writing test mocks, READ the actual service code first
90
+ - Example: MCP client switched from `client.post()` to `client.stream()` but tests still mocked `.post()` β†’ all tests passed locally but broke on integration
91
+ - Always verify mock signatures against the real method being called
92
+
93
+ ### Python import/path conflicts
94
+ - Never place an entrypoint file inside a package with the same name (e.g., `app/app.py` inside `app/` package)
95
+ - Streamlit adds parent dirs to `sys.path`, creating ambiguous imports
96
+
97
+ ### Git hygiene
98
+ - Always check `.gitignore` before committing; never commit `__pycache__/`, `.env`, or binary files
99
+ - Use `git diff --staged` to review before every commit
100
+
101
+ ### Test stability
102
+ - Centralize mock data in `conftest.py` shared fixtures, not inline per-test
103
+ - When data contracts change, update fixtures in ONE place
104
+
105
+ ### Bash output: prefer dedicated tools
106
+ - Use Read/Grep/Glob instead of bash pipes for file operations
107
+ - Keep bash commands simple and single-purpose; complex piped commands risk misreading output
108
+ - Always read the FULL output of bash commands before drawing conclusions
109
+
110
+ ## Cognitive Lessons (avoid repeating these thinking errors)
111
+
112
+ ### Know where configs live β€” don't re-discover every session
113
+ - ALL env vars and defaults: `trialpath/config.py` (single source of truth)
114
+ - Key env vars: `GEMINI_API_KEY`, `GEMINI_MODEL` (gemini-3-pro), `HF_TOKEN`, `MEDGEMMA_ENDPOINT_URL`, `MCP_URL` (:3000), `PARLANT_URL` (:8800), `SESSION_COST_BUDGET`
115
+ - MedGemma retry settings: `MEDGEMMA_MAX_RETRIES`, `MEDGEMMA_RETRY_BACKOFF`, `MEDGEMMA_MAX_WAIT`, `MEDGEMMA_COLD_START_TIMEOUT`
116
+ - `.env` file is gitignored β€” never commit it again (API keys were leaked once in commit 53efc3c)
117
+ - Config consumers: gemini_planner, medgemma_extractor, mcp_client, parlant_bridge, agent/tools, direct_pipeline
118
+
119
+ ### Don't flip-flop on implementation decisions
120
+ - `max_output_tokens` was added (65536) to fix truncation, then removed to "use defaults", causing regressions
121
+ - `os.environ.get()` inline was refactored to config imports, touching 6+ files each time
122
+ - LESSON: Make the decision ONCE with reasoning, document it, stick with it
123
+
124
+ ### Remember the project's fallback chain
125
+ - Pipeline has 3-tier fallback: Parlant β†’ direct API (direct_pipeline.py) β†’ mock data
126
+ - Demo mode bypasses file upload and loads MOCK_PATIENT_PROFILE directly
127
+ - Don't re-implement fallback logic β€” it already exists in `direct_pipeline.py`
128
+
129
+ ### Read existing code before writing new code
130
+ - Service instances were re-created per call in agent/tools.py until caching fix
131
+ - This pattern (wasteful instantiation) could have been caught by reading the code first
132
+ - ALWAYS read the file you're about to modify, especially service constructors
133
+
134
+ ### Don't lose track of what's stubbed vs real
135
+ - MedGemma: real HF endpoint wired (with retry/cold-start logic)
136
+ - Gemini: real API wired (with rate limiting)
137
+ - MCP/ClinicalTrials: has both MCP client AND direct API fallback
138
+ - Parlant: client ready, agent journey logic NOT yet implemented
139
+ - UI: all 5 pages functional with mock data fallback
140
+
141
+ ### Centralize shared state β€” don't scatter it
142
+ - Streamlit state keys: `patient_profile`, `trial_candidates`, `eligibility_ledgers`, `parlant_session_id`, `parlant_session_active`, `last_event_offset`, `journey_state`
143
+ - Test fixtures: centralized in `conftest.py` (root level), not per-test-file
144
+ - Mock data: `app/services/mock_data.py` (single file for all mock objects)