Skip to content

OSAC-2921: PRD — Add standardized display_name and description fields to resource Metadata - #141

Merged
openshift-merge-bot[bot] merged 5 commits into
osac-project:mainfrom
udis:prd/OSAC-2921
Jul 29, 2026
Merged

openshift-merge-bot[bot] merged 5 commits into
osac-project:mainfrom
udis:prd/OSAC-2921

Conversation

@udis

@udis udis commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

PRD: Add standardized display_name and description fields to resource Metadata

Jira: https://redhat.atlassian.net/browse/OSAC-2921

Summary

This PRD proposes adding display_name (max 63 chars) and description (max 256 chars) as optional fields on the shared Metadata struct, giving every OSAC resource type a consistent, user-friendly naming mechanism. Existing per-resource title/description fields are removed from all 12 resource types that currently have them (including flat-shape platform-defined resources such as templates, catalog items, NetworkClass, and HostType) in favor of the shared Metadata fields.

Requesting Review On

  • Reconciliation strategy: removing resource-level title/description from all affected resource types
  • Scope boundaries: identity stays on metadata.name; display behavior deferred to UX/design
  • User stories completeness across Cloud Provider Admin, Cloud Infrastructure Admin, Tenant Admin, and Tenant User personas
  • Validation constraints: 63-char display_name and 256-char description limits

How to Review

  • Comment inline on specific sections
  • Approve when the PRD accurately reflects the agreed requirements

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Adds a PRD defining shared metadata.display_name and metadata.description fields, resource-specific field reconciliation, filtering and sorting requirements, scope boundaries, and implementation dependencies.

Changes

Metadata display name specification

Layer / File(s) Summary
Shared metadata behavior and adoption scope
enhancements/OSAC-2921-metadata-display-name/prd.md
Defines optional mutable and clearable metadata fields, validation limits, resource-specific field removals, filtering and sorting requirements, client display scope, exclusions, and implementation dependencies.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested reviewers: masayag, ronniel1

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets ✅ Passed The only changed file is a PRD markdown doc, and it contains no hardcoded secrets, embedded-credential URLs, private keys, or secret-like literals.
No-Weak-Crypto ✅ Passed Only changed file is a PRD markdown doc; no weak-crypto algorithms, comparisons, or custom crypto code are present.
No-Injection-Vectors ✅ Passed Only a markdown PRD changed; no code paths or patterns like eval, shell, pickle, yaml.load, os.system, or dangerouslySetInnerHTML were introduced.
Container-Privileges ✅ Passed No container/K8s manifests are present in the PR; only a markdown PRD was added, and no privilege flags were found in the tree.
No-Sensitive-Data-In-Logs ✅ Passed Only a PRD markdown file changed; no logging code or sensitive data exposure was added.
Ai-Attribution ✅ Passed All target-file commits use Assisted-by: Claude Code trailers; no Co-Authored-By AI attribution appears.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the PRD’s main change: adding standardized display_name and description fields to resource metadata.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

AI EP Review: EP-141

Score: 9/10 | Verdict: PASS

Criterion Score Notes
What 2/2 Clear user-facing need. The PRD describes adding display_name and description to shared Metadata with specific behaviors (optional, mutable, clearable, max lengths). Three personas (Cloud Provider Admin, Tenant Admin, Tenant User) each have concrete user stories. In Scope and Out of Scope are well-defined with clarification references. Minor gap: no explicit service scoping (BMaaS/CaaS/VMaaS/MaaS/Enclave) and Cloud Infrastructure Admin persona not addressed.
Why 1/2 The problem statement describes real pain — DNS-label constraints on metadata.name, inconsistency across resource types, repeated per-resource discussions, and uneven UX — but stays at a qualitative level. No quantified impact (e.g., how many resources lack friendly names, how often users hit this), no strategic tie-in, no customer evidence. Justification is plausible but generic.
How 2/2 The approach is specific and measurable. Field constraints are defined (63-char display_name, 256-char description). Behaviors are precise (optional, mutable, clearable). UI fallback logic is explicit (show display_name when set, fall back to metadata.name). Filter/sort across UI, CLI, and API is stated. E2E test coverage is scoped. No design leakage — the PRD describes user-observable outcomes without prescribing controllers, reconcilers, or internal components.
Task 2/2 This is a proper product feature enhancement — adds a new platform capability (standardized friendly naming across all resource types) with API, UI, and behavioral changes. Not a bug, chore, task, or documentation-only change.
Size 2/2 Well-scoped and cohesive. Adding display_name/description, UI fallback behavior, filter/sort, and cleanup of duplicate spec-level fields are all interdependent parts of one capability. None could ship independently and provide full value. No unrelated work bundled.

Verdict: A clean, well-structured PRD with clear user-facing outcomes, strong persona coverage, and no design leakage — held back only by a generic business justification that lacks quantified impact or strategic tie-in.

Feedback: Strengthen the WHY: add concrete evidence such as the number of resource types currently lacking friendly names, user feedback or support tickets about the limitation, or tie to a strategic goal like UI consistency for a specific milestone or customer adoption blocker. Also declare which OSAC services are in scope (or state 'all services') and explicitly note whether the Cloud Infrastructure Admin persona is affected or unaffected.

Critical (0)

None.

Important (3)

  1. WHY is qualitative only — the problem statement describes the pain (DNS-label constraints, inconsistency) but provides no quantified impact, customer evidence, or strategic tie-in. Compare calibration example Y=1 ('describes the gap but no impact'). Adding a concrete consequence (e.g., 'X of Y resource types lack friendly names, blocking UI consistency for milestone Z') would strengthen this to a 2.
  2. No explicit OSAC service scoping — the PRD does not declare which services (BMaaS, CaaS, VMaaS, MaaS, Enclave) are in scope, as required by osac-dimensions.md. The feature applies broadly ('every resource type') but this should be stated explicitly.
  3. Cloud Infrastructure Admin persona is absent — osac-dimensions.md lists four canonical personas. The PRD covers three but does not mention Cloud Infrastructure Admin. If unaffected, state so explicitly; if affected (e.g., managing NetworkClass display names), add a user story.

