-
Notifications
You must be signed in to change notification settings - Fork 1
review: value-ranked shedding and dispatch; never shed reviewer requests #260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0afc12e
a31b940
fd3ad8e
1e7b4f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "review": minor | ||
| --- | ||
|
|
||
| Value-ranked shedding and dispatch. The graceful-landing rule's first shed bucket previously said "skip any not-yet-dispatched opt-in reviewers and lenses" with no ordering, so a budget-pressed run shed arbitrarily; the 2026-07-10 production runs showed the consequence (an invocation cap smaller than the roster filled its slots by enablement mechanism, not value, and a matched specialist lens could be shed to afford `conventions`). The shed order is now explicit, lowest value first: conventions, first-principles, holistic, completeness/test-adequacy, and path-triggered specialist lenses last, since a matched lens is the most targeted signal in the run. The same ranking read from the other end (defaults, lenses, targeted opt-ins, generic opt-ins) is now the mandated dispatch order when `maxReviewerInvocations` cannot fit the full roster, with every undispatched reviewer recorded as a planned shed. And reviewer requests (Step 8) are removed from the shed list entirely: pulling a human in matters most on exactly the run whose own coverage is partial. The ranking is a first-cut editorial ordering; replace it with measured per-dimension must-catch contribution from the eval corpus when that data is compiled. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -570,7 +570,11 @@ either run by default; a reviewer earns its `enable` line through the eval suite | |
| not by shipping). Dispatch the default reviewers (`correctness-reviewer`, | ||
| `skill-auditor`, `thread-reconciler`) **plus** every reviewer named in | ||
| `enabledReviewers` **plus** every lens named in `lensesToSpawn`, all **in parallel** | ||
| (one turn), and wait for all. | ||
| (one turn), and wait for all. If `runBudget.maxReviewerInvocations` cannot fit | ||
| that whole set, fill the slots by the dispatch ranking (the budget rule below: | ||
| Step 3, graceful-landing bucket 1): defaults first, then matched lenses, then | ||
| the targeted opt-in dimensions, then the generic ones. Never choose arbitrarily, and record every | ||
| reviewer left undispatched as a planned shed (Step 6 note). | ||
|
|
||
| **One candidate contract.** Every finding-producing reviewer returns `findings[]` | ||
| in the same shape (a `label` per finding, from the fixed label set in Step 4); a | ||
|
|
@@ -799,10 +803,23 @@ When any proxy passes roughly three-quarters of its soft target (or the trajecto | |
| is clearly expensive), stop starting new work and shed remaining work in this | ||
| order: | ||
|
|
||
| 1. Skip any not-yet-dispatched opt-in reviewers and specialist lenses; each becomes | ||
| a skipped dimension (Step 6 note). | ||
| 2. Skip the risks/patterns comment and reviewer requests (Steps 7-8) if they have | ||
| not happened yet. | ||
| 1. Skip not-yet-dispatched opt-in reviewers and specialist lenses in value | ||
| order, lowest value first; each becomes a skipped dimension (Step 6 note). | ||
| The ranking, from first-shed to last-shed: `conventions`, then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note (non-blocking): This ranking hard-codes the five current opt-in reviewers by name with no fallback position for a future one. A newly added opt-in reviewer would be unranked here, quietly reintroducing the arbitrary shed this change removes — until someone remembers to edit this prose. Consider stating where an unranked reviewer sheds (e.g. after |
||
| `first-principles`, then `holistic`, then `completeness` and | ||
|
khan-actions-bot marked this conversation as resolved.
|
||
| `test-adequacy`, and only then any path-triggered specialist lens from | ||
| `lensesToSpawn`. A matched lens is the most targeted signal in the run (the | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thought (non-blocking): "A matched lens ... outranks every generic dimension" treats a path-glob match as proof of value, but |
||
| router chose it for the specific files this PR touches), so it outranks | ||
| every generic dimension; shedding `security-auth` on an auth-path diff to | ||
| afford `conventions` is exactly backwards. This same ranking, read from the | ||
| other end (defaults, lenses, targeted opt-ins, generic opt-ins), is the | ||
| dispatch order when the invocation cap cannot fit the roster (Phase 2). | ||
| The interior order is a first-cut editorial ranking; replace it with | ||
| measured per-dimension must-catch contribution once the eval corpus | ||
| yields that data. | ||
| 2. Skip the risks/patterns comment (Step 7) if it has not happened yet. | ||
| Reviewer requests (Step 8) are **never** shed: pulling a human in matters | ||
| most on exactly the run whose own coverage is partial. | ||
| 3. Last, and never at the soft targets alone: the `claim-validator`. It is the | ||
| false-positive gate, and its cost scales with the candidate count (which you | ||
| can already see when deciding), not with the diff, so validating a small | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (non-blocking): The dispatch order here — "the targeted opt-in dimensions, then the generic ones" — is offered as bucket 1's shed ranking read from the other end, but bucket 1 spells out a five-item order (
conventions<first-principles<holistic<completeness/test-adequacy< lenses) while this side collapses the non-lens opt-ins into two tiers that are never bound to named dimensions. On a budget-pressed run that fits only some of {holistic,first-principles,conventions}, the dispatch rule can't say which to keep even though the shed rule can — the same "choose arbitrarily" gap this PR closes on the shed side. Consider stating the dispatch tiers at bucket 1's granularity: