Repository navigation
feat(yellow-core): add compound-lifecycle skill (W3.10) - #296
Conversation
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
User kinginyellow18@proton.me does not have a PR Review subscription. Go to Team management and add this email to the PR Review subscription. |
📝 WalkthroughWalkthroughThis PR adds documentation for a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Skill as compound-lifecycle Skill
participant Detector as Staleness & Overlap Detector
participant Gate as AskUserQuestion
participant Compounder as knowledge-compounder
participant Archive as docs/solutions/archived/
User->>Skill: Invoke run (interactive or autofix)
Skill->>Detector: Enumerate candidates & gather evidence
Detector->>Detector: Compute composite staleness & detect overlaps
Detector-->>Skill: Return scored clusters/candidates
Skill->>Gate: Present actions (Keep/Update/Consolidate/Replace/Delete)
Gate->>User: Request decision (interactive)
User-->>Gate: Approve action
alt Consolidate or Replace
Gate->>Compounder: Dispatch Task to write merged canonical entry
Compounder-->>Gate: Successor entry created
Gate->>Archive: Move sources to archived/ and add `superseded_by`
else Delete
Gate->>Archive: Move source to archived/ and add `superseded_by: null`
else Update
Gate->>Skill: Apply inline update (or mark stale in autofix)
else Keep
Gate->>Skill: No change
end
Archive-->>User: Archived files at docs/solutions/archived/
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
How to use the Graphite Merge QueueAdd the label graphite-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 1 minor |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Self-review via /review:pr (scoped: 5 of 7 — full keystone subset for the prose-heavy SKILL.md)Verdict: Not ready (8 P1, 9 P2, several P3). Substantive revision required before merge. This PR has the most findings of the four reviewed today. The keystone is doing its job — these are real issues, not noise. P1 — Critical (8, with multi-reviewer agreement)
P2 — Moderate (9, several with multi-reviewer agreement)
P3 (selected)
Resolution pathThis PR has too many cross-cutting issues to amend cleanly in this session. The bugs aren't isolated typos — they reflect interface mismatches with two other agents ( Options:
I'd recommend option 1 or 2. The findings here are good signal that this skill needs more cycles than I gave it the first time. Coverage
|
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Maintain the long-term quality of the docs/solutions/ catalog through three operations: 1. Composite staleness detection — replaces single time-threshold with a 4-component score: 0.4*days_modified + 0.3/inbound_refs + 0.2*embedding_age + 0.1*days_since_retrieved (Atlan-pattern). Heavily-cited evergreen entries don't get false-flagged; recent entries with broken references do. ruvector components contribute zero on graceful degradation. 2. Two-pass overlap detection — category+tags overlap (cheap), then BM25 on problem: lines, then optional ruvector cosine clustering at 0.82 threshold (calibrated for paragraph-level semantic equivalence on markdown corpora — USE convention). 3. AskUserQuestion-gated consolidation hand-off — never auto-merges. Dispatches knowledge-compounder via Task to write the merged canonical entry, then archives the source entries with a superseded_by: frontmatter pointer. Five-outcome classification (Keep / Update / Consolidate / Replace / Delete-and-archive) adapted from upstream ce-compound-refresh at SHA e5b397c9. Yellow-plugins divergence: archive instead of delete. Archived entries move to docs/solutions/archived/<original-category>/ and remain searchable for forensics, citation continuity, and learnings-researcher fallback. learnings-researcher excludes archived/ from default search. Per-project tuning via yellow-plugins.local.md compound_lifecycle.* keys (forward-declared in local-config skill). Autofix mode for scheduled runs: applies unambiguous Updates only; marks ambiguous cases as status: stale for human review. Hard quality dependency for W3.11 ideation skill per the source plan's research note. Bumps yellow-core skill count: 5 → 6. README catch-up from stale 13a/7c/4s/2m to current 17a/8c/6s/1m (drift accumulated across several merged PRs; this brings README in lockstep).
81cd26a to
d289d3d
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…laims, abort guard Addresses 30 review threads from greptile, gemini, codex, devin, and coderabbit: - Staleness formula: replace additive citation term with multiplicative discount (heavily-cited entries are now actually protected from age-based false-flagging) - Step 1: add docs/solutions/_lifecycle-runs/** exclusion (avoid self-ingestion in autofix mode) - Step 8: add explicit abort guard when knowledge-compounder verification fails (no more partial-archive corruption) - Step 5b: change overlap pass-1 to >= 2 members (catches two-entry duplicates) - Step 8 fencing: replace HTML-entity sanitization with --- begin source-entry (reference only) --- delimiters - Config path: yellow-plugins.local.md at repo root (matches local-config schema) - Degraded-mode formula: add max(refs,1) guard (no division by zero) - Never-retrieved fallback: days_since_retrieved = days_since_modified - Frontmatter schema note: status/stale_*/updated/superseded_by are yellow-plugins extensions (upstream snapshot frozen) - README.md: drop yellow-core 1 MCP suffix; remove stale Context7/Ceramic auth rows; retitle Context7 section to (user-level MCP); fix project tree - archived/README.md: markdownlint code fence; correct exclusion claim; replace 30-day guard claim with updated: bump instruction - plugins/yellow-core/README.md: add compound-lifecycle skill row
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…aim, changeset Three follow-up review threads from round 1 push: - README.md: yellow-review row regression (7 agents → 14 agents, the actual count). Both the plugin table and project structure tree had stale 7-agent counts that contradicted plugins/yellow-review/CLAUDE.md '### Agents (14)'. - SKILL.md Step 9: remove the false claim that _lifecycle-runs/ is excluded from learnings-researcher 'by virtue of its _-prefix'. Replace with accurate statement: Step 1's exclusion list excludes it from compound-lifecycle's own discovery, but learnings-researcher does not currently filter _-prefixed paths (separate follow-up). - .changeset/compound-lifecycle-skill.md: remove 'forward-declared in the local-config skill schema; no schema migration needed' claim. Replace with 'yellow-plugins-specific extensions, not yet declared in the schema; parse silently under graceful-degradation'.
7f28bf3 to
bca8ec6
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
Maintain the long-term quality of the docs/solutions/ catalog through
Five-outcome classification (Keep / Update / Consolidate / Replace / Yellow-plugins divergence: archive instead of delete. Archived entries Per-project tuning via yellow-plugins.local.md compound_lifecycle.* Autofix mode for scheduled runs: applies unambiguous Updates only; Hard quality dependency for W3.11 ideation skill per the source plan's Bumps yellow-core skill count: 5 → 6. README catch-up from stale SummaryStack contextTest planNotes for reviewers
Summary by CodeRabbit
Greptile SummaryThis PR introduces the
Confidence Score: 3/5Not safe to merge — two new P1 execution failures in SKILL.md, plus several unresolved P1s from prior review rounds. Two new P1 issues (wrong Task description for Replace, missing mkdir) plus multiple unresolved P1s from previous review threads (staleness formula invariant broken, example calculation wrong by 10×, no abort path on compounder failure, superseded_by value undefined for Delete-and-archive) pull the score well below the P1 ceiling of 4. plugins/yellow-core/skills/compound-lifecycle/SKILL.md requires the most attention — Step 8 Execute section has multiple execution-time defects. Important Files Changed
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
plugins/yellow-core/skills/compound-lifecycle/SKILL.md (3)
279-289:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winArchiving lacks explicit failure guard.
Lines 279-280 say "Verify it exists" but don't state what happens when verification fails. If the knowledge-compounder dispatch fails or returns an invalid path, proceeding with archive moves would create split-brain state (sources archived but no canonical successor). Add explicit abort-on-failure: "If verification fails, abort the consolidation; do not archive sources."
🛡️ Proposed guard clause
3. Wait for the agent's return. The new entry's path is the agent's - output. Verify it exists. + output. Verify it exists. **If the returned path is missing or + invalid, abort this consolidation and skip the archive moves below.** 4. Archive each source entry: `mv` from🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/yellow-core/skills/compound-lifecycle/SKILL.md` around lines 279 - 289, Add an explicit abort-on-failure guard after the agent-return verification step: when the knowledge-compounder dispatch or the returned path check fails (the "Verify it exists" step), stop the consolidation process, do not perform the archive moves in "Archive each source entry" (the mv operations that move docs/solutions/... to docs/solutions/archived/...), and record the failure in the run report; update SKILL.md to state this guard and that the process should log the error and abort the consolidation instead of proceeding to append superseded_by or moving source files.
191-193:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winOff-by-one: "> 2 members" excludes valid pairs.
Line 193 says "Surface groups with > 2 members" which excludes 2-entry overlaps. Two entries with identical category and 2+ shared tags are valid consolidation candidates. Change to ">= 2 members" or "> 1 members".
🔧 Proposed fix
- Surface groups with > 2 members. + Surface groups with >= 2 members.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/yellow-core/skills/compound-lifecycle/SKILL.md` around lines 191 - 193, The rule text under "Cheap pass — `category` + `tags` overlap." currently filters groups with "> 2 members" which wrongly excludes valid pairs; update the sentence that reads "Surface groups with > 2 members." to instead read "Surface groups with >= 2 members." (or "Surface groups with > 1 member.") so groups of two entries are included; locate the phrase in the SKILL.md block titled "Cheap pass — `category` + `tags` overlap." and change the comparison operator accordingly.
291-295:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDelete-and-archive contradicts itself on
superseded_bymetadata.Line 293 says "Same as the archive step in Consolidate (move +
superseded_byfrontmatter)" but line 295 says "there's no successor entry." You cannot add asuperseded_bypointer when there's no successor to point to. Either omitsuperseded_byfor Delete-and-archive, or use a different frontmatter field likearchive_reason: obsoleteto document why the entry was archived without a successor.🔧 Proposed fix
#### Delete-and-archive -Same as the archive step in Consolidate (move + `superseded_by` -frontmatter), but skip the `knowledge-compounder` dispatch since -there's no successor entry. +Move to `docs/solutions/archived/<category>/<slug>.md` (same directory +structure as Consolidate). Append `archive_reason: obsolete` to the +frontmatter to document that the entry was archived without a +successor. Do NOT add `superseded_by` since there is no successor.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/yellow-core/skills/compound-lifecycle/SKILL.md` around lines 291 - 295, The Delete-and-archive step description is self-contradictory about adding a superseded_by pointer when there is no successor; update the SKILL.md text for the "Delete-and-archive" step (referencing Consolidate and the knowledge-compounder dispatch) to remove any instruction to add `superseded_by` and instead document adding an `archive_reason: obsolete` (or similar) frontmatter field to record why the entry was archived without a successor; ensure the doc explicitly notes that the `knowledge-compounder` dispatch is skipped and that `superseded_by` is not used for this flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/yellow-core/skills/compound-lifecycle/SKILL.md`:
- Line 271: Several fenced code blocks in SKILL.md missing language tags cause
markdownlint MD040; annotate the three blocks by adding appropriate language
identifiers: change the block that starts with Task( / contains the Task
pseudo-code (referenced by the "Task(" token) to use ```text or ```python,
update the report template block that begins with "## compound-lifecycle Report"
to use ```markdown or ```text, and mark the configuration example that contains
the "compound_lifecycle:" key with ```yaml; apply these tag additions to the
three corresponding fenced blocks so each opening ``` is replaced with the
correct ```text/```python, ```markdown/```text, and ```yaml respectively.
---
Duplicate comments:
In `@plugins/yellow-core/skills/compound-lifecycle/SKILL.md`:
- Around line 279-289: Add an explicit abort-on-failure guard after the
agent-return verification step: when the knowledge-compounder dispatch or the
returned path check fails (the "Verify it exists" step), stop the consolidation
process, do not perform the archive moves in "Archive each source entry" (the mv
operations that move docs/solutions/... to docs/solutions/archived/...), and
record the failure in the run report; update SKILL.md to state this guard and
that the process should log the error and abort the consolidation instead of
proceeding to append superseded_by or moving source files.
- Around line 191-193: The rule text under "Cheap pass — `category` + `tags`
overlap." currently filters groups with "> 2 members" which wrongly excludes
valid pairs; update the sentence that reads "Surface groups with > 2 members."
to instead read "Surface groups with >= 2 members." (or "Surface groups with > 1
member.") so groups of two entries are included; locate the phrase in the
SKILL.md block titled "Cheap pass — `category` + `tags` overlap." and change the
comparison operator accordingly.
- Around line 291-295: The Delete-and-archive step description is
self-contradictory about adding a superseded_by pointer when there is no
successor; update the SKILL.md text for the "Delete-and-archive" step
(referencing Consolidate and the knowledge-compounder dispatch) to remove any
instruction to add `superseded_by` and instead document adding an
`archive_reason: obsolete` (or similar) frontmatter field to record why the
entry was archived without a successor; ensure the doc explicitly notes that the
`knowledge-compounder` dispatch is skipped and that `superseded_by` is not used
for this flow.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8995cc9-d7cb-4ef5-8ac9-793f9215f1cd
📒 Files selected for processing (5)
.changeset/compound-lifecycle-skill.mdREADME.mddocs/solutions/archived/README.mdplugins/yellow-core/README.mdplugins/yellow-core/skills/compound-lifecycle/SKILL.md
✅ Files skipped from review due to trivial changes (2)
- plugins/yellow-core/README.md
- docs/solutions/archived/README.md
…ntradiction Four follow-up review threads from round 2 push: - SKILL.md L168-180: Math error in round-1 example. The multiplicative discount alone cannot protect heavily-cited entries from age (factor asymptotic to 0.7 = max 30% reduction). A 3-year-old entry with 10 refs scores ~320, NOT ~32 as previously claimed. Add an explicit citation gate (default: refs >= 5 → classify Keep, skip score) as the actual protection mechanism. The discount remains a smoothing correction for low-to-moderate citation cases. - SKILL.md L271, 301: markdownlint MD040 — added 'text' and 'markdown' language tags to fenced blocks (Task pseudo-code and report template). - SKILL.md L396-399 + archived/README.md L26-28: archived/ exclusion contradiction. SKILL.md previously claimed archived/ was excluded from learnings-researcher 'by path filter'; archived/README.md now correctly says no exclusion exists. Aligned both to 'intended exclusion, not yet implemented (separate follow-up)'.
bca8ec6 to
b427503
Compare
Move everyinc-merge-remaining-work.md and everyinc-merge-wave3.md to plans/complete/. Both are fully delivered on main: - everyinc-merge-remaining-work.md: PRs #402, #403, #406, #410, #412 - everyinc-merge-wave3.md: 12 stack items shipped via PRs #287, #293, #296, #306, #307, #308, #310, #311, #312, plus #403/#406/#410 (rolled into the remaining-work plan) Updated wave3 stack-progress checkboxes for items #2/#5/#7 with PR refs and squash-commit SHAs.
Maintain the long-term quality of the docs/solutions/ catalog through
three operations:
Composite staleness detection — replaces single time-threshold with
a 4-component score: 0.4days_modified + 0.3/inbound_refs +
0.2embedding_age + 0.1*days_since_retrieved (Atlan-pattern).
Heavily-cited evergreen entries don't get false-flagged; recent
entries with broken references do. ruvector components contribute
zero on graceful degradation.
Two-pass overlap detection — category+tags overlap (cheap), then
BM25 on problem: lines, then optional ruvector cosine clustering at
0.82 threshold (calibrated for paragraph-level semantic equivalence
on markdown corpora — USE convention).
AskUserQuestion-gated consolidation hand-off — never auto-merges.
Dispatches knowledge-compounder via Task to write the merged
canonical entry, then archives the source entries with a
superseded_by: frontmatter pointer.
Five-outcome classification (Keep / Update / Consolidate / Replace /
Delete-and-archive) adapted from upstream ce-compound-refresh at SHA
e5b397c9.
Yellow-plugins divergence: archive instead of delete. Archived entries
move to docs/solutions/archived// and remain
searchable for forensics, citation continuity, and learnings-researcher
fallback. learnings-researcher excludes archived/ from default search.
Per-project tuning via yellow-plugins.local.md compound_lifecycle.*
keys (forward-declared in local-config skill).
Autofix mode for scheduled runs: applies unambiguous Updates only;
marks ambiguous cases as status: stale for human review.
Hard quality dependency for W3.11 ideation skill per the source plan's
research note.
Bumps yellow-core skill count: 5 → 6. README catch-up from stale
13a/7c/4s/2m to current 17a/8c/6s/1m (drift accumulated across
several merged PRs; this brings README in lockstep).
Summary
Stack context
Test plan
Notes for reviewers
Summary by CodeRabbit
New Features
Documentation
Other
Greptile Summary
Adds the
compound-lifecycleskill (W3.10) toyellow-core, implementing composite staleness scoring, two-pass BM25+cosine overlap clustering, and anAskUserQuestion-gated consolidation hand-off that archives superseded entries todocs/solutions/archived/instead of deleting them. Across previous review rounds several P1 gaps inSKILL.mdwere identified (no abort path whenknowledge-compounderfails before archive moves, missingmkdir -pguard,superseded_byvalue unspecified in the Delete-and-archive path,Taskdescription hardcoded as "Consolidate" for Replace flow); none of those appear resolved in this revision.Confidence Score: 3/5
Not ready to merge — multiple P1 issues from prior review rounds remain open in SKILL.md's execute path
Several P1 findings from previous review rounds are still unaddressed: no abort/rollback path when knowledge-compounder fails before archive moves execute, missing mkdir -p guard, superseded_by value unspecified in Delete-and-archive path, and Task description hardcoded as Consolidate for the semantically distinct Replace flow. New findings in this pass are all P2. The combination of multiple unresolved P1s on the core execute path warrants a score below the P1 ceiling of 4.
plugins/yellow-core/skills/compound-lifecycle/SKILL.md — Step 8 execute path has unresolved P1 gaps from prior rounds
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["/yellow-core:compound-lifecycle [scope-hint]"] --> B[Step 1: Discover candidates docs/solutions/**/*.md exclude archived/ + _lifecycle-runs/] B --> C{Scope hint?} C -- yes --> D[Step 2: Filter by path/category/tag/keyword] C -- no --> E[Step 3: Route by count] D --> E E -- "1-2 Focused" --> F[Investigate directly] E -- "3-8 Batch" --> G[Investigate all] E -- "9+ Broad" --> H[Phase 0 Triage AskUserQuestion cluster order] H --> G F & G --> I[Step 4: Gather evidence refs / accuracy / inbound citations / ruvector recall] I --> J[Step 5a: Staleness score citation gate >= 5 → Keep composite formula otherwise] I --> K[Step 5b: Overlap detection pass1: category+tags pass2: BM25 p90 pass3: ruvector cosine >= 0.82] J & K --> L[Step 6: Classify Keep / Update / Consolidate / Replace / Delete-and-archive] L --> M{Mode?} M -- interactive --> N[Step 7: AskUserQuestion per cluster] M -- autofix --> O[Apply Updates; mark rest stale] N --> P[Step 8: Execute] O --> P P -- Update --> Q[Edit in-place bump updated:] P -- Consolidate/Replace --> R[Task: knowledge-compounder Verify new entry exists mv + superseded_by:] P -- Delete-and-archive --> S[mv + superseded_by: null no compounder dispatch] Q & R & S --> T[Step 9: Report interactive: inline autofix: _lifecycle-runs/timestamp.md]Comments Outside Diff (2)
docs/solutions/archived/README.md, line 188-191 (link)learnings-researcherexclusion claim directly contradicts SKILL.mdThis README states learnings-researcher does not exclude
archived/by default, butSKILL.md(lines 647–650) says the opposite: "Thearchived/subtree is excluded fromlearnings-researcher's default search by path filter." The changeset description also backs SKILL.md: "learnings-researcher excludes archived/ from default search." One of these documents is wrong about the current behaviour. A user reading this README will configure their workflow expecting archived entries to surface in learnings-researcher searches; a user reading SKILL.md will expect the opposite — leading to missed or unexpected recall results depending on which document they consulted.Prompt To Fix With AI
plugins/yellow-core/skills/compound-lifecycle/SKILL.md, line 544-548 (link)superseded_byvalue left unspecifiedStep 8 instructs "same as the archive step in Consolidate (move +
superseded_byfrontmatter)" but the Consolidate archive sub-step says to setsuperseded_byto the new canonical entry's path — which doesn't exist in the Delete-and-archive case.docs/solutions/archived/README.mdspecifies the correct value (superseded_by: null), but the SKILL.md procedure that an executing LLM reads step-by-step never states this. The agent will either omit the field entirely, inherit the wrong Consolidate semantics and leave an empty/broken pointer, or interpolate arbitrarily — all inconsistent with the documented schema.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (6): Last reviewed commit: "fix: round 3 PR #296 review — citation g..." | Re-trigger Greptile