OLS-2882 Align spec structure and fix health issues#2000
Conversation
📝 WalkthroughWalkthroughThis PR consolidates and reorganizes project specifications and documentation: it rewrites the spec index, updates project-structure notes and planned-feature tables, adds a top-level contributor pointer, and introduces a new ARCHITECTURE.md describing runtime patterns and design decisions. ChangesDocumentation and Specification System
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.ai/spec/how/project-structure.md (1)
66-80:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix duplicate heading and mislabeled CSS section.
Line 74 repeats the same heading text, and Lines 70-72 list
src/components/*.cssunder asrc/assets/heading. This makes the module map ambiguous and can trigger markdown lint noise.Suggested doc fix
-### `src/assets/` -- Static assets +### `src/components/` -- Component CSS assets | Path | Purpose | |---|---| | `src/components/general-page.css` | Styles for the GeneralPage chat interface | | `src/components/mcp-app.css` | Styles for MCP App card and iframe container | | `src/components/popover.css` | Styles for the Popover modal and floating button | ### `src/assets/` -- Static assetsAs per coding guidelines: “All specifications live in
.ai/spec/. Start with.ai/spec/README.mdfor project overview, reading order, and structure guide.”🤖 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 @.ai/spec/how/project-structure.md around lines 66 - 80, The markdown has a duplicated "src/assets/" heading and the components CSS entries are miscategorized; change the first heading (the one above the CSS table) from "src/assets/" to "src/components/" (or remove the duplicate and rename accordingly), leave the second "src/assets/" section for logo/user asset rows only, and ensure the `src/components/*.css` entries (GeneralPage, mcp-app, popover) appear under the corrected "src/components/" heading so the module map and linting are consistent.
🤖 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.
Outside diff comments:
In @.ai/spec/how/project-structure.md:
- Around line 66-80: The markdown has a duplicated "src/assets/" heading and the
components CSS entries are miscategorized; change the first heading (the one
above the CSS table) from "src/assets/" to "src/components/" (or remove the
duplicate and rename accordingly), leave the second "src/assets/" section for
logo/user asset rows only, and ensure the `src/components/*.css` entries
(GeneralPage, mcp-app, popover) appear under the corrected "src/components/"
heading so the module map and linting are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8c057fc8-6a26-4007-9886-59e5044dd735
📒 Files selected for processing (9)
.ai/spec/README.md.ai/spec/health-report.md.ai/spec/how/README.md.ai/spec/how/project-structure.md.ai/spec/what/README.md.ai/spec/what/attachments.md.ai/spec/what/tools.mdAGENTS.mdARCHITECTURE.md
💤 Files with no reviewable changes (4)
- .ai/spec/how/README.md
- .ai/spec/what/tools.md
- .ai/spec/what/attachments.md
- .ai/spec/what/README.md
| @@ -0,0 +1,35 @@ | |||
| # Spec health report | |||
There was a problem hiding this comment.
Seems like this file should not be part of the PR, but these details could be included in the commit message / PR description instead.
There was a problem hiding this comment.
@kyoto thank you . Fixed , Please take a look
openshift-ci
Bot
added
the
needs-rebase
|
@xrajesh LGTM, but could you please resolve the conflicts and squash the commits? /approve |
openshift-ci
Bot
added
the
approved
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
xrajesh
force-pushed
the
xav/spec-first-update
branch
from
73a0b94 to
78f4a2e
Compare
openshift-ci
Bot
removed
the
needs-rebase
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kyoto, xrajesh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ARCHITECTURE.md (1)
147-147: 💤 Low valueConsider simplifying the redundant phrasing.
The phrase "adds an additional" contains redundancy. Consider either "adds a proxy" or "adds another proxy" for clearer writing.
✏️ Suggested simplification
-**Plugin proxy for all API calls** — The plugin never connects directly to the OLS service. The console's proxy handles authentication, TLS, and routing. In development, `start-console.sh` adds an additional proxy from the console container to `localhost:8080`. +**Plugin proxy for all API calls** — The plugin never connects directly to the OLS service. The console's proxy handles authentication, TLS, and routing. In development, `start-console.sh` adds a proxy from the console container to `localhost:8080`.🤖 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 `@ARCHITECTURE.md` at line 147, The sentence under "**Plugin proxy for all API calls**" is redundant — change "start-console.sh adds an additional proxy from the console container to `localhost:8080`" to a simpler phrasing such as "start-console.sh adds a proxy from the console container to `localhost:8080`" (or "adds another proxy") to remove the unnecessary word "additional"; update the ARCHITECTURE.md text accordingly.
🤖 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.
Nitpick comments:
In `@ARCHITECTURE.md`:
- Line 147: The sentence under "**Plugin proxy for all API calls**" is redundant
— change "start-console.sh adds an additional proxy from the console container
to `localhost:8080`" to a simpler phrasing such as "start-console.sh adds a
proxy from the console container to `localhost:8080`" (or "adds another proxy")
to remove the unnecessary word "additional"; update the ARCHITECTURE.md text
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b305aa71-88bc-4b1e-8fe4-52a8f958aee7
📒 Files selected for processing (8)
.ai/spec/README.md.ai/spec/how/README.md.ai/spec/how/project-structure.md.ai/spec/what/README.md.ai/spec/what/attachments.md.ai/spec/what/tools.mdAGENTS.mdARCHITECTURE.md
💤 Files with no reviewable changes (4)
- .ai/spec/what/README.md
- .ai/spec/how/README.md
- .ai/spec/what/tools.md
- .ai/spec/what/attachments.md
✅ Files skipped from review due to trivial changes (3)
- AGENTS.md
- .ai/spec/how/project-structure.md
- .ai/spec/README.md
|
/lgtm |
3 participants
Summary
.ai/spec/README.mdwith Structure table, Cross-Reference table, and standard Conventions sectionwhat/README.mdandhow/README.mdinto the main spec README and remove themARCHITECTURE.mdwith Mermaid diagrams (system context, component architecture, data flow)## Specspointer toAGENTS.mdCloseButton.tsx/Modal.tsxrefs, add missingpageContext.ts/validation.ts/CSS files to module map, remove completed tickets from Planned ChangesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit