upravy skillu
This commit is contained in:
@@ -40,38 +40,33 @@ Answer in the language the user writes in.
|
||||
|---|---|
|
||||
| `reflect/findings.jsonl` | one JSON object per line — the findings store |
|
||||
| `reflect/state.json` | cursor and per-run statistics (owned by the script) |
|
||||
| `results/<date>_reflect.md` | full report of a run |
|
||||
| `results/<date>_reflect.md` | full report of a run — read it when the user asks what the last run found |
|
||||
| `log/reflect.log` | append-only audit of every decision — applied, rejected, skipped |
|
||||
|
||||
A finding has: `id`, `status`, `created`, `pattern`, `severity`, `diagnosis`,
|
||||
`last_seen`, `evidence`, `occurrences`, `sessions_affected`, `proposal`, optional `patch`
|
||||
(`file` / `old_text` / `new_text`), optional `patch_drafted_at` (the patch was drafted
|
||||
during a review, not proposed by the analysis), optional `regression_of`, optional `history`,
|
||||
and once decided one of `applied` (`at` / `sha` / `file`, plus `new_text` and
|
||||
`edited_by_user` when the user rewrote it), `rejected` (`at` / `reason`), `skipped`
|
||||
(`count` / `last`).
|
||||
Statuses: `watch` (seen once, not yet worth the user's attention) · `open` (waiting for
|
||||
review) · `applied` · `rejected`. Beyond the obvious fields a finding carries:
|
||||
|
||||
Statuses: `watch` (seen once, not worth the user's attention yet) · `open` (waiting for
|
||||
review) · `applied` · `rejected`.
|
||||
- `last_seen` — the newest date in the **evidence**, not when the record was rewritten;
|
||||
older records lack it, fall back to `created`.
|
||||
- `history` — superseded records of this pattern as `"<created>:<id>"`; the date in
|
||||
`history[0]` is when it was first filed.
|
||||
- `regression_of` — the `id` of the applied fix this pattern came back after.
|
||||
- `patch` (`file` / `old_text` / `new_text`) and `patch_drafted_at` — see step 2.
|
||||
- `applied` / `rejected` once decided, `skipped` while the finding is still open — see
|
||||
[Decision history](#decision-history) for what is in them.
|
||||
|
||||
Two things about the numbers, both of which you must not overstate to the user:
|
||||
|
||||
- A run analyses a **window** of recent sessions, not the whole history. `state.json`
|
||||
holds the window it used; the dated report repeats it in its header.
|
||||
- `occurrences` and `sessions_affected` are **cumulative across runs** — the sum of what
|
||||
the model counted in each slice, not a figure anyone measured over the whole corpus.
|
||||
`last_seen` is the newest date in the evidence, `history[0]` when the pattern was first
|
||||
filed, `created` only when the record was last rewritten. Older records may lack
|
||||
`last_seen`; fall back to `created` for those.
|
||||
Do not overstate the numbers: a run analyses a **window** of recent sessions, not the whole
|
||||
history (`state.json` holds the window, the report repeats it), and `occurrences` /
|
||||
`sessions_affected` are **cumulative across runs** — what the model counted in each slice,
|
||||
not a measurement over the corpus.
|
||||
|
||||
## Procedure
|
||||
|
||||
### 1. Load
|
||||
|
||||
Read `reflect/findings.jsonl`. Take the records with `status: open`, sorted by severity
|
||||
(`high`, `medium`, `low`), then by `last_seen` and then by `occurrences`, all descending.
|
||||
`occurrences` alone would let a stale pattern with a large cumulative count outrank a
|
||||
fresh one.
|
||||
Read `reflect/findings.jsonl`. Take the records with `status: open` and sort them:
|
||||
**regressions first** (`regression_of` is set), then by severity (`high`, `medium`, `low`),
|
||||
then `last_seen`, then `occurrences`, all descending.
|
||||
|
||||
A finding whose `last_seen` is older than the window of the latest run (`state.json`,
|
||||
`window_from`) is **stale**: no run looks that far back any more, so nothing will refresh
|
||||
@@ -83,9 +78,44 @@ and in the audit log, and is never what you ask the user to type.
|
||||
|
||||
If there are none: say so, mention how many `watch` findings are being tracked, and stop.
|
||||
|
||||
### 2. Present one finding
|
||||
### 2. Prepare the patch
|
||||
|
||||
Show exactly one, in this shape:
|
||||
The user decides on a diff, not on a sentence describing one, so have the patch ready
|
||||
*before* you present the finding.
|
||||
|
||||
- **It already has a `patch`** — the analysis wrote one, or `patch_drafted_at` says an
|
||||
earlier review drafted it and ended without deciding; say so when you present it. Either
|
||||
way, verify it still applies:
|
||||
|
||||
```bash
|
||||
uv run --script skills/reflect/scripts/reflect_apply.py --id <internal id> --check
|
||||
```
|
||||
|
||||
It changes nothing and prints the diff. Exit code 2 means the patch no longer applies —
|
||||
relay the printed reason and offer only *skip* / *reject*.
|
||||
|
||||
- **It has none** — most findings. Draft one now, before anything is shown. Read the target
|
||||
file for one thing only: copying `old_text` out of it character-for-character (never
|
||||
retype it, watch the quotation marks); it must occur in the file exactly once. The diff
|
||||
always comes from the script, never from your own reading. File the draft with
|
||||
`--set-patch`:
|
||||
|
||||
```bash
|
||||
# tmp/patch.json: {"file": "SOUL.md", "old_text": "<copied character-for-character>", "new_text": "<the fix>"}
|
||||
uv run --script skills/reflect/scripts/reflect_apply.py --id <internal id> --set-patch tmp/patch.json
|
||||
```
|
||||
|
||||
A refusal (exit 2) changes nothing — relay the reason and draft again. On success it
|
||||
prints the diff, touches no file and decides nothing; applying still needs step 4.
|
||||
|
||||
- **No file edit can fix it** — "think before answering", "be less eager to agree". Say that
|
||||
plainly, offer only *skip* / *reject*, and give no odds. Never invent a patch just to have
|
||||
something to show.
|
||||
|
||||
### 3. Present one finding
|
||||
|
||||
Show exactly one, in this shape — labels in the user's language, the odds line as
|
||||
[Estimating the odds](#estimating-the-odds) prescribes:
|
||||
|
||||
```text
|
||||
[1/4] retry-without-diagnosis · medium · 7× in 4 sessions
|
||||
@@ -97,85 +127,46 @@ Evidence:
|
||||
- …
|
||||
|
||||
Proposal: <proposal>
|
||||
|
||||
<the diff, exactly as the script printed it>
|
||||
|
||||
Odds it works: ~40 % — rewords guidance in a file that is in context, but this pattern
|
||||
already came back once after a fix.
|
||||
```
|
||||
|
||||
Write the labels in the user's language, not necessarily as shown here.
|
||||
|
||||
The second line carries what the count alone hides: `history[0]` for when the pattern
|
||||
was first filed, `last_seen` for when it last actually occurred, and `skipped.count` when
|
||||
the user has already deferred it. Drop the skip part when there is none; a finding deferred
|
||||
several times is worth saying so about, because rejecting it is cleaner than a list that
|
||||
The second line comes from `history[0]` (first filed), `last_seen` (when the pattern last
|
||||
actually occurred) and `skipped.count`; drop the skip part when there is none, and do say
|
||||
when a finding has been deferred several times — rejecting it is cleaner than a list that
|
||||
keeps re-presenting it.
|
||||
|
||||
Mark a finding with `regression_of` clearly as a **regression** — this pattern was fixed
|
||||
before and came back after the fix. Mark a stale finding (see step 1) as such too, and
|
||||
say what it means: the evidence predates the current window, so the pattern may well be
|
||||
gone already.
|
||||
|
||||
If the finding has a `patch`, get the diff from the script — never by reading the file and
|
||||
judging for yourself, that is the duplication gate 4 exists to prevent:
|
||||
|
||||
```bash
|
||||
uv run --script skills/reflect/scripts/reflect_apply.py --id <internal id> --check
|
||||
```
|
||||
|
||||
It changes nothing and prints the diff; show the user what it printed. Exit code 2 means the
|
||||
patch no longer applies — relay the printed reason and offer only *skip* / *reject*.
|
||||
Mark a finding with `regression_of` clearly as a **regression**, and a stale one (step 1)
|
||||
as stale.
|
||||
|
||||
Then ask for a decision and wait.
|
||||
|
||||
### 3. Accept a decision
|
||||
### 4. Decide and apply
|
||||
|
||||
| Input | Meaning |
|
||||
|---|---|
|
||||
| `ok`, `apply` | apply this finding's patch |
|
||||
| `edit: <text>` | the user rewrites `new_text`; show the new diff and ask again |
|
||||
| `skip` | record the deferral, leave it `open`, move to the next |
|
||||
| `reject` | **ask why first**, then close it for good — it never opens again |
|
||||
| `stop` | end the review |
|
||||
Every row runs `uv run --script skills/reflect/scripts/reflect_apply.py --id <internal id>
|
||||
…` — `--id` is the internal `id`, never the display number you showed.
|
||||
|
||||
Accept the equivalents in whatever language the user writes in — the words above are the
|
||||
meanings, not a required vocabulary.
|
||||
| Input | Meaning | Flags |
|
||||
|---|---|---|
|
||||
| `ok`, `apply` | apply this finding's patch | *(none)* |
|
||||
| `edit: <text>` | the user rewrites `new_text` | write their version to a temp file; `--check --new-text-file <path>` shows the new diff and changes nothing, then on `ok` the same file **without** `--check` |
|
||||
| `skip` | record the deferral, leave it `open`, move to the next | `--skip` |
|
||||
| `reject` | **ask why first**, then close it for good — it never opens again | `--reject --reason "<user's words>"` |
|
||||
| `stop` | end the review | *(nothing to run)* |
|
||||
|
||||
Rejection needs a reason and the script will not take it without one. Ask for it in one
|
||||
short question and pass the user's own words through — half of the first findings were
|
||||
rejected, and that number only says something about the analysis if the reasons are on
|
||||
the record. Do not invent a reason, and do not talk the user out of rejecting.
|
||||
Keep that temp file until the finding is decided — the user's wording never enters `patch`.
|
||||
|
||||
A finding without a `patch` cannot be applied. Offer to draft one, and file it with
|
||||
`--set-patch` — **never by writing to `reflect/findings.jsonl` yourself** (gate 4):
|
||||
Accept equivalents in the language the user writes in — the words above are meanings, not a
|
||||
required vocabulary.
|
||||
|
||||
```bash
|
||||
# {"file": "SOUL.md", "old_text": "<copied character-for-character>", "new_text": "<the fix>"}
|
||||
uv run --script skills/reflect/scripts/reflect_apply.py --id <internal id> --set-patch tmp/patch.json
|
||||
```
|
||||
Rejection needs a reason and the script will not take it without one: ask in one short
|
||||
question, pass the user's own words through, invent nothing, and do not talk them out of it.
|
||||
|
||||
`old_text` must be copied from the current file and occur in it exactly once — copy it,
|
||||
never retype it, and watch the quotation marks. The script verifies the patch *before*
|
||||
storing it, so a refusal (exit 2) leaves the finding exactly as it was; relay the reason
|
||||
and draft again. On success it prints the diff — show that, then ask for the `ok`. It
|
||||
changes no file and decides nothing, so applying still needs step 4.
|
||||
|
||||
### 4. Apply (only after `ok`)
|
||||
|
||||
**Never edit the file yourself.** `scripts/reflect_apply.py` does it, and it is what
|
||||
enforces the gates — it refuses a stale or ambiguous patch, commits only the touched
|
||||
file, writes the audit line and updates the store. Editing by hand would bypass all of it.
|
||||
|
||||
```bash
|
||||
uv run --script skills/reflect/scripts/reflect_apply.py --id <internal id>
|
||||
```
|
||||
|
||||
- The `--id` is the finding's internal `id`, not the display number you showed the user.
|
||||
- User rewrote the text (`edit:`)? Write their version to a temp file and pass
|
||||
`--new-text-file <path>`.
|
||||
- *skip* → `--skip`. Changes no file and no status; it only counts the deferral.
|
||||
- *reject* → `--reject --reason "<user's words>"`. It changes no file.
|
||||
- Exit code 2 means refused: relay the printed reason and move on. Do not work around it,
|
||||
do not edit the file to make the patch fit.
|
||||
|
||||
On success the script prints the commit SHA and the revert command. Pass that on, then
|
||||
continue with the next finding.
|
||||
Exit code 2 means refused: relay the printed reason and move on. On success the script
|
||||
prints the commit SHA and the revert command — pass those on, then take the next finding.
|
||||
|
||||
### 5. Close
|
||||
|
||||
@@ -183,6 +174,31 @@ When the user stops or the list is exhausted, summarise: how many applied, skipp
|
||||
rejected, and how many remain `open`. If anything was applied, remind the user that the
|
||||
change lives only on the server and should be pulled back into the tracking repo.
|
||||
|
||||
## Estimating the odds
|
||||
|
||||
Read the odds off the record, not off a feeling — otherwise two reviews of the same finding
|
||||
say 40 % and 75 %. Start from what the patch changes:
|
||||
|
||||
| The patch … | Band |
|
||||
|---|---|
|
||||
| changes mechanism — a script, a gate the agent cannot talk itself past | ~80 % |
|
||||
| adds a hard prohibition to a file that is in context when the pattern occurs (`SOUL.md`, `AGENTS.md`, the evidence's own `SKILL.md`) | ~60 % |
|
||||
| rewords or softens existing guidance in such a file | ~40 % |
|
||||
| edits a file that is not in context at that moment, or leaves the call to the agent's judgement | ~20 % |
|
||||
|
||||
Move one band down for each of these — ~20 % is the floor — and name the reason in the line:
|
||||
|
||||
- `regression_of` — this kind of instruction has already failed on this very pattern.
|
||||
- the evidence spans unrelated contexts — the trigger is not in the file being patched.
|
||||
|
||||
Two rules:
|
||||
|
||||
- it is an estimate from the record, **not a measurement** — say so the first time you
|
||||
give one;
|
||||
- keep to the bands; `63 %` claims a precision that is not there.
|
||||
|
||||
A stale finding (step 1) gets no percentage — say the pattern may already be gone.
|
||||
|
||||
## Decision history
|
||||
|
||||
Asked what was already decided, or what a past decision changed: read `log/reflect.log`
|
||||
@@ -199,7 +215,6 @@ Do not reconstruct this list from memory or from the findings store alone.
|
||||
|
||||
## When a finding is wrong
|
||||
|
||||
Findings come from an LLM reading its own logs and can be plain wrong. That is expected —
|
||||
rejection is a normal outcome, not a failure. If several findings in a row are noise, say
|
||||
so plainly; that is a signal the analysis prompt needs tuning, and it is worth telling the
|
||||
user rather than working through a list of nonsense.
|
||||
Findings come from an LLM reading its own logs and can be plain wrong; rejection is a normal
|
||||
outcome, not a failure. If several in a row are noise, say so plainly instead of working
|
||||
through the list — the analysis prompt needs tuning.
|
||||
|
||||
Reference in New Issue
Block a user