Suggestions (3)

  1. Add a target milestone declaration per osac-dimensions.md milestone scoping guidance.
  2. Consider noting upgrade/migration posture — even if OSAC does not currently support upgrades, the removal of existing spec-level title/description fields from Project, Role, IdentityProvider, and InstanceType implies a data migration concern worth flagging.
  3. The Dependencies section could be expanded to note UI (osac-ui) and E2E test (osac-test-infra) dependencies beyond fulfillment-service.

Review cost

Model: claude-opus-4-6
Cost: $0.2562
Tokens: 6 in / 3.4k out
Cache: 192.1k read
Active time: 1m 19s
API calls: 0

@github-actions github-actions Bot added the rfe-creator-auto-reviewed EP was reviewed by AI label Jul 22, 2026
@udis
udis marked this pull request as ready for review July 22, 2026 13:12
@openshift-ci
openshift-ci Bot requested review from masayag and ronniel1 July 22, 2026 13:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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 `@enhancements/OSAC-2921-metadata-display-name/prd.md`:
- Around line 18-19: Update the requirements around display_name filtering,
sorting, and API responses to explicitly define effective-name behavior across
UI, CLI, and API: use display_name when set and metadata.name otherwise for
filtering and sorting, and state whether APIs expose the raw optional
display_name, the effective label, or both. Ensure all interfaces follow the
same documented behavior.
- Around line 28-29: Normalize the resource name casing across the two scope
statements: use one canonical spelling for the bare-metal instance template in
both the template-parameter and flat-shape resource lists. Update only the
inconsistent BaremetalInstanceTemplate/BareMetalInstanceTemplate references in
the document.
- Around line 16-17: Expand the reconciliation requirements for removed
spec.title and spec.description fields for Project, Role, IdentityProvider, and
InstanceType: define migration of existing values into metadata, precedence when
both legacy spec fields and new metadata fields are present, and compatibility
behavior for legacy reads and updates. Preserve the stated optional, mutable,
and clearable semantics.
- Around line 15-17: Clarify the metadata validation and clearing contract for
display_name and description: specify whether maximum lengths count Unicode
characters or bytes, define distinct behavior for empty strings, null, omitted
fields, and whitespace-only values, and state whether length validation occurs
before or after normalization. Update the surrounding PRD requirements so these
semantics are explicit and testable.
- Around line 15-16: Clarify the metadata adoption and precedence rules for
flat-shape resources in the PRD, including whether they expose both shared
display_name/description and existing title/description fields. Define which
field is authoritative for UI display, API responses, filtering, and sorting
when both are present, and align the related statements at lines 15 and 28-29.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 066a7618-ea32-4b35-afd6-51f1af629003

📥 Commits

Reviewing files that changed from the base of the PR and between 7b5da57 and e607bf2.

📒 Files selected for processing (1)
  • enhancements/OSAC-2921-metadata-display-name/prd.md

Comment thread enhancements/OSAC-2921-metadata-display-name/prd.md Outdated
Comment thread enhancements/OSAC-2921-metadata-display-name/prd.md Outdated
Comment thread enhancements/OSAC-2921-metadata-display-name/prd.md Outdated
Comment thread enhancements/OSAC-2921-metadata-display-name/prd.md Outdated
Comment thread enhancements/OSAC-2921-metadata-display-name/prd.md Outdated
- Removal of existing `title` and `description` fields from the spec of spec-based resources: Project, Role, IdentityProvider, and InstanceType (description only) `[Clarify: R1.Q1, R4.Q2, R5.Q2]`
- Both fields are optional, mutable after creation, and clearable `[Clarify: R3.Q1]`
- Users can filter and sort resource lists by `display_name` across UI, CLI, and API `[Clarify: R2.Q2]`
- UI displays `display_name` in place of `metadata.name` when set; falls back to `metadata.name` when `display_name` is not set — this applies uniformly across list views, detail pages, breadcrumbs, and search results `[Clarify: R4.Q1, R5.Q1]`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the backend should set display_name to metadata.name when it is not provided, so the UI (and CLI) has a single source for the name to display

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree. The backend should not populate fields for the user. If it did, then during edit the user will not understand why the field has a value when they did not set it. The UI should know to fallback

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good discussion. The PRD keeps the UI-side fallback approach (UI displays metadata.name when display_name is not set) rather than backend auto-population, per ygalblum's point about avoiding user confusion during edits. This can be revisited in the design phase if needed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the CLI do the same?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moot now — we've removed the display fallback requirement entirely and deferred display behavior to the UX team and design phase.


## Out of Scope

- Making `display_name` or `description` required for any resource type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "in scope" section already defines these as optional, so we don't need this item

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated — removed the "Making display_name or description required" item from Out of Scope since the In Scope section already defines them as optional. Thanks for catching the redundancy.


- Making `display_name` or `description` required for any resource type
- Renaming or removing existing `metadata.name` semantics
- Enforcing uniqueness constraints on `display_name`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads more like an "In scope" requirement: "display_name does not have to be unique"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — moved to In Scope as a positive statement: "display_name does not have to be unique across resources".

- Renaming or removing existing `metadata.name` semantics
- Enforcing uniqueness constraints on `display_name`
- Template parameter `title`/`description` fields within ComputeInstanceTemplate, BaremetalInstanceTemplate, and ClusterTemplate — only resource-level fields are affected `[Clarify: R1.Q3]`
- Flat-shape, platform-defined resources that already have top-level `title`/`description` fields: ClusterTemplate, ComputeInstanceTemplate, BareMetalInstanceTemplate, NetworkClass, HostType, ComputeInstanceCatalogItem, BareMetalInstanceCatalogItem, and ClusterCatalogItem — these keep their existing fields unchanged `[Clarify: R4.Q2, R5.Q2]`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of having both metadata.display_name and a resource-level title? Could the former replace the latter?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on this. We said that we don't want to keep the resource specific field. This change should remove any such specific field.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those resource are provider-admin persona so that would change the scope, is that ok?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — updated the PRD to remove title/description from ALL resource types, including flat-shape platform-defined resources (NetworkClass, HostType, catalog items, templates). The flat-shape exclusion is reverted. Also added a Cloud Infrastructure Admin user story to cover the persona impact.

