RFC: gRPC API Consolidation (106 → 60 methods)#65
Conversation
Analysis of the current 106-method API surface and proposal to consolidate to 60 methods (43% reduction). Key changes: - Generic permission methods with resource/actor type enums (36→4) - Unified entity CRUD for projects/environments (8→4) - Merged invite patterns for workspace/org (9→5) - Unified member management for groups/orgs (9→5) - Remove redundant CountAuditLogs method Includes: - Complete method breakdown by category - Breaking change assessment - Migration path with deprecation strategy - Implementation notes
There was a problem hiding this comment.
2 issues found across 1 file
Confidence score: 4/5
- This PR looks safe to merge; the only issues are low-severity documentation inconsistencies.
docs/rfcs/grpc-consolidation.mdSummary lists a ~55% reduction that doesn’t match the 60‑method (43%) calculation, which could mislead readers.docs/rfcs/grpc-consolidation.mdtarget of <50 methods conflicts with the recommended 60‑method outcome, creating ambiguity about the intended goal.- Pay close attention to
docs/rfcs/grpc-consolidation.md- align the Summary and target statements with the proposed reduction.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="docs/rfcs/grpc-consolidation.md">
<violation number="1" location="docs/rfcs/grpc-consolidation.md:6">
P3: The stated target (<50 methods) conflicts with the RFC’s recommended outcome (60 methods). Align the target statement with the recommended plan to avoid ambiguity about the intended reduction goal.</violation>
<violation number="2" location="docs/rfcs/grpc-consolidation.md:10">
P3: The reduction percentage in the Summary is inconsistent with the later calculations (60 methods is a 43% reduction, not ~55%). Update the Summary to match the actual proposed reduction.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
vieiralucas
left a comment
There was a problem hiding this comment.
Reviewed the RFC and verified against the actual proto file.
Verification results:
- ✅ Method count: Confirmed 106 methods in
zopp.proto - ✅ Permission method count: Confirmed 36 methods (3 actor types × 3 resource levels × 4 operations)
- ✅ All category breakdowns in the RFC match the actual codebase
- ✅ Proto examples follow correct protobuf conventions (enums starting at 0, proper oneof usage, standard message structures)
- ✅ Consolidation math: 106 - 46 = 60 methods (43% reduction)
The analysis is accurate and the consolidation approach is well-designed:
- Generic permission methods with ResourceType/ActorType enums is the right pattern
- The phased migration path (deprecation → removal) is sensible
- Stopping at 60 methods rather than pushing to <50 is the right call — preserves semantic clarity
LGTM 👍
- Target: <50 methods → 60 methods (matches recommendation) - Reduction: ~55% → 43% (matches actual calculation) Fixes Cubic P3 issues
|
Skipping RFC — implementing directly. |
|
@cubic-dev-ai Please do a full re-review of the PR. |
@vieiralucas I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 1 file
Confidence score: 5/5
- Only minor documentation inconsistencies in
docs/rfcs/grpc-consolidation.md; no functional or code-impacting changes noted, so merge risk is low. - Update the RFC headings to align with the accurate method counts to avoid misleading readers (47 vs 72, and 18→8 vs 18→17).
- Pay close attention to
docs/rfcs/grpc-consolidation.md- heading method counts should match the table and described savings.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="docs/rfcs/grpc-consolidation.md">
<violation number="1" location="docs/rfcs/grpc-consolidation.md:155">
P3: The consolidation header says “18 → 8 methods,” but the proposal only removes one method (18 → 17). Update the header to match the described savings to avoid misleading readers.</violation>
<violation number="2" location="docs/rfcs/grpc-consolidation.md:240">
P3: The header says “47 Methods,” but the table and the corrected recount below show 72. Update the heading to match the accurate count to avoid contradictions in the RFC.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| ## Consolidated API Summary | ||
|
|
||
| ### Methods After Consolidation: 47 Methods |
There was a problem hiding this comment.
P3: The header says “47 Methods,” but the table and the corrected recount below show 72. Update the heading to match the accurate count to avoid contradictions in the RFC.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/rfcs/grpc-consolidation.md, line 240:
<comment>The header says “47 Methods,” but the table and the corrected recount below show 72. Update the heading to match the accurate count to avoid contradictions in the RFC.</comment>
<file context>
@@ -0,0 +1,518 @@
+
+## Consolidated API Summary
+
+### Methods After Consolidation: 47 Methods
+
+| Category | Before | After | Change |
</file context>
|
|
||
| --- | ||
|
|
||
| ### 2. Unified CRUD Pattern for Organizations (18 → 8 methods) |
There was a problem hiding this comment.
P3: The consolidation header says “18 → 8 methods,” but the proposal only removes one method (18 → 17). Update the header to match the described savings to avoid misleading readers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/rfcs/grpc-consolidation.md, line 155:
<comment>The consolidation header says “18 → 8 methods,” but the proposal only removes one method (18 → 17). Update the header to match the described savings to avoid misleading readers.</comment>
<file context>
@@ -0,0 +1,518 @@
+
+---
+
+### 2. Unified CRUD Pattern for Organizations (18 → 8 methods)
+
+Organization-related methods follow a pattern that can be slightly consolidated.
</file context>
|
@cubic-dev-ai Please do a full re-review of the PR. |
@vieiralucas I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 5/5
- Low-risk change overall; the only noted issue is a documentation inconsistency in
docs/rfcs/grpc-consolidation.md, which does not affect runtime behavior. - The RFC’s stated method count (47) conflicts with the table and later recount (72), which could confuse reviewers but is easy to fix.
- Pay close attention to
docs/rfcs/grpc-consolidation.md- reconcile the method count to avoid ambiguity.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="docs/rfcs/grpc-consolidation.md">
<violation number="1" location="docs/rfcs/grpc-consolidation.md:240">
P2: The consolidated method count is inconsistent. This section says "47 Methods" but the table and later recount show 72 methods, which contradicts the stated total and makes the RFC hard to validate.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| ## Consolidated API Summary | ||
|
|
||
| ### Methods After Consolidation: 47 Methods |
There was a problem hiding this comment.
P2: The consolidated method count is inconsistent. This section says "47 Methods" but the table and later recount show 72 methods, which contradicts the stated total and makes the RFC hard to validate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/rfcs/grpc-consolidation.md, line 240:
<comment>The consolidated method count is inconsistent. This section says "47 Methods" but the table and later recount show 72 methods, which contradicts the stated total and makes the RFC hard to validate.</comment>
<file context>
@@ -0,0 +1,518 @@
+
+## Consolidated API Summary
+
+### Methods After Consolidation: 47 Methods
+
+| Category | Before | After | Change |
</file context>
Summary
Analysis of the current gRPC API (106 methods) with a concrete proposal to consolidate to 60 methods — a 43% reduction.
The Problem
The API has grown to 106 methods, with 36 methods (34%) dedicated to permission management across:
Proposed Solution
Primary Consolidation: Generic Permissions (36 → 4 methods)
Replace 36 specific permission methods with 4 generic methods using
ResourceTypeandActorTypeenums:Additional Consolidations
What's in this PR
This is analysis only — the RFC document at
docs/rfcs/grpc-consolidation.mdcontains:Next Steps (after RFC approval)
Summary by cubic
Added an RFC proposing consolidation of the gRPC API from 106 to 60 methods to simplify permissions and reduce duplication. This is analysis-only and includes a deprecation-based migration plan.
Written for commit 5548500. Summary will update on new commits.