Skip to content

Commit 426d67b

Browse files
jose-donatojose-donato
committed
initial commit
0 parents  commit 426d67b

242 files changed

Lines changed: 40971 additions & 0 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.claude/skills/clean-fix-harness/SKILL.md‎

Lines changed: 289 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,289 @@
1+
---
2+
name: clean-fix-harness
3+
description: Use every time the user asks to fix, debug, repair, patch, harden, clean up, or investigate broken behavior in this repo. Applies especially to agent harness, model/tool routing, SQL family, widget data, MCP, streaming, workspace bridge, citations, artifacts, evals, and tests. Enforces the repo's lean-harness rule: reproduce defects faithfully, fix at the root, keep the model responsible for choosing paths, avoid deterministic retries, avoid content/word-based routing, avoid hidden normalization, and reject overfit special cases.
4+
allowed-tools: Read, Glob, Grep, Edit, Write, Bash
5+
---
6+
7+
# Clean Fix Harness
8+
9+
Use this skill for every fix request in this repo. It complements `fix-with-test`: this skill defines
10+
what a good fix is allowed to look like, and `fix-with-test` defines the red/green discipline.
11+
12+
The core rule: **the harness prepares state and exposes tools; the model chooses the path.** Fixes
13+
must remove root causes, not build deterministic crutches around the model.
14+
15+
## Required Workflow
16+
17+
1. **Read the real context first**
18+
- Inspect the current branch diff against `main` when the bug is PR-related.
19+
- Read the relevant source, tests, and logs before proposing a fix.
20+
- Use `rg` for suspicious strings and code paths; do not rely on memory.
21+
- Keep existing user changes. Never reset or revert unrelated work.
22+
23+
2. **Classify the defect**
24+
- Pure code defect: parser, schema, SQL loader, artifact builder, event formatter, MCP handler.
25+
- Harness behavior defect: loop dispatch, streaming, artifact queue, state restoration, bridge routing.
26+
- Model behavior defect: wrong tool chosen, wrong tool args, bad final formatting, wrong reasoning path.
27+
- Contract defect: prompt/tool description/schema says the wrong thing or leaves ambiguity.
28+
29+
3. **Reproduce before fixing**
30+
- For bugs, follow `fix-with-test`: write a failing unit/integration test or eval first.
31+
- Confirm the red is for the real behavior, not bad test setup.
32+
- If the issue is model/tool choice, use a real-LLM eval with deterministic graders where possible.
33+
- If the issue is deterministic code, use unit or integration tests at the lowest faithful layer.
34+
35+
4. **Fix at the root**
36+
- Broken implementation: patch the implementation.
37+
- Broken schema contract: patch the Zod schema or typed contract.
38+
- Broken tool description: patch the tool description.
39+
- Broken model instruction: patch the prompt or eval surface.
40+
- Broken display rendering: patch the renderer or protocol, not model-output cleanup, unless the
41+
protocol explicitly requires sanitization.
42+
43+
5. **Verify**
44+
- Run the focused failing test/eval until green.
45+
- Run nearby tests affected by the changed contract.
46+
- Run `bun run typecheck`.
47+
- Run `LIVE_DEBUG_TRACES=0 bun test tests/` before handing off a merge-ready fix.
48+
49+
## Harness Boundary Rules
50+
51+
The agent harness may:
52+
53+
- Dispatch structurally by tool source or protocol event type.
54+
- Validate tool input against Zod schemas.
55+
- Preserve request-scoped state such as `pendingTables`, `artifactQueue`, citations, loaded skills, and
56+
widget fetch queues.
57+
- Expose state-bound tools that operate on request state, such as SQL-family tools.
58+
- Emit structural SSE events for tool calls, tool results, artifacts, citations, and bridge commands.
59+
- Enforce safety boundaries, such as SQL read-only checks and blocking SQLite internals.
60+
- Strip narrow protocol placeholders that should never be user-visible.
61+
- Cache or rehydrate state when the protocol explicitly requires it.
62+
63+
The agent harness must not:
64+
65+
- Decide the analytical path from user wording.
66+
- Infer that a specific content shape means a specific action.
67+
- Repair model mistakes by silently changing tool args.
68+
- Retry a failed path deterministically because an error string matched a known pattern.
69+
- Convert one identifier kind into another to make a model call "work".
70+
- Hide an invalid model call by dropping conflicting fields or choosing a preferred source.
71+
- Promote, suppress, or transform final answer content because it contains certain words.
72+
- Parse tool-output prose to create a special UI state unless the output is a typed protocol payload.
73+
- Add local branches that encode one observed failure trace instead of fixing the underlying contract.
74+
75+
## Patterns We Never Want
76+
77+
### Content-Based Routing
78+
79+
Never add logic like:
80+
81+
```ts
82+
if (userText.includes("csv")) createArtifact();
83+
if (output.startsWith("SQL error:")) showSqlFailurePath();
84+
if (looksLikeMarkdownTable(finalText)) removeIt();
85+
if (dataLooksStructured(value)) useSqlPath();
86+
if (message.includes("error")) retry();
87+
```
88+
89+
Why: this moves decisions from the model into brittle string matching. It creates overfit behavior that
90+
works for one trace and fails in adjacent cases.
91+
92+
Correct alternatives:
93+
94+
- Make the tool/prompt contract clear enough that the model chooses the right tool.
95+
- Return typed results for things the harness must understand.
96+
- Add a deterministic eval for routing behavior.
97+
- Fix the renderer if display is broken.
98+
99+
### Forced Retry or Deterministic Recovery
100+
101+
Never add logic like:
102+
103+
```ts
104+
if (toolFailed) callSearchAgain();
105+
if (sqlError.includes("no such column")) rewriteSql();
106+
if (widgetFetchFailed) trySameNameWidget();
107+
if (modelStopped) runAnother orchestration loop with a new instruction;
108+
```
109+
110+
Allowed exception: a retry may exist only for a narrow external transient with a structural signal, such
111+
as a stopped sandbox process or a network timeout. The retry must not inspect semantic content, must be
112+
bounded, and must be documented in the owning tool layer.
113+
114+
Correct alternatives:
115+
116+
- Surface the failure to the model as tool output.
117+
- Give the model enough state to choose the next call.
118+
- Patch the prompt/tool description when the model repeatedly chooses the wrong recovery path.
119+
- Patch the root service/tool when the failure is not model-controllable.
120+
121+
### Hidden Identifier Standardization
122+
123+
Never silently convert or reconcile identifiers:
124+
125+
```ts
126+
widget.uuid ??= widget.widget_id;
127+
find(w => w.uuid === id || w.widget_id === id);
128+
dashboardId = lookupByDashboardTitle(userText);
129+
widget_uuid = widget_id;
130+
origin = origin.trim() || null;
131+
```
132+
133+
Why: identifiers are protocol contracts. If the model sends the wrong identifier kind, silently fixing it
134+
teaches the system to depend on ambiguity and masks prompt/schema bugs.
135+
136+
Correct alternatives:
137+
138+
- Make the prompt/tool description say exactly which identifier is required.
139+
- Make schemas reject wrong or missing fields.
140+
- Pass through bridge arguments when the browser bridge owns interpretation.
141+
- Add an eval or integration test proving the model uses the correct identifier.
142+
143+
### Error-String Interpretation
144+
145+
Never parse generic error text to pick a specialized path:
146+
147+
```ts
148+
if (/no such column/.test(error)) suggestColumn();
149+
if (/date_trunc/.test(error)) rewriteForSQLite();
150+
if (/ORDER BY term/.test(error)) injectSortKey();
151+
```
152+
153+
Correct alternatives:
154+
155+
- Return raw tool errors plus structural context such as available table/column inventories.
156+
- Improve tool descriptions and prompts with general dialect constraints.
157+
- Let the model decide how to adjust its next SQL/tool call.
158+
159+
### Tool Output Text Parsing
160+
161+
Never parse human-readable tool output to create artifacts/statuses:
162+
163+
```ts
164+
const rows = parseRowsAfter(output, /^Rows:/);
165+
if (output.match(/^Created chart artifact/)) status = "Chart";
166+
if (output.match(/^Table "([^"]+)"/)) status = "Missing table";
167+
```
168+
169+
Correct alternatives:
170+
171+
- Use typed `SSEEvent`, typed MCP content, Zod schemas, or explicit JSON structures.
172+
- Keep human-readable text as preview only.
173+
- If the UI needs structure, change the tool to return a typed structure rather than parsing prose.
174+
175+
### Silent Source Precedence
176+
177+
Never accept conflicting model inputs and silently choose one:
178+
179+
```ts
180+
const source = sql ?? fromTableId ?? data;
181+
if (sql && fromTableId) ignoreFromTableId();
182+
```
183+
184+
Correct alternative:
185+
186+
- Reject conflicting inputs with a schema/tool error.
187+
- Describe the contract clearly: exactly one source, exactly one mode, or explicit precedence if the
188+
protocol truly requires precedence.
189+
190+
### Final Answer Surgery
191+
192+
Do not "clean up" final text with broad transformations:
193+
194+
```ts
195+
removeMarkdownTables(finalText);
196+
if (userAskedForExport) suppressInlineCsv(finalText);
197+
if (artifactExists) stripMatchingSections(finalText);
198+
```
199+
200+
Correct alternatives:
201+
202+
- Prompt the model to keep final text concise and put tabular output in artifacts.
203+
- Emit artifacts through the artifact queue.
204+
- Strip only narrow protocol placeholder leaks that should never be visible, such as artifact tags.
205+
- Fix renderer bugs where valid Markdown is displayed incorrectly.
206+
207+
### Special-Case Fixes From One Trace
208+
209+
Be suspicious of fixes that mention a single metric, widget, table, column, provider, or error string
210+
unless the defect is truly in that named integration.
211+
212+
Bad smell examples:
213+
214+
- Branches for a particular widget name.
215+
- Branches for one SQL table like `executive_esg_kpi_snapshot`.
216+
- Regex for one model's phrasing.
217+
- Tests that assert one exact model answer instead of the contract.
218+
- Prompt changes that describe one incident instead of a reusable rule.
219+
220+
## What Good Fixes Look Like
221+
222+
Good fixes are usually one of these:
223+
224+
- **Contract fix**: sharpen a tool description, prompt rule, Zod schema, or protocol type.
225+
- **Root implementation fix**: correct the function that owns the behavior.
226+
- **Typed payload fix**: replace prose parsing with structured data.
227+
- **Validation fix**: reject ambiguous/conflicting inputs early.
228+
- **Safety fix**: block unsafe operations structurally.
229+
- **Renderer/protocol fix**: fix how events or artifacts are rendered instead of rewriting model text.
230+
- **Regression guard**: focused test/eval that proves the bad behavior does not return.
231+
232+
## Testing Guidance
233+
234+
Use the lowest faithful layer:
235+
236+
- Unit tests for pure logic, schemas, tool handlers, SQL safety, artifact creation.
237+
- Integration tests for agent loop behavior, streaming, SSE ordering, state restoration, decoration.
238+
- Real evals for model routing and tool-choice behavior.
239+
240+
Do not write tests that bake in model internals:
241+
242+
- Avoid exact final-answer prose unless the formatter is deterministic.
243+
- Avoid exact tool order unless order is the harness contract.
244+
- Do not hard-code a mocked model tool choice to prove a model-routing bug.
245+
- Grade end-state and contracts: tool was available, tool was called/not called, args shape is correct,
246+
artifact exists, invalid input is rejected, no bridge call is emitted.
247+
248+
## Review Checklist Before Finishing a Fix
249+
250+
Ask these before finalizing:
251+
252+
- Did I reproduce the defect at the right layer?
253+
- Did the test fail before the fix for the right reason?
254+
- Did I remove, rather than add, deterministic pathing where possible?
255+
- Did I avoid content/word-based routing?
256+
- Did I avoid hidden identifier conversion or source precedence?
257+
- Did I avoid broad final-answer surgery?
258+
- Did I keep schema validation as validation, not normalization?
259+
- Did I leave unrelated user changes alone?
260+
- Did I update tests/evals to guard the actual contract?
261+
- Did I run `bun run typecheck` and the relevant Bun tests?
262+
263+
## When To Stop And Re-Plan
264+
265+
Stop and re-plan if:
266+
267+
- The only fix you can think of is `includes(...)`, `match(...)`, or a regex on model/tool prose.
268+
- The test can only be made red by mocking the model into a bad call.
269+
- The fix needs to special-case a single observed answer.
270+
- The code must silently guess what the model meant.
271+
- The branch adds a second source of truth for identifiers, table names, or widget mappings.
272+
- A prompt change grows into a long incident-specific explanation.
273+
- You are about to remove user text from the final answer because it "looks duplicated".
274+
275+
In these cases, find the root contract. Usually it is one of:
276+
277+
- Wrong tool description.
278+
- Ambiguous prompt rule.
279+
- Missing schema validation.
280+
- Missing typed payload.
281+
- UI renderer issue.
282+
- Test/eval at the wrong layer.
283+
284+
## Commit Discipline
285+
286+
- Do not commit unless the user asks.
287+
- If committing, stage only files relevant to the fix.
288+
- Never include unrelated dirty files or generated logs by accident.
289+
- Mention untracked or intentionally excluded files in the final response.

0 commit comments

Comments
 (0)