Repository navigation
Conversation
 [Medium impact] The PR appears safe to merge; no new actionable issue was found. Reviews (5) · Last reviewed commit: "no-mistakes(document): Qualify README re..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Speaking as Kun's firstmate: Thanks @Pantoria, this is solid work. I reviewed the diff against main: CI is green on both OSes, the no-mistakes attestation is bound to the current head ( contract-class: new-default (only for one part). Keeping a redacted head/tail, VISION (per rule):
|
8d68e6f to
214227c
Compare
|
Speaking as Kun's firstmate: Thanks @Pantoria. I re-reviewed the new head contract-class: still new-default (only for one part), so the hold is unchanged. The redacted, classed reply trace and the per-file VISION (per rule): the same verdicts as my last stamp: aligns on all seven. The new nested-store reporting strengthens "Failure is loud and named", because nested give-ups no longer go unseen. |
214227c to
38e4621
Compare
|
Speaking as Kun's firstmate: Thanks @Pantoria. I re-reviewed the new head contract-class: still new-default (for one part), so the hold hasn't changed. The redacted, classed reply trace and the per-file VISION (per rule): aligns on all seven, same as my last stamp. Evidence is the only currency (skipped records stay |
…trying them forever An analysis reply with no parseable JSON now records, in its failed evidence record, the reply's length, its redacted first and last 300 characters, and a class judged from the text alone: empty, truncated, unparsed, or prose. acpx reports no stop reason in quiet mode, so none is recorded. After 3 unparseable replies in a row for the same evidence key and analysis call (agent, model, effort, prompt template), the transcript is skipped and named in the run summary and `backpass status`, keeping its last reply trace. A changed key or --force analyzes it again; other failures reset the streak. A skipped transcript stays `failed` and never counts as evidence. Fixes kunchenguid#193 Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ed. The broken rule: `backpass status` has to report every evidence store that analysis writes. Before this fix it read only the root store, so failures, reply classes and give-ups recorded for a nested memory file never appeared. Changes: - src/nested.js: added `layerNestedWeights(rootFile, weights, skills)`. It holds the ancestor and `nestedSurfaceHash` logic that was previously inline in `primaryMemoryFile`. - src/commands/analyze.js: now calls that helper, so analyze and status build each nested file's hash from the same code. - src/commands/status.js: builds one list of stores: the root store plus one per trained nested file (it exists, isn't a pointer, and its state is read from `nestedStateDir` without creating anything). It checks each record against that file's own memory hash. The changes show up in four places: - Evidence counts and the "skipped after N" note now include nested stores. - The failed-transcripts table gets a MEMORY column, but only when nested files are configured, so root-only output is unchanged. - The note about where reply traces are kept points to each file's own store. - `--json` `evidence` has a new `files` array of `{path, ok, failed, skipped, gaveUp}`, one entry per memory file. - README.md (line 416): one clause saying status also reads nested evidence and names the file. - test/analyze-failure-trace.test.js: the fake acpx can now give a nested file its own reply, and the harness can set up a nested `apps/api/AGENTS.md`. The new test runs the real CLI: the root analysis succeeds, the nested one gives up after 3 replies, and status text and JSON show which file it was (`1 ok · 0 skipped · 1 failed (1 skipped after 3 failures)`, an `apps/api/AGENTS.md` row marked skipped, and per-file JSON counts). Editing the nested file turns the row back to `retry`. I checked that this test fails against the old status.js and passes with the fix. Verification: `pnpm run check` passes (lint, format, typecheck, and all 937 tests)
38e4621 to
3de5215
Compare
|
Speaking as Kun's firstmate: thanks @Pantoria for rebasing. I re-reviewed the new head contract-class: still new-default (for one part). The redacted, classed reply trace and the per-file VISION (per rule, main
This is a captain-decision hold with Kun, not waiting on you. There's nothing for you to do right now. |
Intent
Fix issue #193: when analysis output had no parseable JSON, the evidence record kept only status failed plus the error string, dropping the reply, and the transcript was re-analyzed every run forever. The user (issue author, Pantoria) asked to implement their own ready-for-pr backpass issues, each in its own worktree and PR. Follow AGENTS.md invariants and VISION.md. No live model calls were made while implementing.
Implemented all three proposal parts. (1) Failed records keep reply {class, chars, head, tail}: the whole reply is passed through the existing redact() before cutting 300-char head/tail (so a secret can't be split at a snippet edge); chars is measured by backpass from the raw reply. No harness stop reason is stored because acpx runs --format quiet and exposes none - deliberately not invented (never trust/invent model numbers). (2) A deterministic class empty/prose/truncated/unparsed (rules documented in classifyUnparsedReply) shows in status and in the error text. empty is unreachable today because assertNonEmptyOutput fires first, kept for completeness (open PR #183 may change that path). (3) Skip-after-N, which the maintainer allowed only if named in status and the last trace kept: MAX_UNPARSED_REPLIES = 3 constant (deliberately no config option), counted per failureKey = evidence key + answering agent/model/effort + analysis prompt hash, so any change resets; only unparseable replies count (timeouts/exec failures reset and always retry so a flaky provider can't make backpass give up); --force re-analyzes. Skips are named in a run warning, summary suffix, summary.gaveUp in --json, and status text/JSON. Skipped records stay failed, so never count as evidence. No ANALYSIS_INDEX_VERSION bump: only failure records change, not what analysis accepts; legacy failed records still load.
What Changed
reply: {class, chars, head, tail}. The whole reply goes throughredact()before the 300-char head and tail are cut, andcharsis measured from the raw reply.classifyUnparsedReplydecides the class (empty,prose,truncatedorunparsed) from the reply text alone, and the class also appears in the error text.MAX_UNPARSED_REPLIES(3) unparseable replies in a row with the same failure key. The key covers evidence key, answering agent, model, effort and analysis prompt hash. Timeouts and exec failures reset the count. Changing any part of the key, or passing--force, analyzes the transcript again. Skipped records stayfailed, so they never count as evidence. The skip is reported in a run warning, as askipped after 3 failuressuffix on the analyze/run/bootstrap evidence summary lines, and assummary.gaveUpin--json.backpass statusnow reads the root evidence store and the store of each trained nested memory file. SharedlayerNestedWeightslogic insrc/nested.jsgives status and analyze the same nested-file hashes. Status shows reply classes, give-ups and a MEMORY column for failed transcripts (only when nested files are configured), and--jsonevidencegains a per-filefilesarray. README and--forcehelp text are updated, andtest/analyze-failure-trace.test.jsadds end-to-end coverage.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The change only touches failed-record bookkeeping and status reporting; given-up records stay
failedand never count as evidence. Analyze and status now compute the root and nested hashes with the same shared helper, and the user decisions from the earlier fix rounds (a current-surface check for give-ups, nested evidence stores in status) are implemented correctly.Testing
Ran the targeted end-to-end test file, which runs the real CLI with a fake acpx (all 9 pass). Then drove backpass by hand in a throwaway repo and HOME. A secret placed across the snippet edge is redacted before the cut and never stored. A truncated JSON reply is classed
truncatedand a prose replyprose, each with a character count backpass measures itself. After 3 prose replies the next run makes no model call and names the skip in the warning, the summary, and status text and JSON. --force calls the model again, and a timeout resets the count. One informational note: an acpx exit 1 with a pinned agent aborts the whole run, as it did before this change. That writes no record, so the streak is neither reset nor counted. It is not a regression. No UI surface, so no screenshots.Evidence: CLI transcript of disposable end-to-end runs
Evidence: Driver script for the manual runs
Evidence: Targeted e2e test output
Evidence: status after give-up
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
src/commands/status.js:241-isGivenUpin status only checksunparsedReplies >= MAX_UNPARSED_REPLIES. It never checks that the record still matches the current evidence key. Here is a sequence that happens in normal use: a transcript gives up at memory hash H1, thenbackpass applyedits AGENTS.md so the memory surface becomes H2.backpass statusstill shows the record asskipped after 3 failures: in the NEXT RUN column, in the evidence summary suffix (status.js:147) and inevidence.gaveUpin --json (status.js:84). But the next run will retry it, becausereplyFailureKeyincludesevidenceKey(..., H2)and the streak resets (hasGivenUpin src/analyze.js). The label is wrong but no error is raised. The comment at status.js:236-240 accepts this for the agent dimension, since status does not resolve agents. The memory-surface and transcript-content parts of the key can be checked cheaply, though: status already resolves the memory files for its budgets. Smallest fix: treat a record as given up only whenrecord.memoryHashequals the currentmemorySurfaceHashfor its memory path, and optionally whenrecord.keymatches the currentevidenceKey. Otherwise show it asretry. The same predicate feeds three places: status.js:31 (count), status.js:147 (text suffix) and status.js:181 (row label).🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ No issues found.
✅ No issues found.
✅ No issues found.
✅ No issues found.
node --test test/analyze-failure-trace.test.js: test 'status still lists an older failed record without the trace fields' drives the real CLInode --test test/analyze-failure-trace.test.js(8/8 pass; runs the real CLI end to end with a fake acpx)Manual CLI session in a throwaway HOME/repo with a fakepiand a fakeacpx(reply set from a file, each call logged): 3xbackpass analyze --harness pi --since all --analysis-agent pi --jobs 1, then inspected the evidence JSON and grepped it for the secret4thbackpass analyze(skip, model call count unchanged),--jsonsummary,backpass status,backpass status --jsonbackpass analyze --forceafter the skipEdited AGENTS.md, thenbackpass status(shows retry) andbackpass analyze(re-analyzes)Adversarial: 2 unparseable replies, then repeated acpx exit-3 timeouts: the count resets, no reply trace is kept, and every run still calls the modelRecovery: a parseable JSON reply turns the record into ok evidence; status --json reports gaveUp 0✅ No issues found.
node --test test/analyze-failure-trace.test.js(the 9 targeted tests for this change, which run the real CLI with a stub acpx)~/.no-mistakes/evidence/01M503V3YM500JB8EM5J3KKE5C/drive.shin a throwaway git repo and HOME with a nested apps/api/AGENTS.md:backpass analyzerun repeatedly,backpass status,backpass status --json,analyze --force, editing both memory files, a valid reply, and 4 timeout runs✅ No issues found.
node --test test/analyze-failure-trace.test.js(9/9 pass; the CLI-level tests spawn bin/backpass.js against a fake acpx)Manual: 4 consecutivebackpass analyze --harness pi --since all --analysis-agent pi --jobs 1runs against a throwaway repo with a root and a nested AGENTS.md; model-call count stays at 6 on run 4Manual: inspected .backpass/evidence/*.json for reply {class, chars, head, tail} and unparsedReplies;grep -r sk-ant-abc .backpassfound no secretManual:backpass statusandbackpass status --jsonafter give-up (MEMORY column, both files 'skipped after 3 failures', evidence.files per-file counts)Manual:backpass analyze --forcere-calls the model for skipped transcriptsManual: appended a line to nested apps/api/AGENTS.md, thenbackpass statusshows that row asretrywhile the root row stays skippedManual: acpx exit 3 (timeout) with --force leaves a record with no reply trace and no counter; the next prose reply counts as unparsedReplies 1Manual: memory edit plus a valid JSON reply gives an ok record, summary gaveUp 0ℹ️
src/analyze.js- With a pinned analysis agent, an acpx exit 1 aborts the whole run ('pinned analysis agent pi failed unexpectedly'). This is pre-existing behaviour. Because no per-transcript record is written, the unparseable streak is neither reset nor counted: prose, prose, exit 1, then prose gives up after the third prose reply. This matches 'only unparseable replies count' but not a literal reading of 'exec failures reset'. Per-transcript timeouts do reset, as tested.Live validation: ✅ go - 9 of 9 scenarios driven live against the product
node --test test/analyze-failure-trace.test.js(9/9 pass; it runs the real CLI with a fake acpx and covers classing, redaction, the 3-strike skip and its reset by --force / model / memory change, status give-up only at the current memory surface, the nested store with a MEMORY column and per-file JSON, the timeout reset, and old-format records)bash evidence/drive.sh <worktree>: manual CLI run in a disposable HOME and repo with a fake acpx and pi; runsbackpass analyze --harness pi --since all --analysis-agent pi --jobs 1repeatedly, plusbackpass status,backpass status --json,--force, and an acpx exit 1 partway through a streakAdversarial: secret straddling the 300-char head edge (API_KEY=sk-...at offset 285); checked that the persisted evidence JSON never contains the secretRead the persisted.backpass/evidence/*.jsonfailure records (reply.class/chars/head/tail, unparsedReplies, status)✅ **Document** - passed
✅ No issues found.
✅ No issues found.
✅ No issues found.
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ No issues found.
✅ No issues found.
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
✅ No issues found.
✅ No issues found.
✅ No issues found.