Skip to content

fix(outcome): attribute capture to the exercised skill, not brigade-work - #647

Merged
solomonneas merged 2 commits into
mainfrom
fix/628-artifact-id-signals
Aug 1, 2026
Merged

fix(outcome): attribute capture to the exercised skill, not brigade-work#647
solomonneas merged 2 commits into
mainfrom
fix/628-artifact-id-signals

Conversation

@solomonneas

@solomonneas solomonneas commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Decision for outcome: generic brigade-work artifact id collapses distinct skill signals #628: hard-coded brigade-work is a defect. Distinct skills should accumulate their own signals; brigade-work remains the fallback when no artifact is known.
  • Verify receipts stamp outcome capture intent. Retry guidance uses the failed receipt stamp before the current --capture value.
  • Claude hooks record a skill only when its id is installed in the target. Reads of reference skills outside the target do not affect attribution.
  • Automatic card attribution is intentionally out of scope for this PR. Explicit card captures remain supported; automatic attribution is limited to installed target skills.
  • Existing append-only outcome rows are unchanged.

Closes #628.

Verify receipt

20260801-200756-work-verify-fce43d

brigade work verify run --target . --command "./scripts/verify" --capture brigade-work

Acceptance criteria to tests

Criterion Test
Distinct skills accumulate separate rank signals test_distinct_skill_captures_accumulate_separate_rank_signals
Retry uses the failed receipt stamp over a current capture test_capture_before_retry_prefers_receipt_stamp_over_current_capture
Retry uses a receipt-stamped artifact test_capture_before_retry_uses_receipt_stamped_artifact
Retry uses current --capture when the receipt is unstamped test_capture_before_retry_uses_current_capture_when_receipt_unstamped
Unknown skill falls back to brigade-work test_capture_before_retry_falls_back_to_brigade_work
External SKILL.md reads do not set attribution test_posttooluse_read_external_skill_does_not_set_exercised_artifact
Installed SKILL.md reads set attribution test_posttooluse_read_skill_sets_exercised_artifact

Test plan

  • Full project verification
  • Rebased onto origin/main
  • External grader review before merge

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • coderabbit-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d5436bbb-ccbd-4d53-9e58-4a52a23a8950

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@solomonneas

Copy link
Copy Markdown
Collaborator Author

Grader review: APPROVE-WITH-CHANGES. Attribution mechanism and backward compatibility are sound; ledger rows untouched. Required before merge:

  1. Rebase: branch is CONFLICTING with main - fix(outcome): dedupe reused verify receipts via reused_from #650 rewrote _verify_receipt_has_outcome_capture in the same verification.py region.
  2. Priority inversion in _enforce_capture_before_retry: resolve_capture_artifact_id puts the CURRENT run's --capture ahead of the failed receipt's own stamp, so retrying skill B's verify after skill A's failed run cross-attributes A's hurt signal to B. The receipt stamp is ground truth for that run; swap the order and add the both-present conflict test (existing tests only cover one-source cases).
  3. Constrain the last-SKILL.md-read heuristic: the path matcher accepts any */skills//SKILL.md anywhere on disk, including other repos and meta-skills browsed for reference. Constrain to artifacts installed in the target (capture already has _artifact_known) or explicitly document last-read-wins and filter meta-skills.

Also state in the PR body whether card attribution (issue #628 says 'skills or cards') is intentionally out of scope. Ping for re-grade after push.

solomonneas and others added 2 commits August 1, 2026 16:05
Hard-coded brigade-work capture ids in hook replacements and
capture-before-retry messages collapsed distinct skill signals into one
rank bucket. Stamp capture intent on verify receipts, track the exercised
skill in Claude session state, and fall back to brigade-work only when
unknown.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-Authored-By: Codex <codex@openai.com>
@solomonneas

Copy link
Copy Markdown
Collaborator Author

Rebased onto origin/main in 6fce03d.

a561695 maps the grader changes as follows:

  • Retry attribution now prioritizes the failed receipt outcome_capture artifact id over the current --capture value. Covered by test_capture_before_retry_prefers_receipt_stamp_over_current_capture.
  • Read-path attribution now requires an installed target skill. Covered by test_posttooluse_read_external_skill_does_not_set_exercised_artifact.

Card attribution scope is stated in the PR body: automatic card attribution is out of scope; explicit card capture remains supported.

Full verification passed: ./scripts/verify, receipt 20260801-200756-work-verify-fce43d.

@solomonneas
solomonneas force-pushed the fix/628-artifact-id-signals branch from f41416a to a561695 Compare August 1, 2026 20:18
@solomonneas

Copy link
Copy Markdown
Collaborator Author

Grader: APPROVE, merging. All three required changes verified: the branch is rebased (6fce03d), the priority inversion is corrected so the failed receipt's own outcome_capture stamp wins over the current --capture value (test_capture_before_retry_prefers_receipt_stamp_over_current_capture covers the both-present conflict that the earlier tests missed), and the read-path heuristic now requires an installed target skill (test_posttooluse_read_external_skill_does_not_set_exercised_artifact). Card attribution scope is stated explicitly in the PR body rather than left ambiguous. Receipt 20260801-200756-work-verify-fce43d, CI green.

@solomonneas
solomonneas merged commit 4f89436 into main Aug 1, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

outcome: generic brigade-work artifact id collapses distinct skill signals

1 participant