chore(ai-integrations): add openspec for RHIDP-15200 KFMR client code removal#3718
Conversation
gabemontero
requested review from
a team,
Lucifergene,
johnmcollier,
karthikjeeyar and
rohitkrai03
as code owners
|
🤖 Finished Review · ✅ Success · Started 6:46 PM UTC · Completed 6:51 PM UTC |
PR Summary by QodoAdd OpenSpec (proposal/design/tasks/spec) for removing KFMR client code
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
14 rules 1.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3718 +/- ##
=======================================
Coverage 54.59% 54.59%
=======================================
Files 2361 2361
Lines 90132 90132
Branches 25209 25209
=======================================
Hits 49204 49204
+ Misses 39410 39400 -10
- Partials 1518 1528 +10
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
rhdh-qodo-merge
Bot
added
the
documentation
Review — approveSummaryDocumentation-only PR adding an openspec for the planned KFMR client code removal (RHIDP-15200). Five new files are added under
Verification of factual claimsAll claims in the spec documents were verified against the current source tree:
Structure conformanceThe file layout ( AssessmentNo findings. This is a well-structured planning document that accurately describes the current codebase state and the intended changes. The design decisions are sound, the task breakdown is thorough, and the verification spec covers the key invariants (no KFMR remnants, shared utilities preserved, build/test pass). Labels: Documentation-only PR adding planning/specification documents |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
the
ready-for-merge
Adds proposal, design, tasks, and post-removal verification spec for removing KubeFlow Model Registry client code from the kserve-kubeflow-connector-backend plugin per RHDHPLAN-404 Decision 4. Key design insight: shared utilities (PropertyKeys, NormalizerFormat, helper functions) must be relocated from Kfmr.ts to types.ts before deletion since KServe.ts depends on them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rajin-kichannagari
left a comment
rajin-kichannagari
left a comment
There was a problem hiding this comment.
The line range for processKFMR in tasks.md (task 3.6) says ~lines 285-465 but fullsend's own verification shows it actually goes from 286 to 623. So the function is ~337 lines, not ~180 like proposal.md says. if someone follows the task literally they'd only delete half the function
… openspec The openspec documents for RHIDP-15200 were conflating removal of KubeFlow Model Registry code with removal of KubeFlow Model Catalog code. Only Model Registry should be removed; Model Catalog integration must be preserved. Key changes across all four documents: - Catalog route discovery, getModelCard(), CatalogModel type, and catalog-related PropertyKeys are now explicitly marked as preserved and relocated (not deleted) - kfmrCatalogRoute renamed to catalogRoute on ReconcilerConfig - processKFMR model card fetching logic (catalog API call) preserved - New task group (2.x) for relocating catalog code from Kfmr.ts - New design decision D6 for catalog code preservation - New PRESERVED capability section in spec for Model Catalog integration - Future Work section added describing planned Model Catalog enhancements: inference-service-driven catalog lookup and full catalog polling Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gabemontero
force-pushed
the
rhidp-15200-openspecs
branch
from
b382bb7 to
f6f2968
Compare
Qodo review correctly identified that verification steps referenced kserve-kubeflow-connector-backend/src/ without the plugins/ prefix. The actual path from the workspace root is plugins/kserve-kubeflow-connector-backend/src/. Fixed 5 instances across tasks.md, spec.md, and design.md. Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Valid finding from Qodo — the verification paths in the openspec docs were missing the |
Rajin correctly identified that processKFMR spans lines 287-625 (~340 lines), not ~180 as originally stated. Fixed the remaining instance in proposal.md Starting Point section. Signed-off-by: gabemontero <gmontero@redhat.com> Assisted-by: Claude Opus 4.6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gabemontero
left a comment
gabemontero
left a comment
There was a problem hiding this comment.
Good catch @rajin-kichannagari — both issues are now fixed:
- The "~180 lines" in proposal.md is corrected to "~340 lines, lines 287-625" in commit 3186811.
- The old task 3.6 with the wrong "~lines 285-465" range was already rewritten (now task 4.6 without a truncated line range) in commit a5447b710c, as part of a larger rework to properly distinguish Model Registry code (remove) from Model Catalog code (preserve).
|
|
also @rajin-kichannagari - if possible, try to use the in-line review comments feature of github reviews ... makes it easier not only to interpret your comment, but verify if subsequent pushes altered the lines you are referring to thanks |
|
also @rajin-kichannagari @johnmcollier right before the update to the comments from qodo and Rajin, I noticed that the openspec's did not properly capture that we wanted to keep the model CATALOG client updates for that have been pushed as well |
3 participants
Adds proposal, design, tasks, and post-removal verification spec for removing KubeFlow Model Registry client code from the kserve-kubeflow-connector-backend plugin per RHDHPLAN-404 Decision 4.
Key design insight: shared utilities (PropertyKeys, NormalizerFormat, helper functions) must be relocated from Kfmr.ts to types.ts before deletion since KServe.ts depends on them.
Hey, I just made a Pull Request!
✔️ Checklist