Skip to content

skills: code-review-graph — one skill for discovery, active use, and branch-switch convergence - #56

Merged
omp-harness[bot] merged 17 commits into
mainfrom
skill/code-review-graph
Oct 31, 2026
Merged

omp-harness[bot] merged 17 commits into
mainfrom
skill/code-review-graph

Conversation

@omp-harness

@omp-harness omp-harness Bot commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

User description

What

Replaces skills/code-discovery/ with a single skills/code-review-graph/ skill covering all three graph concerns:

  1. Discovery precedence (absorbed from code-discovery): structural questions (callers_of, importers_of, …) start with the graph, not grep — it is parsed AST and finds callers text search misses. Blast radius informs where you test, not whether you edit.
  2. Active use at common development moments (new): before editing a called function, locating where X is handled, entering unfamiliar code, tracing data-flow bugs, picking tests for a change, review/refactor prep — each with the concrete tools.
  3. Branch-switch doctrine (new, verified by experiment this session):
    • The crg-watch watcher converges the graph automatically on checkout — additions, deletions, and recorded branch/commit metadata, within seconds. No rebuild ceremony.
    • If the watcher was down during a switch, the graph is silently stale and restarting the watcher does NOT recover it (purely event-driven, no startup sweep).
    • Detect via code-review-graph status ("Built at commit" ≠ HEAD / branch WARNING); recover with one incremental code-review-graph build. Never --full-rebuild.

Why one skill, why this name

code-discovery only matched one of the three concerns — a stale-graph or impact-radius question would never surface it. Every situation that needs this knowledge already contains the token code-review-graph: the mounted tool names (mcp__code_review_graph_*), the CLI (code-review-graph status), staleness warnings, and repo AGENTS.md text. Naming the skill after the tool maximizes discovery; alwaysApply is retained so the discovery-precedence guarantee is unchanged.

Evidence for the branch-switch claims

