You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: .claude/CLAUDE.md
+176-5Lines changed: 176 additions & 5 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -91,14 +91,38 @@ Fix spelling, grammar, and other minor problems without asking the user. Label a
91
91
92
92
Only report what you have changed.
93
93
94
+
## Programming rules for LLMs
95
+
1. mostly lower-case comments
96
+
2. no in-line comments
97
+
3. don't number comments
98
+
4. comments are followed by '----', such as "# dependencies----". nothing that takes a full line.
99
+
5. no unnecessary code changes beyond what is already done
100
+
6. unless I ask, don't print too much code at once
101
+
7. don't do unnecessary print statements within the code.
102
+
8. don't add unnecessary try/catch statements or make unnecessary validation checks. i'm working by myself, no need for these things -- I want to see the errors!
103
+
9. NO EMOJIS
104
+
10. in R code, 2 spaces per tab and base R pipe
105
+
94
106
## Refactor rules
95
-
We're attemping to refactor many issues with this repositroy. In general, let's:
96
-
- make sure all examples are functional and not wrapped in dontrun{}
107
+
108
+
### Per-function checklist
109
+
110
+
When refactoring any estimator or internal function, apply all of these:
111
+
112
+
1. Make runnable examples (no `\dontrun{}`)
113
+
2. Remove commented-out code
114
+
3. Remove TODOs
115
+
4. Add type checks (checkmate)
116
+
5. Factor out tidyverse (`filter` → `df[cond, ]`, `mutate` → direct assignment)
117
+
6.`<-` for assignment (not `=`)
118
+
7. Drop debug prints (`cat`, `print`, commented `# print(...)`)
119
+
8. Consistent variable naming without periods (e.g. `pi_S` not `pi.S`)
120
+
9. Drop backtick column access (`temp$\`piA\`` → `temp$piA`)
121
+
122
+
### General rules
123
+
97
124
- make sure we have test cases for all functions
98
-
- remove @TODO blocks and other ugly code
99
125
- remove magrittr pipe and replace with base R pipe
100
-
- remove tidyverse dependencies
101
-
- replace "=" with "<-"
102
126
- make sure every function is documented
103
127
- add validation to function inputs
104
128
@@ -109,3 +133,150 @@ Run this one-liner to validate the package before committing:
1.**Better method constructors** — replace `setup_method_weighting(method_name="IPW", ...)` with `ec_ipw()`, etc. Each constructor carries its own estimation logic.
142
+
2.**Polymorphic dispatch** — `run_analysis()` calls a generic on the method object instead of an if/else tree. Adding a new method = writing one constructor.
143
+
3.**Merge bootstrap** — bootstrap is an inference option on the method, not a separate code path.
144
+
4.**(Future) Model formula interface** — `outcome ~ treatment | covariates` instead of column name args.
While new method constructors coexist with the old if/else tree in `run_analysis()`, each new class gets an `else if` block at the end of `run_analysis()` that calls `estimate()`:
223
+
224
+
```r
225
+
# In run_analysis.R, BEFORE the final } else { stop(...) }:
226
+
} elseif (is(method, "ec_ipw_method")) {
227
+
res<- estimate(method,
228
+
data=data,
229
+
outcomes=outcome_col_name,
230
+
treatment=treatment_col_name,
231
+
trial_status=trial_status_col_name,
232
+
covariates=covariates_col_name,
233
+
alpha=alpha,
234
+
quiet=quiet
235
+
)
236
+
}
237
+
```
238
+
239
+
This pattern is repeated for each new method class as it's created. The old if/else branches for the legacy classes remain untouched. Once all 6 methods are migrated, the entire if/else tree is replaced with a single `estimate()` call.
240
+
241
+
New method objects inherit from `method_primary_obj` or `method_OLE_obj`, so they pass the existing `checkmate::assert_class(method, "method_primary_obj")` validation in `setup_analysis_primary()`.
242
+
243
+
### Implementation order
244
+
245
+
1. Create full-pipeline regression tests for all 6 methods (old API, locked numerical values) ✓
246
+
2. Create all 6 method constructors (start with `ec_ipw()`)
247
+
3. Each constructor returns an S4 object with estimation logic via `estimate()` generic
248
+
4. For each new method, add an `else if (is(method, "xxx_method"))` to `run_analysis()`
249
+
5. Add new-API tests to each pipeline test file (same expected values)
250
+
6. Once all 6 are done: refactor `setup_analysis()` into a single function (merge `_primary`/`_OLE`, add `T_cross = NULL`)
251
+
7. Once all 6 are done: replace the entire if/else in `run_analysis()` with one `estimate()` call
-**S4 classes for method objects** — keeps rigorous type definitions, consistent with existing package patterns.
257
+
-**`T_cross` goes in `setup_analysis()`** — it's a property of the study design, not the method.
258
+
-**`bootstrap_ci_type` is nullable** — defaults to `"perc"` when `bootstrap` is non-NULL, ignored otherwise.
259
+
260
+
### Full-pipeline regression tests
261
+
262
+
Before refactoring any estimator, we lock in its numerical outputs on `SyntheticData` so any code change that alters results is caught. Tests use the old API (setup_method → setup_analysis → run_analysis) with exact values at `tolerance = 1e-6`. As new API constructors are added, we add parallel assertions against the same expected values.
263
+
264
+
Rename existing `test-vignette_results_*` files and split by method:
0 commit comments