Repository navigation
feat: index direct task and steering instructions - #154
andrewesweet wants to merge 27 commits into
Conversation
 [Medium impact] The PR appears safe to merge, with two non-blocking concerns remaining from the previous review. Reviews (13) · Last reviewed commit: "no-mistakes(review): Align evidence cach..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export const TASK_AUTHORITY = "direct-task"; | ||
| export const STEERING_AUTHORITY = "direct-steering"; | ||
|
|
||
| const FENCE = /^\s*(`{3,}|~{3,})([^\n]*)$/; |
There was a problem hiding this comment.
| const FENCE = /^\s*(`{3,}|~{3,})([^\n]*)$/; | |
| const FENCE = /^ {0,3}(`{3,}|~{3,})([^\n]*)$/; |
andrewesweet
changed the title
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
Speaking as Kun's firstmate: reviewed this PR against main Diff verdict: safe for the intended scope. Adds always-on VISION per-rule:
contract-class: new-default (independent of contributor claim) — main's unconfigured analysis path has no directive index and CI: fork workflows approved (attempt 2). All required checks green (CI ubuntu+macos, Guard, no-mistakes). Greptile ignored. mergeable/ready on checks. Hold: this is waiting on captain, not on the author. Auto-merge is blocked solely by the new-default decision. Flagging for Firstmate/captain merge-or-not. |
|
Speaking as Kun's firstmate: stale-restamp pass. Thanks for your patience on this one, @andrewesweet. The head is still The earlier per-rule VISION verdict still stands: everything aligns. The contract-class is unchanged: new-default, because every unconfigured analyze run gets the always-on directive index and the |
andrewesweet
changed the title
|
The branch was updated by merging current main (release 0.1.32) into it as a true The remaining Greptile P2 suggestion ("Indented code changes authority") is Live evidence for the directive index, built without a model call by reading one The session has 9 user turns and was not elided; its long first turn was clamped |
|
Speaking as Kun's firstmate: thanks @andrewesweet, both blockers from the last pass are cleared.
I reviewed the commits added since VISION per-rule (re-checked against the current VISION.md and this head):
contract-class: new-default. Main's unconfigured analyze path has no directive index. This head always builds the index and bumps Nothing is left for you to do right now. This is waiting on Kun's merge-or-not call on that default change, and I'm flagging it to him now. |
andrewesweet
changed the title
| The user's own task message (TASK-1) and later steering (STEER-N) are addressable | ||
| instruction sources with an authority of their own, separate from the memory file. | ||
| Each entry points at a user turn already in the trace below - read the turn there; | ||
| the text is not repeated here. |
There was a problem hiding this comment.
carveEnvelope excludes fenced blocks and quoted lines, but this prompt points the model at the full user turn without explaining that boundary. A turn containing a real request plus a quoted command can therefore produce false claims that the quoted command was part of TASK-1.
Tell the model explicitly that these IDs cover only unquoted text outside fenced blocks.
| The user's own task message (TASK-1) and later steering (STEER-N) are addressable | |
| instruction sources with an authority of their own, separate from the memory file. | |
| Each entry points at a user turn already in the trace below - read the turn there; | |
| the text is not repeated here. | |
| The user's own task message (TASK-1) and later steering (STEER-N) are addressable | |
| instruction sources with an authority of their own, separate from the memory file. | |
| Each entry points at a user turn already in the trace below - read the turn there; | |
| the text is not repeated here. Only unquoted text outside fenced blocks carries | |
| instruction authority. Fenced blocks and lines beginning with `>` are pasted evidence, | |
| not instructions covered by that turn's TASK-1 or STEER-N id. |
|
Speaking as Kun's firstmate: thanks @andrewesweet for keeping this current. What changed since the last pass ( State: VISION per-rule (VISION.md unchanged since the last pass, re-checked against this head):
contract-class: new-default, unchanged. Main's unconfigured analyze path has no directive index. This head always builds one, preserves message whitespace for every typed-block adapter, and bumps the analysis index version, which invalidates every cached analysis. Greptile's newest P2 (spell out in the prompt that only unquoted text outside fenced blocks carries Nothing is waiting on you. This is still a captain-decision hold: Kun decides whether to merge this default change. |
e9a1b2f to
f1d1a3c
Compare
andrewesweet
changed the title
|
Rebase-only refresh onto current main validated: the branch was rebased onto upstream Test evidence note for this run: in the live validation pass, 5 of 6 scenarios were driven live against the product. The model-backed analysis scenario (issued directive citations persist with metadata only, invented IDs are rejected, and updated evidence is reusable) was not driven live in this run — the validation lane's isolated model auth blocked it, and the inconclusive verdict was accepted as-is rather than adding model calls or spend. The live evidence for that scenario remains the pi-session directive index posted earlier in this thread (the |
andrewesweet
changed the title
|
Fix-round validation for the list-item-fence finding above: the review step caught that the first cut let marker-prefixed lines close a paste; closing fences now require a whitespace-only prefix, Test evidence note, same ruling as the previous run: 7 of 8 scenarios were driven live against the product. The authenticated model-backed analysis scenario (issued directive citations persist with metadata only, invented IDs rejected, unsupported quotes rejected) was not driven live in this run — the validation lane's isolated model auth blocks it and the inconclusive verdict was accepted as-is rather than authorising model spend. The live evidence for directive citations remains the pi-session directive index posted earlier in this thread (the |
|
Speaking as Kun's firstmate: thanks @andrewesweet for the rebase and the list-item fence fix. What changed since the last pass ( State: VISION per-rule (VISION.md unchanged, re-checked against this head):
contract-class: new-default, unchanged. Main's unconfigured analyze path has no directive index. This head always builds one and bumps the analysis index version, which invalidates every cached analysis. Nothing is waiting on you. This is still a captain-decision hold: Kun decides whether to merge this default change. |
e332d93 to
3293ea0
Compare
andrewesweet
changed the title
|
Speaking as Kun's firstmate: thanks @andrewesweet for the quick fix on the list-item blockquote case. What changed since the last pass ( State: VISION per-rule (VISION.md unchanged, re-checked against this head):
contract-class: new-default, unchanged. Main's unconfigured analyze path has no directive index. This head always builds one and bumps the analysis index version, which invalidates every cached analysis. Nothing is waiting on you. This is still a captain-decision hold: Kun decides whether to merge this default change. |
andrewesweet
changed the title
|
Speaking as Kun's firstmate: the only change since the last pass ( Nothing is needed from you, @andrewesweet. This is still a captain-decision hold: indexing direct task and steering instructions as sources changes what an unconfigured run treats as evidence, so it's waiting on Kun's call, not on you. |
|
Speaking as Kun's firstmate: the only change since the last pass ( Nothing is needed from you, @andrewesweet. This is still a captain-decision hold: indexing direct task and steering instructions as sources changes what an unconfigured run treats as evidence, so it's waiting on Kun's call, not on you. |
…e spans The initial task message and later user steering become addressable instruction units (TASK-1 / STEER-N) with source kind, turn, a direct-task/direct-steering authority tier, and a lifetime closed only by an explicit revision signal. Spans render in the analysis prompt by turn reference, never by duplicating turn text; only the authoritative envelope of a user message indexes (fenced blocks and blockquote lines are structurally excluded, assistant/tool text never is). The fold validates cites against the spans each session issued and renders them apart from memory-instruction rows. ANALYSIS_INDEX_VERSION 3 -> 4.
…persede heuristic
…ces; format directives
…ay "see turn N in the trace" only when that turn's text actually reached the trace. Two ways it cannot: capTrace elision (already handled) and per-message clampMessage, which was unhandled because the envelope is now carved pre-clamp. Changes: - `src/distill.js`: each message turn records `clamped: text !== redacted.trim()` — true exactly when clampMessage dropped a middle from the text the model reads. Whole-turn flag, matching the existing whole-turn `elided` flag (same conservatism as the tail-boundary case). - `src/directives.js`: spans carry `clamped`; `renderDirectiveIndex` renders `turn N - clamped from the trace, open the raw transcript for its text` (after the elided branch, before the pointer branch). Header/JSDoc comments updated. No other consumer touched: `directiveMetadata` still persists only ids/turn/authority/lifetime. - `test/directives.test.js`: new test — a user message over the 6,000-char clamp whose only instruction sits in the clamped middle yields `spans[0].clamped === true`, renders the clamped entry, and never renders "see turn 1 in the trace"; also asserts the trace itself lacks the instruction. ci-4 out of scope per instruction. `pnpm run check` exits 0 (711 tests pass, lint/format/typecheck clean)
The branch already bounds fence indentation relative to the opener, which is
what the indented-code review note asked for while keeping list-item fences
working; an absolute ^ {0,3} form would regress those. Add the missing boundary
test: a closer three columns past the opener closes, four past it does not.
Join typed-block texts without trimming so indented fence markers survive into the carved envelope; whitespace-only blocks stay omitted via the non-empty check. New adapter-level regression test plus fixture. Bump ANALYSIS_INDEX_VERSION 5->6: preserved leading/trailing whitespace changes what analysis accepts.
…nvelope Greptile P2 on PR 154 (comment 4322029695): carveEnvelope missed fence openers behind a list marker, so a pasted '- ```sh' block stayed instruction text and its indented closer opened a phantom fence that swallowed every later instruction. The opener now matches after an optional bullet (-, *, +) or ordered marker (digits with . or )) plus spaces, and the closer indent is measured from the fence's own column after that marker. Regression tests cover the comment's exact case and the ordered-list variant. Bump ANALYSIS_INDEX_VERSION to 7 for the changed envelope boundary.
…elope A blockquote behind a single list marker (- > x, * > x, 1. > x) passed the quote check and was indexed as instruction text instead of staying evidence, so a paste-only turn could receive TASK-1. The QUOTE check now allows the same optional single marker the fence check accepts; markers require whitespace after them, so an unspaced dash keeps its old behaviour. Bump ANALYSIS_INDEX_VERSION to 9 so cached analysis accepted under the old carve is invalidated.
…ess only after installing its SIGTERM handler. Reproduced CI’s exact failure with a forced scheduling delay; same probe passes after fix. All 43 harness/subprocess tests pass locally. Temporary probe removed
4e1916e to
c8714af
Compare
|
Speaking as Kun's firstmate: thanks @andrewesweet for the new round. What changed since the last pass ( Nothing is needed from you. This is still a captain-decision hold: indexing direct task and steering instructions as sources changes what an unconfigured run treats as evidence, so it's waiting on Kun's call, not on you. The VISION per-rule verdict from earlier passes stands (aligned on evidence-is-currency and enforced-in-code; the open question is only the default change). Contract class: new-default. |
Intent
Backpass indexes only project memory files as instruction sources, yet in measured sessions nearly half of the instructions that governed agent behaviour came from the initial task message or later user steering, which Backpass shows only as undifferentiated transcript evidence. This change makes direct task and steering instructions addressable as instruction sources with stable IDs and source, turn, authority, and lifetime metadata, referencing transcript text already sent rather than duplicating it, while keeping quoted transcripts, tool results, and assistant restatements as evidence rather than authority.
What Changed
TASK-1andSTEER-<turn>sources with authority and lifetime metadata, referencing existing trace text without duplication.Risk Assessment
✅ Low: The changes preserve the approved provenance boundaries and cache invalidation, with no new material defects found across the affected flows.
Testing
Focused executable tests and isolated real CLI/model runs passed all eight scenarios. Setup lock, permission and adapter issues were resolved. CLI transcripts, generated prompts and persisted outputs were captured; fixtures were removed. No rendered UI changed, so screenshots were unnecessary.
Evidence: Generated instruction indexes
Evidence: Real model directive citations
Evidence: Persisted partial-transcript metadata
Evidence: Version 10 cache reuse
Evidence: Version 9 cache invalidation
Evidence: Aggregated evidence
Evidence: Generated proposal
Evidence: Validation setup and results
Pipeline
Updates from git push no-mistakes
✅ **Intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
src/directives.js:122- Intent requires the “initial task message” and “stable IDs,” but this hunk assigns TASK-1 to the first retained user turn. For logs over 64 MiB, src/discovery/adapters/shared.js:47 discards the prefix. An initial task followed by enough tool output and later steering therefore makes that steering TASK-1 with direct-task authority. Further appends can shift retained STEER-N IDs. Affected callers: src/discovery/adapters/claude.js:86, src/discovery/adapters/codex.js:105, src/discovery/adapters/pi.js:229, src/discovery/adapters/grok.js:64. Related changed sites: src/distill.js:127 restarts numbering; src/directives.js:124 derives steering IDs, :130 derives lifetimes; src/analyze.js:358 persists the incorrect metadata. Decide how partial transcripts must satisfy provenance requirements, then preserve original task identity and turn positions at the shared reader/adapter boundary.🔧 Fix applied.
1 error still open:
test/config.test.js:241- Fix round 1 (c604517) bumped ANALYSIS_INDEX_VERSION to 10 at src/state.js:251 but left this test requiring v9. Assertions at test/config.test.js:248–249 necessarily fail. Update the literal oracle to reject v9 evidence and accept v10; update the title at :237 and explanation at :246 accordingly.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
node bin/backpass.js --helpnode --import ./test/helpers/hermetic-config.js --test test/directives.test.jsnode --import ./test/helpers/hermetic-config.js --test --test-name-pattern='evidence accepted before the partial-transcript fix' test/config.test.jsnode --import ./test/helpers/hermetic-config.js --test --test-name-pattern='a quote that is not in the distilled trace|sanitizeEvidence accepts issued directive ids and drops the rest' test/distill.test.js test/directives.test.jsnode .test-provenance/live.mjs: isolated CLI scan, analysis prompt checks and appended-session checksnode .test-provenance/model-check.mjsandnode .test-provenance/inspect-model.mjs: real model startup and permission diagnosticsnode .test-provenance/citations.mjs: real model citations, persisted metadata, cache reuse and invalidationnode .test-provenance/partial-model.mjs: real model analysis of a truncated lognode .test-provenance/fold-cli.mjs: real CLI proposal generation and persisted aggregation checksArchived drivers and product outputs in the evidence directory; removed disposable fixtures; confirmed cleangit status --shortandgit diff --exit-code.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.