udis added 2 commits July 23, 2026 12:04
…fields to resource Metadata

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: ushkalim <ushkalim@redhat.com>
…rces in scope

Revert the flat-shape exclusion per reviewer feedback from sk-ilya and
ygalblum. All 12 resource types with existing title/description fields
now have them removed in favor of shared Metadata display_name and
description. Added Cloud Infrastructure Admin user story, moved
uniqueness to In Scope, and normalized BareMetalInstanceTemplate naming.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: ushkalim <ushkalim@redhat.com>
@github-actions

Copy link
Copy Markdown

AI EP Review: EP-141

Score: 9/10 | Verdict: PASS

Criterion Score Notes
What 2/2 Clear, specific user-facing need. All four OSAC personas (Cloud Provider Admin, Cloud Infrastructure Admin, Tenant Admin, Tenant User) have dedicated sections with concrete user stories. The problem — DNS-label constraints making metadata.name unsuitable as a friendly label, inconsistent workarounds across resource types — is well-articulated. In Scope and Out of Scope are cleanly delineated with clarification traceability tags.
Why 1/2 The problem statement names the pain (DNS-label constraints, inconsistent per-resource workarounds) and a consequence (repeated design discussions, uneven UX). However, it doesn't quantify user impact, cite user feedback, or tie to a strategic goal like adoption or competitive positioning. The justification is plausible but generic — 'uneven user experience' doesn't convey urgency or severity.
How 2/2 The PRD stays user-focused with minimal design leakage. Field constraints (max 63 / max 256 chars), mutability, optionality, and clearability are specified as user-observable behaviors. UI fallback logic (display_name → metadata.name) is described in user terms. Filter and sort capabilities are stated. The Dependencies section mentions 'fulfillment-service proto and server changes' which is minor design leakage but contextually appropriate. No controllers, reconcilers, or internal conditions are
Task 2/2 This is a genuine platform capability enhancement: adding standardized display_name and description fields to the shared Metadata type, migrating existing per-resource fields, adding UI display/fallback behavior, and enabling filter/sort. It requires API, server, and UI changes — not documentation or content.
Size 2/2 Tightly coupled scope. The metadata fields, per-resource field migration, UI fallback behavior, and filter/sort capability all depend on each other — display_name without UI fallback is incomplete, migration without the shared field is meaningless. No independent capabilities that could ship separately.

Verdict: A well-structured PRD with clear user stories across all four personas, specific behavioral requirements, and tight scope — held back only by a generic business justification that doesn't quantify impact or tie to strategic goals.

Feedback: Strengthen the WHY by naming a concrete consequence: what breaks for users today, how many resources lack friendly names, or what adoption/support metric this blocks. A sentence like 'X% of resources have no display name, leading to Y' would move WHY from 1 to 2. Consider explicitly declaring which OSAC services are in scope (implied to be all, since Metadata is shared, but the PRD never states this). The cross-cutting dimensions checklist (installation, E2E testing details, documentation scope) could be addressed more explicitly, even if just to say 'covered by the shared Metadata change with no per-dimension variance.'

Critical (0)

None.

Important (1)

  1. WHY (business justification) is generic: the problem statement describes the gap (DNS-label constraints, inconsistent workarounds) but the consequence is vague ('uneven user experience', 'repeated discussions'). No quantified impact, user feedback, or strategic tie. Adding a concrete consequence — e.g., support burden, user confusion metrics, or adoption blocker — would elevate from 1 to 2.

Suggestions (3)

  1. Explicitly declare which OSAC services are in scope. Since display_name and description are added to shared Metadata, this presumably affects all services (BMaaS, CaaS, VMaaS, MaaS, Enclave), but the PRD never states this.
  2. The Dependencies section ('fulfillment-service proto and server changes must land before UI and E2E test changes') is minor design leakage — consider rephrasing as 'API changes must land before UI and test changes' to stay at the product level.
  3. Cross-cutting dimensions (installation impact, E2E testing scope, documentation plan, UI milestone scope) are mentioned only briefly or implicitly. Even a one-line 'no installation impact — inherited by all resources via shared Metadata' would satisfy the dimensions checklist.

Review cost

Model: claude-opus-4-6
Cost: $0.5564
Tokens: 7 in / 3.9k out
Cache: 202.9k read
Active time: 1m 35s
API calls: 0

@github-actions

Copy link
Copy Markdown

AI Design Review: EP-141

Score: 1/8 | Verdict: FAIL

