Add skill health reporting#257
Conversation
|
Warning Review limit reached
More reviews will be available in 48 minutes and 16 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds end-to-end skill health tracking: a new ChangesSkill Health Feature
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over Client,CodeIntelStore: Observation Recording Phase
Client->>MCPServer: skill_lookup(skill_id: "safe-git-workflow")
MCPServer->>MCPServer: lookupSkill(skill)
MCPServer->>CodeIntelStore: RecordSkillObservation(tool=skill_lookup, outcome=unknown)
MCPServer-->>Client: skill response
Client->>MCPServer: skill_recommend(diagnostic)
MCPServer->>MCPServer: recommendSkills(recommendations)
MCPServer->>CodeIntelStore: RecordSkillObservation per recommendation (tool=skill_recommend)
MCPServer-->>Client: {recommendations: [...]}
end
rect rgba(144, 238, 144, 0.5)
Note over Client,CodeIntelStore: Health Reporting Phase
Client->>MCPServer: code_intel_skill_health(skill_id, format)
MCPServer->>MCPServer: codeIntelSkillHealth(skillHealthProvenance)
MCPServer->>CodeIntelStore: SkillHealth(SkillHealthQuery with 7d/30d windows)
CodeIntelStore->>CodeIntelStore: skillOutcomeRows → buildSkillHealthRecords → finalizeSkillHealthRecord
CodeIntelStore-->>MCPServer: SkillHealthReport
MCPServer-->>Client: JSON or TOON report
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the code_intel_skill_health tool and CLI command to measure and report skill usage, trends, and health from remediation-outcome evidence. The changes span documentation, CLI commands, MCP server integration, and core logic in Go. The review feedback highlights three key performance optimization opportunities: opening the database connection outside the loop when recording multiple skill recommendations, parsing the constant nowUTC timestamp once rather than repeatedly inside loops, and simplifying the maxUTC helper by using direct lexicographical string comparison instead of parsing RFC3339 strings into time.Time objects.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go/internal/codeintel/skill_health.go`:
- Around line 208-218: The summarizeSkillHealth function is being called after
the records have been truncated to query.Limit, causing the summary to only
reflect the limited subset of records instead of all skills. Move the
summarizeSkillHealth call to execute before the records are sliced to
query.Limit, so the summary is calculated on the complete records set rather
than the paginated set. This ensures the summary accurately represents all
skills.
- Around line 461-475: The UnknownSkill assignment in the
finalizeSkillHealthRecord function relies on an implicit invariant that the
Generated field correctly indicates catalog membership, which is fragile. Since
catalog membership is already explicitly known through the knownCatalog
parameter passed at record-creation time, refactor the UnknownSkill logic to use
knownCatalog directly rather than inferring it from the Generated field.
Simplify the assignment on line 472 to remove the dependency on the Generated
field, making the code more maintainable and less susceptible to future
refactoring errors by directly checking only knownCatalog and record.Total.
In `@go/internal/mcp/server.go`:
- Around line 842-845: The recordSkillLookupObservation method calls are
currently propagating errors that cause the entire skill lookup and skill
recommend tools to fail. Since observation recording is telemetry-only and not
critical to tool functionality, log the error instead of returning it. Find both
occurrences of the recordSkillLookupObservation error handling (around lines
842-845 and 887-890) and replace the error propagation with a log statement that
records the error but allows execution to continue.
🪄 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 Plus
Run ID: 0164c406-f43a-4105-8919-0dbbbaa35283
📒 Files selected for processing (11)
README.mddocs/CODE_INTEL.mddocs/MCP_SERVER.mdgo/internal/codeintel/skill_health.gogo/internal/codeintel/skill_health_test.gogo/internal/codeintelcli/main.gogo/internal/codeintelcli/main_internal_test.gogo/internal/mcp/protocol.gogo/internal/mcp/server.gogo/internal/mcp/server_test.gogo/internal/mcp/skill_health.go
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Validate GitHub workflows
- GitHub Check: Coding Ethos SARIF Gate / Coding Ethos SARIF Gate
- GitHub Check: Test (Python 3.11)
- GitHub Check: Test (Python 3.13)
- GitHub Check: Go coverage
- GitHub Check: Unified lint
- GitHub Check: Go fuzz smoke
- GitHub Check: CodeQL (go)
🔇 Additional comments (11)
go/internal/codeintel/skill_health_test.go (1)
14-202: LGTM!go/internal/codeintelcli/main.go (1)
25-25: LGTM!Also applies to: 100-100, 206-260
go/internal/codeintelcli/main_internal_test.go (1)
957-1011: LGTM!Also applies to: 1105-1107
go/internal/codeintel/skill_health.go (1)
680-729: LGTM!go/internal/mcp/protocol.go (1)
366-371: LGTM!Also applies to: 481-482, 879-879, 927-927, 1270-1295, 1590-1590
go/internal/mcp/skill_health.go (1)
17-54: LGTM!Also applies to: 56-65, 67-115, 117-136, 138-173
go/internal/mcp/server.go (1)
228-228: LGTM!go/internal/mcp/server_test.go (1)
54-55: LGTM!Also applies to: 85-85, 2029-2105
README.md (1)
368-370: LGTM!Also applies to: 433-439, 472-472
docs/CODE_INTEL.md (1)
192-200: LGTM!Also applies to: 280-280, 596-598
docs/MCP_SERVER.md (1)
71-73: LGTM!Also applies to: 105-111
|
Stage 2 update
PR is ready for merge once required review and pending checks complete. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
374-376: 📐 Maintainability & Code Quality | 🟡 MinorFix inconsistent window descriptions in MCP_SERVER.md (lines 82-84).
The skill health documentation has inconsistencies across files. README.md and CODE_INTEL.md consistently specify "7-day and 30-day" windows, but MCP_SERVER.md (line 82-84) vaguely states "recent usage windows". Align MCP_SERVER.md to match the specific window terminology used elsewhere. Additionally, clarify the evidence source: README.md (line 374-376) should mention that the report draws from "remediation-outcome evidence" or "code-intel outcome evidence" to match CODE_INTEL.md (line 605-607) and MCP_SERVER.md (line 124-127).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 374 - 376, The skill health documentation contains inconsistencies across README.md, CODE_INTEL.md, and MCP_SERVER.md. In MCP_SERVER.md (lines 82-84), replace the vague term "recent usage windows" with the specific terminology "7-day and 30-day" to match what is documented in README.md and CODE_INTEL.md. Additionally, update the code_intel_skill_health description in README.md (lines 374-376) to explicitly mention that the report draws from "remediation-outcome evidence" or "code-intel outcome evidence" to align with the evidence source terminology referenced in CODE_INTEL.md (lines 605-607) and MCP_SERVER.md (lines 124-127).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/CAPABILITY_SURFACE_DECISION.md`:
- Around line 74-84: The minimum grounding checklist under "Minimum grounding by
category" is missing an explicit requirement for code-intel storage contracts
related to skill health surfaces. Add a new bullet point to the existing list of
grounding requirements (which currently covers MCP tools, CLI commands, policy
IDs, provider behavior, and output contracts) that specifically requires new
code-intel measurement features to document their storage table, schema, and
windowing contract in the code-intel capability documentation. This ensures that
skill health observations persisted in code-intel tables like
remediation_outcomes are properly grounded and discoverable.
---
Outside diff comments:
In `@README.md`:
- Around line 374-376: The skill health documentation contains inconsistencies
across README.md, CODE_INTEL.md, and MCP_SERVER.md. In MCP_SERVER.md (lines
82-84), replace the vague term "recent usage windows" with the specific
terminology "7-day and 30-day" to match what is documented in README.md and
CODE_INTEL.md. Additionally, update the code_intel_skill_health description in
README.md (lines 374-376) to explicitly mention that the report draws from
"remediation-outcome evidence" or "code-intel outcome evidence" to align with
the evidence source terminology referenced in CODE_INTEL.md (lines 605-607) and
MCP_SERVER.md (lines 124-127).
🪄 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 Plus
Run ID: 373be931-6e43-40d5-abc9-0834bea1727c
📒 Files selected for processing (13)
.gitattributesREADME.mddocs/CAPABILITY_SURFACE_DECISION.mddocs/CODE_INTEL.mddocs/MCP_SERVER.mdgo/internal/codeintel/skill_health.gogo/internal/codeintel/skill_health_test.gogo/internal/codeintelcli/main.gogo/internal/codeintelcli/main_internal_test.gogo/internal/mcp/protocol.gogo/internal/mcp/server.gogo/internal/mcp/server_test.gogo/internal/mcp/skill_health.go
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Unified lint
- GitHub Check: Test (Python 3.11)
- GitHub Check: Coding Ethos SARIF Gate / Coding Ethos SARIF Gate
- GitHub Check: Validate GitHub workflows
- GitHub Check: Go coverage
- GitHub Check: Test (Python 3.13)
- GitHub Check: Go fuzz smoke
- GitHub Check: CodeQL (go)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-06-23T12:26:47.373Z
Learnt from: paudley
Repo: paudley/coding-ethos PR: 258
File: go/internal/geminiprompts/render.go:125-132
Timestamp: 2026-06-23T12:26:47.373Z
Learning: In the paudley/coding-ethos repo, the documented guideline “Gemini prompt pack should be generated in go/internal/geminiprompts/” refers to the generator implementation location (Go source), not the location of the generated prompt-pack artifact. Always treat PromptPackPath as the enforced output artifact path `.coding-ethos/gemini/prompt-pack.json` (e.g., render_test.go asserts PromptPackPath, and compiler_policies.go / hookrunner lookup / README document the expected path). Do not flag a code review issue just because PromptPackPath points to `.coding-ethos/gemini/prompt-pack.json`; only flag violations if the artifact output path logic breaks that enforced contract.
Applied to files:
go/internal/codeintel/skill_health_test.gogo/internal/mcp/protocol.gogo/internal/mcp/server.gogo/internal/codeintelcli/main_internal_test.gogo/internal/codeintelcli/main.gogo/internal/mcp/skill_health.gogo/internal/mcp/server_test.gogo/internal/codeintel/skill_health.go
🪛 LanguageTool
docs/CAPABILITY_SURFACE_DECISION.md
[grammar] ~23-~23: Ensure spelling is correct
Context: ...s, MCP tools, or runnable commands when those own the facts. 3. **Does an agent need...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~79-~79: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... must appear in docs/CODE_INTEL.md. - New policy IDs must live with their ETHOS p...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (15)
go/internal/codeintelcli/main.go (1)
45-47: LGTM!Also applies to: 71-71, 104-104
go/internal/codeintelcli/main_internal_test.go (1)
17-18: LGTM!Also applies to: 45-146
go/internal/codeintel/skill_health.go (1)
122-123: LGTM!Also applies to: 212-213, 385-386, 475-475
go/internal/codeintel/skill_health_test.go (1)
103-104: LGTM!Also applies to: 114-140
.gitattributes (1)
8-8: LGTM!docs/CAPABILITY_SURFACE_DECISION.md (1)
14-51: 📐 Maintainability & Code Quality | ⚡ Quick winSurface decision tree accurately grounds skill health across MCP, CLI, and code-intel storage.
The decision tree (question 5 at line 37-40) correctly identifies that durable evidence, audit, and trend analysis require SARIF/code-intel storage—precisely how skill health observations are persisted and reported. Question 3 (line 25-29) correctly routes interactive structured agent access to MCP, grounding the
code_intel_skill_healthtool. Question 4 (line 31-35) justifies the CLIskill-healthcommand as a one-shot local operation. The guide documents why multiple surfaces own parts of the skill health feature—a pattern the PR follows.docs/CODE_INTEL.md (2)
192-200: 📐 Maintainability & Code Quality | ⚡ Quick winSkill health measurement model clearly documented.
The new "Skill use" section (lines 192-200) accurately documents the measurement-only behavioral model: unknown-outcome observations from skill_lookup/recommend when code-intel is configured, explicit remediation outcomes for success/failure/abandonment, and 7d/30d windowed health reports. The clarification that learned/evolved skills are excluded from repo artifact promotion without explicit separate policy change aligns precisely with PR objectives and prevents scope creep.
280-283: 📐 Maintainability & Code Quality | ⚡ Quick winCLI command and MCP tool documentation is consistent with README and matches implementation.
The
skill-health --format toonCLI command (line 283) andcode_intel_skill_healthMCP tool (line 605-607) are consistently documented across CODE_INTEL.md, README.md, and MCP_SERVER.md. Both mention 7-day/30-day windows and unknown skill ID handling. The integration into the existing command/tool lists follows established patterns.Also applies to: 605-607
docs/MCP_SERVER.md (1)
82-84: 📐 Maintainability & Code Quality | ⚡ Quick winTool documentation and dataflow clearly explain skill health measurement model.
The
code_intel_skill_healthtool entry (lines 82-84) and dataflow explanation (lines 122-128) consistently describe the measurement-only behavior across all three documentation files. The dataflow correctly identifies that unknown-outcome observations flow from skill_lookup/skill_recommend through code-intel storage, thatcode_intel_skill_healthreads both observations and remediation outcomes, and that the result is measurement-only with no artifact promotion. The placement in the tool list (betweencode_intel_change_riskandcode_intel_health) is logically ordered.Also applies to: 122-128
go/internal/mcp/protocol.go (1)
312-330: LGTM!Also applies to: 500-512, 942-1022, 1697-1741
go/internal/mcp/server.go (2)
196-239: LGTM!Also applies to: 242-330
938-942: LGTM!Also applies to: 981-988
go/internal/mcp/skill_health.go (1)
12-16: LGTM!Also applies to: 141-148
go/internal/mcp/server_test.go (2)
56-57: LGTM!Also applies to: 75-77, 113-116, 119-190
338-397: LGTM!
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
374-376: 📐 Maintainability & Code Quality | 🟡 MinorFix inconsistent window descriptions in MCP_SERVER.md (lines 82-84).
The skill health documentation has inconsistencies across files. README.md and CODE_INTEL.md consistently specify "7-day and 30-day" windows, but MCP_SERVER.md (line 82-84) vaguely states "recent usage windows". Align MCP_SERVER.md to match the specific window terminology used elsewhere. Additionally, clarify the evidence source: README.md (line 374-376) should mention that the report draws from "remediation-outcome evidence" or "code-intel outcome evidence" to match CODE_INTEL.md (line 605-607) and MCP_SERVER.md (line 124-127).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 374 - 376, The skill health documentation contains inconsistencies across README.md, CODE_INTEL.md, and MCP_SERVER.md. In MCP_SERVER.md (lines 82-84), replace the vague term "recent usage windows" with the specific terminology "7-day and 30-day" to match what is documented in README.md and CODE_INTEL.md. Additionally, update the code_intel_skill_health description in README.md (lines 374-376) to explicitly mention that the report draws from "remediation-outcome evidence" or "code-intel outcome evidence" to align with the evidence source terminology referenced in CODE_INTEL.md (lines 605-607) and MCP_SERVER.md (lines 124-127).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/CAPABILITY_SURFACE_DECISION.md`:
- Around line 74-84: The minimum grounding checklist under "Minimum grounding by
category" is missing an explicit requirement for code-intel storage contracts
related to skill health surfaces. Add a new bullet point to the existing list of
grounding requirements (which currently covers MCP tools, CLI commands, policy
IDs, provider behavior, and output contracts) that specifically requires new
code-intel measurement features to document their storage table, schema, and
windowing contract in the code-intel capability documentation. This ensures that
skill health observations persisted in code-intel tables like
remediation_outcomes are properly grounded and discoverable.
---
Outside diff comments:
In `@README.md`:
- Around line 374-376: The skill health documentation contains inconsistencies
across README.md, CODE_INTEL.md, and MCP_SERVER.md. In MCP_SERVER.md (lines
82-84), replace the vague term "recent usage windows" with the specific
terminology "7-day and 30-day" to match what is documented in README.md and
CODE_INTEL.md. Additionally, update the code_intel_skill_health description in
README.md (lines 374-376) to explicitly mention that the report draws from
"remediation-outcome evidence" or "code-intel outcome evidence" to align with
the evidence source terminology referenced in CODE_INTEL.md (lines 605-607) and
MCP_SERVER.md (lines 124-127).
🪄 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 Plus
Run ID: 373be931-6e43-40d5-abc9-0834bea1727c
📒 Files selected for processing (13)
.gitattributesREADME.mddocs/CAPABILITY_SURFACE_DECISION.mddocs/CODE_INTEL.mddocs/MCP_SERVER.mdgo/internal/codeintel/skill_health.gogo/internal/codeintel/skill_health_test.gogo/internal/codeintelcli/main.gogo/internal/codeintelcli/main_internal_test.gogo/internal/mcp/protocol.gogo/internal/mcp/server.gogo/internal/mcp/server_test.gogo/internal/mcp/skill_health.go
📜 Review details
🔇 Additional comments (15)
go/internal/codeintelcli/main.go (1)
45-47: LGTM!Also applies to: 71-71, 104-104
go/internal/codeintelcli/main_internal_test.go (1)
17-18: LGTM!Also applies to: 45-146
go/internal/codeintel/skill_health.go (1)
122-123: LGTM!Also applies to: 212-213, 385-386, 475-475
go/internal/codeintel/skill_health_test.go (1)
103-104: LGTM!Also applies to: 114-140
.gitattributes (1)
8-8: LGTM!docs/CAPABILITY_SURFACE_DECISION.md (1)
14-51: 📐 Maintainability & Code Quality | ⚡ Quick winSurface decision tree accurately grounds skill health across MCP, CLI, and code-intel storage.
The decision tree (question 5 at line 37-40) correctly identifies that durable evidence, audit, and trend analysis require SARIF/code-intel storage—precisely how skill health observations are persisted and reported. Question 3 (line 25-29) correctly routes interactive structured agent access to MCP, grounding the
code_intel_skill_healthtool. Question 4 (line 31-35) justifies the CLIskill-healthcommand as a one-shot local operation. The guide documents why multiple surfaces own parts of the skill health feature—a pattern the PR follows.docs/CODE_INTEL.md (2)
192-200: 📐 Maintainability & Code Quality | ⚡ Quick winSkill health measurement model clearly documented.
The new "Skill use" section (lines 192-200) accurately documents the measurement-only behavioral model: unknown-outcome observations from skill_lookup/recommend when code-intel is configured, explicit remediation outcomes for success/failure/abandonment, and 7d/30d windowed health reports. The clarification that learned/evolved skills are excluded from repo artifact promotion without explicit separate policy change aligns precisely with PR objectives and prevents scope creep.
280-283: 📐 Maintainability & Code Quality | ⚡ Quick winCLI command and MCP tool documentation is consistent with README and matches implementation.
The
skill-health --format toonCLI command (line 283) andcode_intel_skill_healthMCP tool (line 605-607) are consistently documented across CODE_INTEL.md, README.md, and MCP_SERVER.md. Both mention 7-day/30-day windows and unknown skill ID handling. The integration into the existing command/tool lists follows established patterns.Also applies to: 605-607
docs/MCP_SERVER.md (1)
82-84: 📐 Maintainability & Code Quality | ⚡ Quick winTool documentation and dataflow clearly explain skill health measurement model.
The
code_intel_skill_healthtool entry (lines 82-84) and dataflow explanation (lines 122-128) consistently describe the measurement-only behavior across all three documentation files. The dataflow correctly identifies that unknown-outcome observations flow from skill_lookup/skill_recommend through code-intel storage, thatcode_intel_skill_healthreads both observations and remediation outcomes, and that the result is measurement-only with no artifact promotion. The placement in the tool list (betweencode_intel_change_riskandcode_intel_health) is logically ordered.Also applies to: 122-128
go/internal/mcp/protocol.go (1)
312-330: LGTM!Also applies to: 500-512, 942-1022, 1697-1741
go/internal/mcp/server.go (2)
196-239: LGTM!Also applies to: 242-330
938-942: LGTM!Also applies to: 981-988
go/internal/mcp/skill_health.go (1)
12-16: LGTM!Also applies to: 141-148
go/internal/mcp/server_test.go (2)
56-57: LGTM!Also applies to: 75-77, 113-116, 119-190
338-397: LGTM!
🛑 Comments failed to post (1)
docs/CAPABILITY_SURFACE_DECISION.md (1)
74-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Add explicit grounding requirement for skill health surfaces.
The minimum grounding checklist (lines 74-84) lists MCP tools, CLI commands, policy IDs, provider behavior, and output contracts, but does not explicitly mention code-intel storage contracts. Since skill health observations are persisted in code-intel tables (remediation_outcomes), the checklist should clarify that new measurement/outcome features must document their code-intel storage contract alongside MCP/CLI surfaces. Optionally add a new bullet: "New code-intel measurement features must document their storage table, schema, and windowing contract in the code-intel capability docs."
🧰 Tools
🪛 LanguageTool
[style] ~79-~79: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... must appear indocs/CODE_INTEL.md. - New policy IDs must live with their ETHOS p...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/CAPABILITY_SURFACE_DECISION.md` around lines 74 - 84, The minimum grounding checklist under "Minimum grounding by category" is missing an explicit requirement for code-intel storage contracts related to skill health surfaces. Add a new bullet point to the existing list of grounding requirements (which currently covers MCP tools, CLI commands, policy IDs, provider behavior, and output contracts) that specifically requires new code-intel measurement features to document their storage table, schema, and windowing contract in the code-intel capability documentation. This ensures that skill health observations persisted in code-intel tables like remediation_outcomes are properly grounded and discoverable.
Summary
Validation
Closes #183