Repository navigation
Add backlog notes for agent-first planning and review workflow - #20
Merged
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds six new METHOD backlog process documents defining contracts for: generated-reference sync decoupling, typed frontmatter access, backlog query surface, design-doc template catalog, next-work recommendation menu, and review closeout helper. Documentation-only; no code or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/method/backlog/bad-code/PROCESS_generated-reference-sync-coupling.md`:
- Around line 7-12: Update the acceptance_criteria list in
PROCESS_generated-reference-sync-coupling.md so all items use descriptive
contract language: change the fourth item currently phrased as "The slice
includes regression coverage proving a scoped reference-doc refresh leaves
BEARING and CHANGELOG unchanged on branch-local runs." to a descriptive
requirement (e.g., "Regression coverage demonstrates that a scoped reference-doc
refresh does not change BEARING or CHANGELOG on branch-local runs") so it
matches the style of the other criteria; ensure the text references the same
artifacts ("BEARING" and "CHANGELOG") and the same intent but avoids prescribing
implementation details.
- Around line 34-44: Clarify the contract: change the ambiguous phrase around
"add or define a narrower reference-doc generation path" to explicitly state
whether this PR introduces a new CLI/command (e.g., a new "sync-ref" CLI
subcommand) or merely documents existing behavior of an existing function/method
(e.g., method "sync ship"); replace "should" with MUST where scope visibility is
required (or explicitly mark it OPTIONAL if not required); replace "the command
or code path" with the concrete surfaces this applies to (e.g., CLI command
"sync-ref" and TypeScript function "generateReferenceDocs"); and require that
the command output explicitly lists which targets were refreshed (examples:
docs/CLI.md, docs/MCP.md) while excluding ship-only files (docs/BEARING.md,
CHANGELOG.md).
In `@docs/method/backlog/bad-code/PROCESS_typed-frontmatter-access.md`:
- Around line 47-49: The proposed contract must be extended to define a
typed-write/merge API and migration rules: add a "Typed writes" subsection
specifying that a new function (e.g., a typed write/merge helper) will validate
input types against the frontmatter schema and either reject or require explicit
opt‑in for any operation that would downgrade a typed field (e.g., writing a
string over an array), clarify that updateFrontmatter remains available for
plain string-only fields but MUST NOT be used for schema-backed typed fields,
and require automated writers (e.g., GitHub adapter's pushItem) to be audited
and switched to the typed write API before rolling out typed reads; include
error semantics and a migration path for existing frontmatter values so callers
of updateFrontmatter, updateFrontmatterMerge, or pushItem know to opt into or
migrate to the new typed write/merge functions.
- Around line 41-43: Edit the sentence that reads "strings, booleans, numbers
when intentionally present, and arrays of strings..." to either remove the
ambiguous qualifier or replace it with a precise definition; for example,
specify exactly what "intentionally present" means (e.g., "numeric YAML scalars
(not stringified numbers) and non-null numeric values" or "numbers that pass the
project's numeric validation rules") or simply change to "numbers" if no
stricter contract exists; update the surrounding text in
PROCESS_typed-frontmatter-access.md to reflect the clarified rule so consumers
know whether JSON/YAML stringified numbers, nulls, or annotated types are
allowed.
In `@docs/method/backlog/cool-ideas/PROCESS_backlog-query-surface.md`:
- Around line 40-43: The doc currently uses ambiguous "bounded or paged" in the
Filtering / first slice description; replace that with a concrete contract:
specify that implementations MAY choose either (a) bounded results with a hard
limit (recommend max 100 items, default 50) and a clear statement of the limit,
or (b) cursor-based pagination (fields: page_size default 50, max 100; request
param limit and optional cursor; response includes items, next_cursor/null, and
total_count optional). Update the "Filtering" / "first slice" language to state
which approach is allowed and include the exact numeric limits and pagination
field names so callers know the API (e.g., page_size, cursor, next_cursor).
- Around line 36-39: The contract in PROCESS_backlog-query-surface.md requires
fields not present on the BacklogItem domain model (BacklogItem in
src/domain.ts), so update the doc to either (A) declare this as a prerequisite
and reference the typed frontmatter work (PROCESS_typed-frontmatter-access) and
state that BacklogItem must be extended, or (B) reduce the proposed contract to
only the existing fields (stem, lane, path, legend, slug), or (C) explicitly
specify the schema extension required (add title, priority, owner,
has_acceptance_criteria to BacklogItem) as part of this contract; also fix the
boolean wording to read "has_acceptance_criteria is true when the field exists
in frontmatter and false when it does not."
In `@docs/method/backlog/cool-ideas/PROCESS_design-doc-template-catalog.md`:
- Around line 50-53: Clarify enforcement for the "Must contain" lists by adding
an "Enforcement level" statement in the Proposed Contract section (before the
existing content under "Proposed Contract") that specifies whether these are
soft guidance (scaffold prompts; docs using contract-surface SHOULD address the
topics but can omit with reviewer rationale) or hard validators (map each "Must
contain" item to required section headings and describe a validation check that
verifies frontmatter declares the template type); reference the "Must contain"
phrase and the contract-surface template name so reviewers and any validation
tooling know which behavior to apply.
- Around line 115-117: The current scaffold used when no template is specified
does not enforce the required `default-change` sections; update renderDesignDoc
(src/renderers.ts -> renderDesignDoc) to emit a guaranteed scaffold containing
YAML frontmatter plus explicit headings and placeholders for "Intended behavior
/ contract", "Main happy path", "Expected failures / edge cases", and
"Verification plan" (in addition to the existing
title/legend/cycle/source_backlog and backlogBody), so that invoking method pull
without --template produces docs that conform to the `default-change` template
contract and reviewers can rely on the required sections being present.
In `@docs/method/backlog/cool-ideas/PROCESS_next-work-menu.md`:
- Around line 93-94: The phrase "meaningful maintenance debt" is ambiguous;
either define a concrete threshold or remove the qualifier — update the
PROCESS_next-work-menu text so that the rule either (a) specifies a clear,
measurable condition for when `bad-code` outranks `cool-ideas` (e.g., "when
there are X `bad-code` items", "when `bad-code` items exceed Y% of backlog", or
"when any `bad-code` item has priority `high`" and mention `BEARING` if it
affects behavior), or (b) remove the qualifier and state simply "`bad-code`
ranks above `cool-ideas` in the default lane hierarchy." Ensure you edit the
sentence referencing `bad-code`, `cool-ideas`, and `BEARING` accordingly.
- Around line 47-50: Update the PROCESS_next-work-menu.md contract to explicitly
acknowledge implementation prerequisites: state that typed frontmatter access
(refer to PROCESS_typed-frontmatter-access.md) and an extended BacklogItem
schema (fields like priority, owner, has_acceptance_criteria referenced in
src/domain.ts) must exist or be implemented as part of this work, and note that
the backlog query surface (PROCESS_backlog-query-surface.md) is
optional/reusable; add a short "Implementation Prerequisites" section before
"Repo-Truth Inputs" listing these dependencies and clarifying whether
next-work-menu will wait for them or include them in the same slice.
- Line 81: Line 81 declares score_band values but lacks semantics; update the
"Output Shape" section to define score_band behavior: state that score_band is a
relative ranking within the returned menu (not an absolute score), that the menu
MUST be sorted by score_band (highest → strong → worth-considering) and then by
lane precedence within each band, that multiple items may share the same band
(ties allowed) and bands are assigned by relative position (e.g., typical
allocation: top 1–2 items → "highest", next 2–3 → "strong", remainder →
"worth-considering"), and clarify that thresholds are relative to the current
menu and not fixed numeric cutoffs; reference the score_band values
("highest","strong","worth-considering") and the Output Shape/menu when adding
this text.
- Around line 89-94: The ranking text contains a contradiction about `asap` vs
`up-next`; rewrite the paragraph to present a clear precedence hierarchy instead
of two conflicting sentences: state that `asap` always outranks other lanes when
populated, `up-next` is next when `asap` is empty or overridden by `BEARING`,
then `bad-code` ranks after `up-next` unless `BEARING` explicitly points
elsewhere, and any remaining lanes follow; update the lines referencing `asap`,
`up-next`, `bad-code`, and `BEARING` to reflect this single, ordered rule set.
- Line 52: Replace the leaked absolute path in PROCESS_next-work-menu.md where
it currently shows [BEARING.md](/Users/james/git/method/docs/BEARING.md); update
the link target to a portable path (for example a repository-relative or
relative link such as [BEARING.md](/docs/BEARING.md) or
[BEARING.md](../BEARING.md) depending on repo structure) so the markdown
references BEARING.md instead of the local filesystem path.
In `@docs/method/backlog/cool-ideas/PROCESS_review-closeout-helper.md`:
- Around line 33-36: The doc's "Read-first design" describes read behavior but
omits the write contract—update PROCESS_review-closeout-helper to define the
write API surface (explicit flags/subcommands/interactive mode), specify whether
writes are batched or per-thread (and provide batching semantics), require
confirmation rules (prompts, --yes/--force, dry-run), enumerate exit codes for
write failures and their meanings, and state idempotency guarantees for
operations like replying, resolving threads, and posting rollup comments so
callers know retry behavior.
- Around line 44-45: Update the documentation for the round_summary array to
explicitly define how each outcome value (fixed, explained, left-open,
not-applicable) is determined: state whether outcomes are user-annotated,
LLM-determined, inferred from GitHub thread state or commit messages, or a
combination; specify the resolution precedence (e.g., user annotation overrides
agent inference), required evidence for each outcome (e.g., commit SHA, comment
IDs, LLM rationale), any automated heuristics or matching algorithms used
(briefly name functions or modules that perform inference if applicable), and
the audit metadata to be recorded (actor, timestamp, confidence score);
reference the round_summary field name and list the exact rules and fallbacks so
consumers can implement or validate automation reliably.
- Around line 53-55: The doc currently says a closeout "allow[s] a closeout
summary to associate one or more commit SHAs with each addressed thread" but
doesn't state who/what performs that mapping; update the "Commit mapping"
section to specify a single chosen mechanism (e.g., manual user mapping in the
UI, automatic heuristic matching in the tool, parsing structured markers in
commit messages, or AI-assisted suggestion with user confirmation). Explicitly
name the mechanism you choose (for example "manual mapping via the Closeout UI"
or "automatic heuristic matching by the Closeout engine"), describe the mapping
flow (who initiates it, whether suggestions are presented, and how conflicts are
resolved), and include any failure/edge-case behavior (e.g., when no match is
found the system prompts the user or leaves threads unmapped). Ensure you
reference the "closeout summary" and "commit SHAs" terms so readers can locate
the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 87785207-6f08-4f3b-bbab-dd7913cec3ca
📒 Files selected for processing (6)
docs/method/backlog/bad-code/PROCESS_generated-reference-sync-coupling.mddocs/method/backlog/bad-code/PROCESS_typed-frontmatter-access.mddocs/method/backlog/cool-ideas/PROCESS_backlog-query-surface.mddocs/method/backlog/cool-ideas/PROCESS_design-doc-template-catalog.mddocs/method/backlog/cool-ideas/PROCESS_next-work-menu.mddocs/method/backlog/cool-ideas/PROCESS_review-closeout-helper.md
Owner
Author
|
Review closeout summary for
Verification: |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/method/backlog/bad-code/PROCESS_generated-reference-sync-coupling.md`:
- Around line 30-49: Update the spec to match the implementation by either (A)
replacing vague "such as" wording with an explicit list of the generated
reference signpost files exactly: ARCHITECTURE.md, docs/CLI.md, docs/MCP.md,
docs/GUIDE.md (and explicitly mark CHANGELOG.md and docs/BEARING.md as
ship-only), or (B) define a precise membership rule that determines generated
reference docs (for example: "files in the signpost directory containing a <!--
generate:NAME --> marker") and ensure that rule exactly matches the behavior of
generateReferenceDocs() and method sync refs; also verify that
generateReferenceDocs() returns that exact target list and method sync refs
prints the refreshed targets.
In `@docs/method/backlog/bad-code/PROCESS_typed-frontmatter-access.md`:
- Around line 49-55: The "Typed writes" section currently mandates failing with
an error naming the field, expected shape, and attempted shape but lacks a
concrete example; add a short example error message immediately after the
sentence ending with "attempted shape" (the Typed writes paragraph) showing the
exact wording implementers should produce — e.g. a single-line example like:
Cannot downgrade typed field 'acceptance_criteria': expected array<string>,
attempted string — and label it as an "Example error" or footnote so readers
know it's illustrative.
- Around line 41-45: Locate the "Supported first-cut types" section and replace
the vague phrase "Quoted number-looking strings remain strings" with an explicit
rule: either remove the qualifier and state "Quoted values remain strings (YAML
preserves quoting)" or explicitly specify the numeric pattern you treat as
numeric (e.g., integers, floats, and scientific notation such as
/^\-?\d+(\.\d+)?([eE][+\-]?\d+)?$/) and clarify that quoted matches of that
pattern are treated as strings; update the line containing "Quoted
number-looking strings remain strings" accordingly so the behavior is
unambiguous.
In `@docs/method/backlog/cool-ideas/PROCESS_backlog-query-surface.md`:
- Around line 35-39: The current doc states a dependency on extending
BacklogItem and typed frontmatter but doesn't resolve whether implementation
must wait; update the contract in the section referencing BacklogItem and typed
frontmatter to include an explicit "Implementation phasing" clause that offers
three options: (1) this is a design-only doc and implementation must wait for
typed frontmatter + BacklogItem extension, (2) the implementation slice includes
the typed frontmatter work and BacklogItem extension together, or (3) ship an
initial reduced surface returning only existing BacklogItem fields (path, stem,
slug, lane, legend) and add title, priority, owner, has_acceptance_criteria
later once typed frontmatter lands; make sure to mention the fields title,
priority, owner, and has_acceptance_criteria and reference BacklogItem and typed
frontmatter so reviewers know which symbols are affected.
In `@docs/method/backlog/cool-ideas/PROCESS_design-doc-template-catalog.md`:
- Around line 52-64: The `default-change` template lacks mandated section
headings; update the `default-change` template definition to include a
recommended scaffold such as "## Intended Behavior", "## Happy Path", "## Edge
Cases and Failure Modes", and "## Verification Plan" (noting authors may
merge/rename with reviewer agreement), and apply the same scaffold pattern to
all other template definitions in this document so reviewers and automation can
reliably detect required topics; reference the `default-change` template name
and the document's template sections when making the edits.
In `@docs/method/backlog/cool-ideas/PROCESS_next-work-menu.md`:
- Around line 114-120: The spec is underspecified: the BEARING-driven override
language (references to BEARING, BEARING.md, and the "must cite the exact
bearing evidence") requires a defined detection mechanism and a quantitative
threshold for "materially changes"; update PROCESS_next-work-menu.md to specify
how BEARING influences populate the `signals` array (e.g., require the tool to
emit signals.type `bearing_mention` by literal substring matching of backlog
item stems against BEARING.md's "Where are we going?" and "What feels wrong?"
sections and forbid LLM/fuzzy/manual overrides), or alternatively mandate a
human-in-the-loop flag (e.g., --bearing-item=<stem>) to trigger elevation; also
define a numeric or rank-shift threshold for "materially changes" (for example,
elevation of an item by N or moving it past the previous top K) and document
that the output must include the matching `bearing_mention` evidence when that
threshold is met so `signals` fully describes BEARING-derived overrides.
In `@docs/method/backlog/cool-ideas/PROCESS_review-closeout-helper.md`:
- Around line 63-71: The document introduces a `confidence` field for
round_summary[*].outcome but leaves it undefined; either remove the field or
explicitly specify its type, allowed values, setter and interpretation—for
example define `confidence` as a string enum (`confirmed`, `inferred-high`,
`inferred-low`, `unknown`), state who sets it (operator sets `confirmed`, the
helper/inference engine sets `inferred-*` or `unknown`), and describe the
meaning and audit action (e.g., `inferred-low` must be flagged for human
review); update the text that references `actor, timestamp, and confidence` to
reflect this concrete spec so downstream automation can rely on it.
- Around line 37-48: Clarify and fully specify exit-code semantics for the write
contract: define exit 0 as "all requested mutations applied or no mutations
needed (explicit no-op)" vs a distinct non-zero if callers need to
differentiate; define exit 2 as "partial write failure where some threads were
mutated and others failed" and explicitly state whether partial successes are
left as-is (no automatic rollback) or must be rolled back (pick one and
implement accordingly) for the commands that perform mutations (flags: --reply,
--resolve, --post-summary combined with --apply); keep exit 3 for pre-write
validation/mapping errors and exit 4 for authentication/permission failures
(either pre-flight or mid-batch — state whether mid-batch auth failures are
treated as exit 2 or 4); require that stdout/stderr include parseable per-thread
error details and summary, and specify that --json mode must preserve the same
numeric exit codes but emit a machine-readable JSON object with per-thread
results and error codes for use by callers and CI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 13316884-5cce-404d-a7a7-f2ae49a7b59a
📒 Files selected for processing (6)
docs/method/backlog/bad-code/PROCESS_generated-reference-sync-coupling.mddocs/method/backlog/bad-code/PROCESS_typed-frontmatter-access.mddocs/method/backlog/cool-ideas/PROCESS_backlog-query-surface.mddocs/method/backlog/cool-ideas/PROCESS_design-doc-template-catalog.mddocs/method/backlog/cool-ideas/PROCESS_next-work-menu.mddocs/method/backlog/cool-ideas/PROCESS_review-closeout-helper.md
Owner
Author
|
Second review-round SHA proof for
Verification: |
4 of 6 tasks
This was referenced Jun 1, 2026
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This batches the backlog captures that came out of the review-state work and the follow-on agent-first triage pass.
Added backlog items:
PROCESS_design-doc-template-catalogPROCESS_next-work-menuPROCESS_generated-reference-sync-couplingPROCESS_backlog-query-surfacePROCESS_review-closeout-helperPROCESS_typed-frontmatter-accessWhy these:
Verification:
npm test -- tests/docs.test.tsgit diff --check