Criterion Score Notes
Feasibility 0/2 Score 0. The PR contains only a PRD (prd.md), not a design document. There are no implementation details: no proto schemas, no error codes, no validation rules (beyond mentioning 'max 63 characters' / 'max 256 characters'), no data structures, no lifecycle workflow (create/get/list/update/delete), no error handling or failure modes, no risks with concrete mitigations, and no drawbacks section. The PRD mentions removing per-resource title/description fields from 12 resource types but provides zer
Testability 0/2 Score 0. A single In Scope bullet mentions 'E2E test coverage for create, update, and clear of display_name and description across representative resource types' but there is no test plan section. No unit test strategy (e.g., validation logic for max lengths, metadata mutation), no integration test infrastructure (kind cluster setup, database migration testing), no specific e2e scenarios, and no graduation criteria. The review-patterns.md explicitly flags 'Unit and integration tests will be adde
Scope 1/2 Score 1. The PRD has reasonably clear In Scope / Out of Scope boundaries and covers all four OSAC personas with user stories. Dependencies are identified (fulfillment-service must land first). However, when evaluated as a design document: there is no Summary section (3-5 sentences), Goals are not stated as user-visible outcomes separate from implementation, Non-Goals are not in the expected format, there is no Alternatives section at all (a frequent reviewer feedback theme per review-patterns.md
Architecture 0/2 Score 0. No architectural content is present. There are no proto schemas showing the updated Metadata message, no description of how owner-reference and tenant annotations interact with the new fields, no controller pattern discussion, no cross-repo change enumeration (which repos need changes and in what order beyond a single dependency bullet), no integration description with existing services (how does the generic server handle the field migration? how does the rendering layer update?), no te

Verdict: This PR submits a PRD (prd.md) but no design document (design.md) — when evaluated against the design review rubric, it scores 0 on Architecture, Feasibility, and Testability because it contains none of the required implementation detail, proto schemas, or test plans that a design document must provide.

Feedback: This PRD appears well-structured for its purpose (clear scope, good persona coverage, traceability to clarify rounds), but a design document (design.md) is needed alongside it. The design should include: (1) the updated Metadata proto schema with display_name and description fields, the migration plan for removing per-resource title/description from 12 resource types, and the generic_server.go/rendering changes; (2) a concrete test plan with unit tests for field validation, integration tests for migration, and e2e scenarios; (3) an Alternatives section (e.g., keeping per-resource fields vs. centralizing in Metadata, or using annotations vs. spec fields).

Critical (4)

  1. No design document present: The PR contains only prd.md. The design review rubric requires a design.md with architecture, API schemas, implementation details, and test plans. This is a structural gap — the PRD cannot substitute for a design document.
  2. No proto schema: The updated Metadata message definition with display_name and description fields is not provided. Per the feasibility rubric, proto schemas are required for new or modified resources.
  3. No migration strategy: The PRD lists 12 resource types whose per-resource title/description fields must be removed (Project, Role, IdentityProvider, InstanceType, ClusterTemplate, etc.), but there is no design for how this migration occurs — no discussion of backward compatibility, data migration, or breaking change handling.
  4. No test plan: There is no test plan section. A single bullet mentioning E2E coverage does not constitute a test strategy. The design must specify unit tests (validation, metadata mutation), integration tests (kind cluster, database), and e2e scenarios.

Important (3)

  1. No Alternatives section: review-patterns.md identifies missing alternatives as a frequent feedback anti-pattern. The design should compare centralizing in Metadata vs. keeping per-resource fields, and metadata annotations vs. spec fields.
  2. Cross-cutting dimensions not addressed: UI (display_name fallback rendering, list views, breadcrumbs), Documentation (API reference, user guides), Installation (any schema migration prerequisites), and E2E Testing dimensions from osac-dimensions.md are all relevant but not covered in a design context.
  3. No cross-repo change enumeration: The dependency section mentions fulfillment-service but doesn't enumerate the full set of repos affected (fulfillment-service proto + server, osac-operator CRDs if applicable, osac-ui rendering, osac-test-infra e2e tests) or their merge ordering.

Suggestions (3)

  1. When the design document is written, reference the computeinstance-phase-condition-expansion EP from the reference library as a calibration benchmark — it covers API evolution for existing resources, which is the same pattern as adding fields to shared Metadata.
  2. Consider whether display_name should be indexed for efficient server-side filtering/sorting, and document the database impact in the design.
  3. The PRD's traceability markers ([Clarify: R2.Q1], [PR review: sk-ilya]) are a strength — carry this practice into the design document for architectural decisions.

Review cost

Model: claude-opus-4-6
Cost: $0.3573
Tokens: 6 in / 3.7k out
Cache: 178.3k read
Active time: 1m 30s
API calls: 0

@mhrivnak mhrivnak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, a lot of the value that is being proposed here is a reach. We're not going to add new fields that establish unique identity. The name field is a very common convention that people use with k8s, podman, docker, leading cloud services, etc. without difficulty, so I'm having a hard time seeing that this proposal would make a significant improvement in usability that way. We also already have labels.

I could see that these fields are useful for display purposes. If a human looking at a list of clusters clicks on one, showing them a plain-language name and description for that cluster could be helpful as context. But that's about it. Anything to do with unique identity, sorting, auditing, labeling, etc. seems well-handled with the fields we have. I welcome you to make the case otherwise, but then it would help a lot to get specific and use some examples.

### Cloud Provider Admin

- As a Cloud Provider Admin, I want resources across all tenant organizations to show a consistent, human-readable `display_name` and `description` so that I can quickly identify and audit resources when reviewing or supporting tenants, regardless of resource type.
- As a Cloud Provider Admin, I want to filter and sort resource lists by `display_name` so that I can locate specific resources across tenants without memorizing DNS-label names. `[Clarify: R2.Q2]`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example might help illustrate the value.

DNS-style names don't exactly require "memorization". Things like node03.cluster01.example.com are fairly easy to interpret. When you talk of filtering and sorting, having some structure typically makes that easier.

How would a "Display Name" be helpful, or even better, for filtering and sorting?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea is that if the UI shows the display_name in the list view, it only make sense to allow the user to filter and sort by it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok that makes sense. Maybe then we should remove the part about memorization, and leave this at "As a Cloud Provider Admin, I want to filter and sort resource lists by display_name so that I can find resources by using natural language".

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated per consensus — reworded to "so that I can find resources across tenants using natural-language terms."


### Tenant Admin

- As a Tenant Admin, I want all resource types I manage (VMs, virtual networks, public IPs, security groups, etc.) to support a friendly `display_name` and `description` so that I can label and document resources meaningfully instead of being limited by `metadata.name` restrictions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Labels are a different concept entirely.

Can you be more specific about what restrictions you want to overcome and why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that's a bit of AI slop. Trying to give the tenant admin additional reasons why they would want ti

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded — removed "label" (confused with Kubernetes labels) and clarified the constraint: "so that I can give resources a natural-language name and description that are not constrained to DNS-label format."

### Tenant User

- As a Tenant User, I want to give my resources a friendly `display_name` (up to 63 characters) and `description` when creating them so that I can identify and organize them more easily than relying on the constrained `metadata.name` field. `[Clarify: R2.Q1]`
- As a Tenant User, I want list views to show `display_name` when set and fall back to `metadata.name` when it is not, so that I always see the most useful identifier regardless of whether a display name was provided.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mixing these fields could lead to a confusing UX. I suggest we clearly define what each field is for and stick to it. That said, we can also let the UX experts decide when/if to display each piece of information to users.

name is clearly about establishing human-usable unique identity. Example: dell-xe9680
display_name could be about having a more natural name that aligns with how something is described in the real world. Example: Dell PowerEdge XE9680

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, we can also let the UX experts decide when/if to display each piece of information to users.

That's the behaviour defines by the UI.

name is clearly about establishing human-usable unique identity. Example: dell-xe9680
display_name could be about having a more natural name that aligns with how something is described in the real world. Example: Dell PowerEdge XE9680

This is exactly the point of this PRD. What do you think is missing to make it clear.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's generally a bad UX "to show display_name when set and fall back to metadata.name when it is not". As described, they have different purposes. I'd remove that requirement from this PRD. That doesn't prevent someone from doing it, but at least doesn't mandate it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind removing it and letting the UX make their decision later. At this point, the important part is to facilitate it.
The main reason for this PRD is the fact that some resource had it while other didn't. In the ones that did, some called it title and some display_name. So, we wanted to align it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the UI fallback requirement from the PRD and moved display behavior to Out of Scope — deferred to the UX team and design phase, per the consensus here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — the PRD now makes this distinction explicit: metadata.name for identity (stated in Out of Scope), display_name for natural-language naming. The example you gave (dell-xe9680 vs Dell PowerEdge XE9680) captures it well.

Comment thread enhancements/OSAC-2921-metadata-display-name/prd.md
Comment thread enhancements/OSAC-2921-metadata-display-name/prd.md
@ygalblum

Copy link
Copy Markdown
Contributor

@udis, I don't have any further comments, but @mhrivnak's comments should be addressed (I think mainly reduce the AI slop in the doc).
BTW I'll change the PR's title to have the ticket first as the bot expects

@ygalblum ygalblum changed the title PRD: OSAC-2921 — Add standardized display_name and description fields to resource Metadata OSAC-2921: PRD — Add standardized display_name and description fields to resource Metadata Jul 24, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 24, 2026 •

Copy link
Copy Markdown

@udis: This pull request references OSAC-2921 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "5.0.0" version, but no target version was set.

Details

In response to this:

PRD: Add standardized display_name and description fields to resource Metadata

Jira: https://redhat.atlassian.net/browse/OSAC-2921

Summary

This PRD proposes adding display_name (max 63 chars) and description (max 256 chars) as optional fields on the shared Metadata struct, giving every OSAC resource type a consistent, user-friendly naming mechanism. Existing title/description fields on spec-based resources (Project, Role, IdentityProvider, InstanceType) are removed in favor of the shared fields, while flat-shape platform-defined resources (templates, catalog items, NetworkClass, HostType) keep their existing fields unchanged.

Requesting Review On

  • Reconciliation strategy: removing spec-level title/description from 4 resource types while keeping flat-shape resources unchanged
  • Scope boundaries: whether the flat-shape vs. spec-based split is the right dividing line
  • User stories completeness across Cloud Provider Admin, Tenant Admin, and Tenant User personas
  • Validation constraints: 63-char display_name and 256-char description limits

How to Review

  • Comment inline on specific sections
  • Approve when the PRD accurately reflects the agreed requirements

Summary by CodeRabbit

  • Documentation
  • Added product requirements for optional shared metadata fields: display_name (max 63) and description (max 256).
  • Specified UI and API behavior to prefer display_name over resource name when present, with fallback to the existing name.
  • Defined expectations for mutability/clearing, plus list filtering and sorting by display_name.
  • Clarified which current fields are replaced and what naming semantics remain unchanged.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

…avior

Reframe In Scope to focus on concepts, personas, and coverage per
mhrivnak's feedback. Remove UI fallback requirement and defer display
behavior (how clients present display_name vs metadata.name) to UX
team and design phase per mhrivnak and ygalblum consensus.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: ushkalim <ushkalim@redhat.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@enhancements/OSAC-2921-metadata-display-name/prd.md`:
- Line 19: Remove “E2E test coverage and documentation” from the PRD’s In Scope
list, and track these delivery artifacts in the design or implementation plan
instead. Keep the remaining In Scope items focused on product behavior.
- Line 17: Update the reconciliation statement in the PRD to remove only
spec-level title/description fields from Project, Role, IdentityProvider, and
InstanceType, while retaining existing fields on NetworkClass, HostType,
templates, and catalog items. Split the affected and retained resource lists,
and document how retained flat-shape fields coexist with shared metadata.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a4787f05-e176-45b4-a397-f8dbfaecba8c

📥 Commits

Reviewing files that changed from the base of the PR and between 6aaeb80 and 41a965b.

📒 Files selected for processing (1)
  • enhancements/OSAC-2921-metadata-display-name/prd.md


- Consistent, user-friendly resource naming across all OSAC resource types, all personas, and all client interfaces (API, CLI, Web UI) `[PR review: mhrivnak]`
- Two new shared Metadata fields: `display_name` (optional, max 63 characters) and `description` (optional, max 256 characters) — both mutable, clearable, and not required to be unique `[Clarify: R2.Q1, R3.Q1, R4.Q4, PR review: sk-ilya]`
- Reconciliation of existing per-resource `title`/`description` fields — removed from all 12 resource types that currently have them: Project, Role, IdentityProvider, InstanceType (description only), ClusterTemplate, ComputeInstanceTemplate, BareMetalInstanceTemplate, NetworkClass, HostType, ComputeInstanceCatalogItem, BareMetalInstanceCatalogItem, ClusterCatalogItem `[Clarify: R1.Q1, PR review: sk-ilya, ygalblum]`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Align the removal list with the stated flat-shape boundary.

Line 17 removes fields from NetworkClass, HostType, templates, and catalog items, but the PR objective says only spec-level fields on Project, Role, IdentityProvider, and InstanceType are removed; flat-shape resources retain their existing fields. Split the lists and document how retained fields coexist with shared metadata.

🤖 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 `@enhancements/OSAC-2921-metadata-display-name/prd.md` at line 17, Update the
reconciliation statement in the PRD to remove only spec-level title/description
fields from Project, Role, IdentityProvider, and InstanceType, while retaining
existing fields on NetworkClass, HostType, templates, and catalog items. Split
the affected and retained resource lists, and document how retained flat-shape
fields coexist with shared metadata.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declining this suggestion — it reflects an earlier scope that reviewers explicitly rejected.

@sk-ilya and @ygalblum asked to remove resource-level title/description from all affected resource types (including flat-shape platform resources). That is locked in the PRD (all 12 listed types). The PR description was stale and still described the old flat-shape split; it has been updated to match.

Comment thread enhancements/OSAC-2921-metadata-display-name/prd.md Outdated
@github-actions

Copy link
Copy Markdown

AI EP Review: EP-141

Score: 9/10 | Verdict: PASS

Criterion Score Notes
WHAT (clear need) 2/2 Clear capability: standardized display_name and description fields across all OSAC resource types. All four personas (Cloud Provider Admin, Cloud Infrastructure Admin, Tenant Admin, Tenant User) have explicit user stories grouped under persona headings. The reconciliation of 12 existing per-resource title/description fields is well-scoped. Minor gap: services (BMaaS, CaaS, VMaaS, MaaS, Enclave) are not explicitly declared in scope, though 'all OSAC resource types' is implicitly cross-cutting.
WHY (justification) 1/2 The problem statement names concrete pain — DNS-label constraints make metadata.name unsuitable as a friendly label, and the current mix of per-resource title/description fields vs. no friendly name at all produces an uneven UX. However, the justification stays at the level of developer friction ('forces repeated per-resource-type discussions') and generic UX inconsistency without quantifying business impact, citing user requests, or tying to a strategic goal. Plausible but not compelling.
User-Facing Focus 2/2 The PRD describes user-observable outcomes throughout. Field names (display_name, description) and constraints (max 63/256 chars, optional, mutable, clearable) are user-facing API properties, not internal implementation. The listing of 12 affected resource types is scope clarity for a breaking change (field removal), not design leakage. Only the Dependencies section has mild implementation ordering language ('fulfillment-service proto and server changes'), which is standard PRD structure.
Right-Sized 2/2 Tightly coupled scope: adding shared Metadata fields, reconciling existing per-resource fields, and filtering/sorting by display_name all require each other. Adding fields without removing old ones would increase inconsistency; filtering/sorting is essential for the new field to be useful in lists. No independent capabilities bundled.
Testability 2/2 Every user story is verifiable by using the product: create resources with display_name, verify field presence across resource types, update and clear fields, filter and sort lists. The reconciliation of old fields is testable by confirming removed fields return errors or are absent. Field constraints (max length, optionality) are directly testable via API calls.

Verdict: A solid, focused PRD with clear user-facing capabilities, comprehensive persona coverage, and testable requirements; held back slightly by a generic business justification that describes the gap without quantifying its impact.

Feedback: Strengthen the WHY by adding concrete evidence of user impact — e.g., how many resource types lack friendly names today vs. how many have workarounds, whether this has surfaced as user feedback or blocked adoption, or how it relates to OSAC's competitive positioning. Also explicitly declare which services are in scope (likely all five) per the OSAC dimensions checklist, and consider noting which cross-cutting dimensions are not applicable to show they were considered.

Critical (0)

None.

Important (2)

  1. WHY justification is generic: the problem statement describes the gap (DNS-label constraints, inconsistency across 12 resource types) and names consequences ('forces repeated per-resource-type discussions', 'uneven user experience'), but does not quantify business impact, cite user requests, or tie to a strategic goal. Adding evidence like 'X resource types lack friendly names, blocking tenant self-service adoption' would elevate this from plausible to compelling.
  2. Services not explicitly declared: the OSAC dimensions checklist requires declaring which services (BMaaS, CaaS, VMaaS, MaaS, Enclave) are in scope. The PRD says 'all OSAC resource types' but does not list the affected services, leaving reviewers to infer.

Suggestions (3)

  1. The Dependencies section mentions 'fulfillment-service proto and server changes' — consider rephrasing as 'API definition changes must land before UI and E2E test changes' to keep the PRD free of component-level implementation references.
  2. 'E2E test coverage and documentation' in the In Scope section is a delivery artifact, not a user-facing capability — consider moving it to acceptance criteria or a Definition of Done section.
  3. Briefly note which cross-cutting dimensions (tenant onboarding, installation, networking, storage) are not applicable to this feature, to demonstrate they were considered per the dimensions checklist.

Review cost

Model: claude-opus-4-6
Cost: $0.5992
Tokens: 6 in / 6.0k out
Cache: 150.2k read
Active time: 2m 12s
API calls: 0

Reword Cloud Provider Admin filter/sort story per mhrivnak/ygalblum
consensus (natural-language terms instead of memorization). Reword
Tenant Admin story to remove 'label' term (Kubernetes labels confusion)
and clarify DNS-label format constraint.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: ushkalim <ushkalim@redhat.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
enhancements/OSAC-2921-metadata-display-name/prd.md (2)

18-18: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Define filtering and sorting behavior for unset names.

Because display_name is optional and display fallback is deferred, clarify whether filters match only explicitly set values, where unset values sort, and whether matching is case-sensitive. Otherwise API, CLI, and UI implementations can diverge.

🤖 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 `@enhancements/OSAC-2921-metadata-display-name/prd.md` at line 18, Clarify the
filtering and sorting requirements for the optional display_name in the PRD:
specify whether filters include only explicitly set names, where unset names
appear in sort order, and whether matching is case-sensitive. Document behavior
consistently for API, CLI, and UI implementations.

16-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make validation and clearing semantics testable.

Specify whether limits count Unicode characters or bytes, whether normalization precedes validation, and how omitted, null, empty, and whitespace-only values behave. Without this, services may apply inconsistent validation and clearing behavior.

🤖 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 `@enhancements/OSAC-2921-metadata-display-name/prd.md` at line 16, Update the
shared Metadata field specification for display_name and description to define
whether length limits use Unicode characters or bytes, whether values are
normalized before validation, and the exact handling of omitted, null, empty,
and whitespace-only inputs. Document these rules so validation, mutation, and
clearing behavior are consistent and testable across services.
♻️ Duplicate comments (2)
enhancements/OSAC-2921-metadata-display-name/prd.md (2)

19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move delivery artifacts out of product scope.

Keep E2E coverage and documentation in the design or implementation plan; In Scope should describe product behavior. Based on learnings, these deliverables are not expected in PRD scope sections.

🤖 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 `@enhancements/OSAC-2921-metadata-display-name/prd.md` at line 19, Remove “E2E
test coverage and documentation” from the PRD’s In Scope section. Keep these
delivery artifacts in the design or implementation plan instead, ensuring the
scope describes only product behavior.

Source: Learnings


17-17: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Align the reconciliation scope and define migration behavior.

Line 17 removes fields from flat-shape resources, conflicting with the PR objective that templates, catalog items, NetworkClass, and HostType retain their existing fields. It also does not define how existing values migrate into metadata, which field wins when both exist, or how legacy reads and updates behave. Split the affected and retained resource lists and specify these compatibility rules.

🤖 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 `@enhancements/OSAC-2921-metadata-display-name/prd.md` at line 17, Update the
reconciliation section in the PRD to separate resources whose existing
title/description fields are removed from those that retain them, ensuring
templates, catalog items, NetworkClass, and HostType remain in the retained
list. Define migration of existing values into metadata, precedence when legacy
and metadata values both exist, and compatibility behavior for legacy reads and
updates.
🤖 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 `@enhancements/OSAC-2921-metadata-display-name/prd.md`:
- Line 18: Clarify the filtering and sorting requirements for the optional
display_name in the PRD: specify whether filters include only explicitly set
names, where unset names appear in sort order, and whether matching is
case-sensitive. Document behavior consistently for API, CLI, and UI
implementations.
- Line 16: Update the shared Metadata field specification for display_name and
description to define whether length limits use Unicode characters or bytes,
whether values are normalized before validation, and the exact handling of
omitted, null, empty, and whitespace-only inputs. Document these rules so
validation, mutation, and clearing behavior are consistent and testable across
services.

---

Duplicate comments:
In `@enhancements/OSAC-2921-metadata-display-name/prd.md`:
- Line 19: Remove “E2E test coverage and documentation” from the PRD’s In Scope
section. Keep these delivery artifacts in the design or implementation plan
instead, ensuring the scope describes only product behavior.
- Line 17: Update the reconciliation section in the PRD to separate resources
whose existing title/description fields are removed from those that retain them,
ensuring templates, catalog items, NetworkClass, and HostType remain in the
retained list. Define migration of existing values into metadata, precedence
when legacy and metadata values both exist, and compatibility behavior for
legacy reads and updates.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 47bb6971-1789-4d5b-b777-77ec102df594

📥 Commits

Reviewing files that changed from the base of the PR and between 41a965b and ede0a66.

📒 Files selected for processing (1)
  • enhancements/OSAC-2921-metadata-display-name/prd.md

@github-actions

Copy link
Copy Markdown

AI EP Review: EP-141

Score: 9/10 | Verdict: PASS

Criterion Score Notes
WHAT (clear need) 2/2 Clear user-facing need. The PRD describes a concrete new capability: standardized display_name and description fields across all OSAC resource types, plus filtering/sorting and reconciliation of existing inconsistent fields. All four personas (Cloud Provider Admin, Cloud Infrastructure Admin, Tenant Admin, Tenant User) have dedicated user story sections with 'As a ...' stories. Affected resources are enumerated. Services are implicitly all (metadata is cross-cutting) though not explicitly named.
WHY (justification) 1/2 The problem statement names the pain (DNS-label constraints making metadata.name unsuitable as a user-friendly label) and describes concrete consequences (inconsistency forces repeated per-resource-type discussions, uneven UX across VMs/networks/IPs). However, it does not quantify impact, cite user feedback, or tie to a strategic goal like platform adoption or competitive positioning. The justification is solid but stays at the 'this is inconsistent' level without connecting to business outcomes
User-Facing Focus 2/2 The PRD is clean and user-focused. In Scope describes user-observable capabilities (field constraints, mutability, filtering/sorting). User stories describe what each persona can do. The only mild leak is the Dependencies section mentioning 'fulfillment-service proto and server changes,' but this is sequencing context, not prescriptive design. No controllers, reconcilers, playbook parameters, or internal conditions are named. Platform vocabulary (ComputeInstance, VirtualNetwork, etc.) is used ap
Right-Sized 2/2 Tightly coupled scope. The three capabilities — adding metadata fields, reconciling existing per-resource title/description fields, and filtering/sorting by display_name — all require each other. Adding fields without reconciling old ones would worsen inconsistency. Filtering without the fields is meaningless. This is one coherent feature.
Testability 2/2 All requirements are verifiable by using the product. A QA engineer can: create a resource with display_name and description, verify max-length constraints (63/256 chars), update and clear the fields, filter/sort by display_name, and check that the 12 previously inconsistent resource types now use the shared metadata fields. No requirement describes internal behavior or system internals.

Verdict: A well-structured, user-focused PRD with strong persona coverage and coherent scope; the only weakness is a business justification that describes the inconsistency without connecting it to a strategic goal or quantified user impact.

Feedback: Strengthen the WHY by connecting the inconsistency to a concrete business outcome — e.g., does the lack of friendly names cause support tickets, slow down tenant onboarding demos, or block UI features that depend on display names? One sentence tying the pain to platform adoption or a specific user complaint would lift WHY from 1 to 2. Consider also explicitly naming which OSAC services are in scope (all of BMaaS/CaaS/VMaaS/MaaS/Enclave, since metadata is cross-cutting) to satisfy the dimensions checklist.

Critical (0)

None.

Important (1)

  1. Business justification stays at the 'this is inconsistent' level without connecting to business outcomes. The problem statement should name who is affected and what they cannot do today — e.g., 'This blocks UI resource lists from showing user-friendly names, forcing users to memorize DNS-label identifiers' or cite a specific user complaint or support pattern.

Suggestions (3)

  1. Explicitly state which OSAC services are in scope (presumably all, since metadata is cross-cutting) to satisfy the dimensions checklist — a one-line note like 'Applies to all OSAC services (BMaaS, CaaS, VMaaS, MaaS, Enclave)' would suffice.
  2. Consider adding a brief Milestone Scoping section stating whether this targets a specific milestone and whether documentation/UI work is deferred (the Out of Scope section partially covers this but a dedicated section would be clearer).
  3. The Out of Scope item about display behavior ('how clients present display_name vs metadata.name') is a good call-out — consider also noting whether search (not just filter/sort) by display_name is deferred or in scope.

Review cost

Model: claude-opus-4-6
Cost: $0.3270
Tokens: 7 in / 4.8k out
Cache: 244.3k read
Active time: 1m 51s
API calls: 0

Remove E2E/documentation from In Scope (delivery artifacts belong in
design/implementation). Reply round addresses CodeRabbit and mhrivnak
top-level feedback.

Assisted-by: Claude Code <noreply@anthropic.com>
@udis

udis commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@mhrivnak Thanks — we've narrowed the PRD around that feedback:

  1. Primary driver is consistency, not a new identity model: some resources already had title/description/display_name-like fields under inconsistent names; most others had none. This standardizes on shared metadata.display_name / metadata.description and removes the per-resource duplicates.
  2. Identity stays on metadata.name — now explicit in Out of Scope.
  3. Display behavior is deferred to UX/design (no mandated fallback).
  4. User stories were reworded away from identity/audit/labeling language toward natural-language naming and filter/sort by display_name (e.g. dell-xe9680 vs "Dell PowerEdge XE9680").

Happy to adjust further if this still overclaims.

@github-actions

Copy link
Copy Markdown

AI EP Review: EP-141

Score: 9/10 | Verdict: PASS

Criterion Score Notes
WHAT (clear need) 2/2 Clear, specific user-facing capability: standardized display_name and description fields across all OSAC resources, replacing inconsistent per-resource title/description fields. All four OSAC personas (Cloud Provider Admin, Cloud Infrastructure Admin, Tenant Admin, Tenant User) have dedicated headings with concrete 'As a ...' user stories. User stories cover creation, update, clearing, filtering, and sorting. The problem (DNS-label constraints making metadata.name unsuitable as a friendly label)
WHY (justification) 2/2 Concrete justification in the Problem Statement: metadata.name is constrained to DNS-label format (lowercase alphanumeric, hyphens, max 63 chars), some resource types have ad-hoc title/description while most have none, and this inconsistency 'forces repeated per-resource-type discussions and produces an uneven user experience across VMs, virtual networks, public IPs, and other resources.' The causal chain is clear: constraint → inconsistency → poor UX and engineering churn.
User-Facing Focus 1/2 Mostly user-focused but with minor design leakage. The In Scope bullet on reconciliation lists 12 specific resource types by internal name as implementation work items ('removed from all 12 resource types') rather than describing the user outcome. The Dependencies section names 'fulfillment-service proto and server changes' — an internal component and implementation ordering that belongs in the design document. The field names (display_name, description) and resource type names (Project, Network
Right-Sized 2/2 Tightly scoped and coherent. The three capabilities — adding shared metadata fields, consolidating existing inconsistent per-resource fields, and supporting filtering/sorting — are mutually dependent. Standardization without removing old fields leaves inconsistency; filtering without the fields is meaningless; removing old fields without the new ones breaks existing functionality. This is one feature, not a bundle.
Testability 2/2 All requirements are verifiable by using the product. A PM/QA engineer can: create a resource with display_name and description, verify they appear; update and clear them on existing resources; filter and sort resource lists by display_name; verify platform-defined resources (NetworkClass, HostType, catalog items) use the same fields; verify constraints (max 63/256 chars, optional, not unique). The reconciliation of old title/description fields is also testable — verify old fields are gone and n

Verdict: Strong PRD with clear user-facing need, concrete justification, focused scope, and testable requirements; the only weakness is minor design leakage in the reconciliation scope and dependencies section.

Feedback: Reframe the reconciliation In Scope bullet as a user outcome ('All OSAC resources use the same display_name and description fields, replacing the per-resource title/description fields used by some resource types today') instead of listing 12 internal resource types as implementation work items. Move the Dependencies section content (fulfillment-service proto ordering) to the design document — a PRD should state what depends on what from the user's perspective, not which internal components must land first. Consider explicitly listing which services are in scope (even if all of them) and adding a brief note on cross-cutting dimensions like documentation and E2E testing scope.

Critical (0)

None.

Important (2)

  1. In Scope reconciliation bullet reads as implementation scope — 'removed from all 12 resource types that currently have them: Project, Role, IdentityProvider...' is an engineering work list, not a user outcome. Reframe as: 'All OSAC resources use the same display_name and description fields, replacing the inconsistent per-resource title/description fields currently used by some resource types.' The 12-type list belongs in the design document.
  2. Dependencies section names internal components ('fulfillment-service proto and server changes: Must land before UI and E2E test changes'). This implementation ordering belongs in the design document. A PRD dependency section should describe user-visible or cross-team dependencies, not internal component sequencing.

Suggestions (4)

  1. Explicitly state which services are in scope — even 'All services (BMaaS, CaaS, VMaaS, MaaS, Enclave)' — to satisfy the OSAC dimensions checklist.
  2. Add brief notes on cross-cutting dimensions: documentation needs (API reference updates, user guides), E2E testing scope (which flows need coverage), and installation impact (even if just 'no installation changes required').
  3. The [Clarify: R2.Q1, PR review: mhrivnak] annotations are useful provenance but may distract in the published document — consider moving them to footnotes or consolidating in the Provenance section.
  4. No milestone scoping section — state which milestone this targets and whether anything is deferred beyond the Out of Scope items already listed.

Review cost

Model: claude-opus-4-6
Cost: $0.5503
Tokens: 6 in / 4.5k out
Cache: 147.3k read
Active time: 1m 36s
API calls: 0

@mhrivnak mhrivnak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@openshift-ci openshift-ci Bot added the lgtm label Jul 29, 2026
@openshift-ci

openshift-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mhrivnak, udis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit 3849613 into osac-project:main Jul 29, 2026
5 checks passed
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.

5 participants