Verified in a throwaway repo this session (build → watch → branch add/delete/switch → inspect graph nodes + status):

  • Watcher running during switch: graph converged fully, including Built on branch/Built at commit metadata.
  • Watcher down during switch, then restarted: graph stayed stale (old branch's symbols), WARNING present.
  • One incremental build recovered content and metadata, cleared the warning.

Notes

  • No other file references skill://code-discovery; the old installed symlink was removed by hand after make install (install does not prune dead links).
  • The installed skill is live after the next omp restart.
  • Deployment: takes effect via make install in omp-config + an omp restart; until restart the deleted code-discovery skill stays loaded (harmless — it duplicates the new skill's discovery rule).

PR Type

Documentation


Description

  • Replace code-discovery skill with unified code-review-graph

  • Keep graph-first structural discovery precedence

  • Add active-use moment table and recovery doctrine

  • Document watcher convergence across branch switches


Diagram Walkthrough

flowchart LR
  A["code-discovery (removed)"] -- "replaced by" --> B["code-review-graph skill"]
  B --> C["Graph-first discovery"]
  B --> D["Active-use moments"]
  B --> E["Branch-switch doctrine"]
Loading

File Walkthrough

Relevant files
Documentation
SKILL.md
Delete superseded code-discovery skill                                     

skills/code-discovery/SKILL.md

  • Removes the superseded code-discovery skill in full
  • Its graph-first precedence rule moves to the new skill
+0/-26   
SKILL.md
Add unified code-review-graph skill documentation               

skills/code-review-graph/SKILL.md

  • Adds the single code-review-graph skill with alwaysApply
  • Codifies graph-first structural discovery before grep
  • Adds a table mapping development moments to graph tools
  • Documents branch-switch convergence and stale-graph recovery
+59/-0   

Pin + output contract (1f11330)

The lucidlint pin moves to 8761305 (the three-shape fix contract build — lucidlint PR #14): fix output is silence / needs-input / applied, with anchor re-attach. The per-kind fix table is shrunk to that contract; the git-diff-after-every-fix guidance is gone — the tool says what it did. Merge order: lucidlint#14 first, then this PR (it carries the pin; #58's pin hunk is stripped so nothing reverts it). The Makefile's bundle pin (v0.3.0) tracks release tags — bump it to the next lucidlint release tag when one is cut.

…cedence, active-use moments, branch-switch convergence doctrine
@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 6cf3f16)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit a99fb44

Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
…ed 0.4.0 matrix (stale/magic/dup apply; undeclared/positional prescribe)
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 25fc6b3

Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 39c2b82

Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/lucidlint-workflow/SKILL.md Resolved
…pattern list, spell out positional-literals mis-binding
@omp-harness

omp-harness Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Deploy note addressed in 2c3e7e9 + the PR description now carries an explicit Deployment line (make install + omp restart; the deleted code-discovery skill stays loaded until restart, duplicating only the discovery rule).

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 2c3e7e9

Comment thread skills/lucidlint-workflow/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit ab85d56

Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/lucidlint-workflow/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
…d full-rebuild fallback, git-recovery wording for duplicate-block
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 0c3b4f4

Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
…uation, stale-answer recovery pointer, sentence reflow
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 6d175a8

Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/lucidlint-workflow/SKILL.md Resolved
…14); shrink the per-kind fix table to the output contract — applied / needs input / silence, anchor re-attach
ashbywinch pushed a commit that referenced this pull request Sep 5, 2026
…contract build; this PR no longer touches the pin line
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 1f11330

Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
…ew-bot fix (applied line reports the re-anchored line)
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Standalone PR Review

PR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

…th a trust rule, build-failure handling, patterns as bullets, impact analysis references its tools; lucidlint-workflow: git-restore caveat (pre-commit only, git status first)
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Standalone PR Review

PR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Compliance: one-topic

docs/writing-documentation.md applies the one-topic-per-file rule to
skills: one task or procedure per skill. This always-applied skill
bundles separate workflows — the graph-first discovery/impact rules,
a six-row tool table for distinct development moments, and a
standalone branch-switch staleness detect/recover/verify procedure.
An agent that only hits stale-graph status must still load and pay
context for the other usage rows, and the recovery sequence is a
separate procedure by the standard's own test. Split the branch-switch
recovery into its own skill, or link it only where relevant, so the
always-loaded skill body answers one question.

## Use it at these development moments

| Moment | Tools | Answers |
|---|---|---|
| Before editing a function others call | `query_graph` (`callers_of`), `get_impact_radius` | Which callsites must migrate or re-test? |
| "Where is X handled?" | `semantic_search_nodes`, `traverse_graph` | Locate by concept, not keyword |
| Entering unfamiliar code | `get_architecture_overview`, `get_community` | Structure without reading every file |
| Debugging a data-flow bug | `get_flow`, `traverse_graph` | Entry point → root cause chain |
| Picking tests for a change | `query_graph` (`tests_for`), `get_affected_flows` | Which tests exercise this code? |
| Review / refactor prep | `get_review_context`, `detect_changes`, `find_large_functions` | Focused context, refactor targets |

## Branch switches and staleness

The watcher converges the graph automatically: a checkout is just file
events, and additions, deletions, and the recorded branch/commit update
within seconds. Do not run rebuild ceremonies after a branch switch while
the watcher is running. Nothing reports the watcher's downtime, so treat
staleness as possible after any switch and run this sequence once:

1. **Detect** — `code-review-graph status`. A branch WARNING or a built
   commit ≠ HEAD means the graph is stale. (A watcher that was down
   during a switch recovers nothing when restarted — it is purely
   event-driven, with no startup sweep; only `build` recovers.)
2. **Recover** — `code-review-graph build`: incremental; re-parses only
   files whose stored hash differs from disk. `build` is standalone — it
   reads the working tree directly and needs no watcher. If `build`
   errors, or `status` still reports stale afterwards, check file
   permissions and graph-state corruption first; `--full-rebuild` is the
   last resort for confirmed corrupt state — never routine (it discards
   the incremental fast path).
3. **Verify** — re-run `status`: the signal from Detect must be gone
   before trusting graph answers.

**When to trust the graph:** only after step 3 (or a `status` with no
WARNING and a built commit equal to HEAD). When graph answers contradict
the code you are reading, suspect the graph first and run the sequence
before distrusting the code.
Compliance: rollout note

Skill/standard changes in this repo are required to keep make test
green and to state that the change only takes effect after make
install and an omp restart. This PR modifies two skills but carries no
such statement, so agents and humans in installed repos will continue
loading the previous skill versions after merge. Add the self-check
result and the install/restart rollout note to the PR.

---
name: code-review-graph
description: |
  How to use the code-review-graph code knowledge graph: answer structural
  questions with its tools before falling back to grep — who calls this
  function, where is this handled, what would this change affect, which
  tests cover this code. Also what to do when the graph is stale, e.g.
  after a branch switch while the watcher was down.
alwaysApply: true

Comment thread skills/lucidlint-workflow/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Standalone PR Review

PR-Agent could not safely update the persistent review. This standalone result will not replace the canonical review.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

…-workflow: anchor-moved is a prefix of applied

Both from review findings. The shorthand rule named the <name> tool forms, but
the bullets under query_graph (callers_of, importers_of, ...) read as tool
names that could be expanded the same way. And the fix-output section claimed
exactly three shapes while a fourth line — anchor moved — appears in the same
stream; it is a prefix on applied, not a separate outcome.
Comment thread skills/lucidlint-workflow/SKILL.md Resolved
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 046be72

Comment thread skills/code-review-graph/SKILL.md Resolved
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit c504a33

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

Comment thread skills/lucidlint-workflow/SKILL.md Resolved
Comment thread skills/lucidlint-workflow/SKILL.md Resolved
Comment thread skills/code-review-graph/SKILL.md Resolved
…ce; lucidlint-workflow: plain install step, clearer fix outcomes
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 6cf3f16

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@omp-harness
omp-harness Bot merged commit afd721b into main Oct 31, 2026
3 checks passed
@omp-harness
omp-harness Bot deleted the skill/code-review-graph branch October 31, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant