HYPERFLEET-1017 - refactor: rename Available to LastKnownReconciled and align example reasons#43
Conversation
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughDocumentation was expanded in CLAUDE.md to describe the TypeSpec provider-alias workflow (the Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request updates validation constraints for status condition arrays across the API schema and model definitions. The Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 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)
Warning Review ran into problems🔥 ProblemsLinked repositories: Couldn't analyze Linked repositories: Couldn't analyze Linked repositories: Couldn't analyze Comment |
be41887 to
4f6584e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@aliases.tsp`:
- Line 1: Replace the internal statuses import to restore public/internal API
separation: in aliases.tsp remove the import of
"./services/statuses-internal.tsp" and import "./services/statuses.tsp" instead
so only the public read-only statuses API is exposed; ensure no other references
in aliases.tsp rely on internal-only endpoints.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e25aa231-7591-4244-95c8-d45cb4aa5b68
📒 Files selected for processing (10)
CHANGELOG.mdCLAUDE.mdaliases.tspmain.tspmodels/clusters/model.tspmodels/nodepools/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yaml
4f6584e to
d734384
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@main.tsp`:
- Line 23: Replace the current non-canonical `@info` decorator usage with the
extractor-friendly canonical object layout so the CI can parse the version;
specifically update the `@info`(...) expression (the `@info` decorator on the file)
to use a plain JS/TS-style object like `@info`({version: "1.0.12", contact: {name:
"HyperFleet Team"}, license: {name: "Apache 2.0", url:
"https://www.apache.org/licenses/LICENSE-2.0"}}) ensuring the version field
remains "1.0.12".
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 052553e5-6d92-4cdf-bd1b-1dfb3f9a57ed
📒 Files selected for processing (11)
.gitignoreCHANGELOG.mdCLAUDE.mdaliases.tspmain.tspmodels/clusters/model.tspmodels/nodepools/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yaml
💤 Files with no reviewable changes (1)
- aliases.tsp
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@CLAUDE.md`:
- Around line 195-198: Update the CLAUDE.md wording to clarify that released
versions must be their own top-level sections (e.g., "## [X.Y.Z] - YYYY-MM-DD")
and must not be nested under the "## [Unreleased]" heading; change the
instruction around line referencing "## [Unreleased]" so it explicitly directs
maintainers to add a new sibling section for each release, list changes under
the appropriate subheadings ("Added", "Changed", "Fixed", "Removed"), and update
comparison links at the bottom to reference the new version tag—use the headings
"## [Unreleased]" and "## [X.Y.Z] - YYYY-MM-DD" as the identifiers to locate and
rewrite the confusing sentence.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e8e6dbd1-5995-4609-8f00-93698d2e1c80
📒 Files selected for processing (1)
CLAUDE.md
b4cba3b to
78646c0
Compare
|
I think minItems might be related to HYPERFLEET-853. We should keep minItems to 2 as we are in the transition period of Ready to Reconciled. Bumping to 3 tightens the schema contract — clients generated from the updated spec will reject responses with only 2 conditions, which can happen if the server hasn't fully rolled out the third condition yet This could be removed when we remove Ready completely (creating the JIRA ticket right now, will update this comment with the same). Ref: #34 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@CLAUDE.md`:
- Line 38: The CLAUDE.md guidance is out of sync with our artifact policy:
update the paragraph about aliases.tsp to state that aliases.tsp is a generated
symlink created/updated by build-schema.sh (and should not be tracked as a
source file), remove the sentence that says it must be tracked and always point
to aliases-core.tsp, and instead instruct authors to add aliases.tsp to
.gitignore (or confirm it's listed) so build artifacts aren't committed; also
add a brief note in build-schema.sh comments that the script manages
aliases.tsp/aliases-core.tsp symlink.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 29d9f5b3-a9e4-49d8-8767-f016b2287b29
📒 Files selected for processing (9)
CHANGELOG.mdCLAUDE.mdmain.tspmodels/clusters/model.tspmodels/nodepools/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yaml
|
@tirthct Good call — you're right. Reverted |
0bdfec1 to
2ce58a6
Compare
…E.md - Restore aliases.tsp symlink in git pointing to aliases-core.tsp by default - Document aliases.tsp convention in CLAUDE.md - Add changelog update instructions to CLAUDE.md
2ce58a6 to
cd11b21
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
main.tsp (1)
23-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCI version extraction is failing on the
@infometadata path.Validation is currently failing to extract the version from
main.tsp, which blocks merge/release automation. Please verify the workflow extractor pattern against this exact@infoline and align one side (workflow regex or parse target format) so extraction is deterministic.Run this read-only verification to pinpoint the mismatch:
#!/bin/bash set -euo pipefail echo "=== main.tsp version line context ===" nl -ba main.tsp | sed -n '20,26p' echo echo "=== grep-based version extraction commands found in CI workflows ===" if [ -d .github/workflows ]; then rg -n --iglob '*.yml' --iglob '*.yaml' 'grep -oP|version' .github/workflows -C2 else echo "No .github/workflows directory found." fi echo echo "=== Reproduce likely extractor variants against main.tsp ===" echo "- Double-quote lookbehind pattern:" grep -oP '(?<=version: ")[^"]+' main.tsp || true echo "- Single-quote lookbehind pattern:" grep -oP "(?<=version: ')[^']+" main.tsp || true🤖 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 `@main.tsp` at line 23, The CI extractor is not matching the `@info` line in main.tsp (which contains `@info`(#{ version: "1.0.12", contact: #{ name: "HyperFleet Team" } ... })) because the workflow's grep/regex expects a different spacing/format around `version:`; update the CI workflow regex to handle optional whitespace and the surrounding #{ } and quotes (for example use a lookbehind like (?<=version:\s*")[^"]+ or a variant supporting single quotes), or alternatively change the `@info` metadata to put version on its own predictable line (e.g., version: "1.0.12") so the existing extractor matches; reference `@info` and the literal `version: "1.0.12"` when making the change.
🤖 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.
Duplicate comments:
In `@main.tsp`:
- Line 23: The CI extractor is not matching the `@info` line in main.tsp (which
contains `@info`(#{ version: "1.0.12", contact: #{ name: "HyperFleet Team" } ...
})) because the workflow's grep/regex expects a different spacing/format around
`version:`; update the CI workflow regex to handle optional whitespace and the
surrounding #{ } and quotes (for example use a lookbehind like
(?<=version:\s*")[^"]+ or a variant supporting single quotes), or alternatively
change the `@info` metadata to put version on its own predictable line (e.g.,
version: "1.0.12") so the existing extractor matches; reference `@info` and the
literal `version: "1.0.12"` when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c008322a-03e7-4028-8fcd-2fbd8215b2b3
📒 Files selected for processing (7)
CHANGELOG.mdCLAUDE.mdmain.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yaml
|
Closing — the rename changes (Available → LastKnownReconciled) are already in main. The generated openapi.yaml will be copied to hyperfleet-api in a separate PR that also adapts the Go code to the new TypeSpec types. |
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)
schemas/core/openapi.yaml (1)
1-1:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRegenerate and commit schema artifacts to fix CI blocker
The PR currently fails validation because
schemas/is out of sync with TypeSpec sources. Please run and commit the generated output before merge:./build-schema.sh core --swagger ./build-schema.sh gcp --swagger git diff -- schemas/As per coding guidelines, "Do not modify generated files in
schemas/ortsp-output/directly" and "run./build-schema.sh gcpand./build-schema.sh core(and their--swaggervariants if applicable), then verify outputs exist inschemas/*/openapi.yaml."🤖 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 `@schemas/core/openapi.yaml` at line 1, The failure is caused by generated schema artifacts being out of sync; run the generation scripts and commit their outputs: execute "./build-schema.sh core --swagger" and "./build-schema.sh gcp --swagger", verify the generated openapi.yaml files appear under schemas (and ensure tsp-output/ if applicable), do not edit generated files by hand, then add and commit the updated generated files and confirm "git diff -- schemas/" is clean before merging.
🤖 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 `@schemas/core/openapi.yaml`:
- Line 1: The failure is caused by generated schema artifacts being out of sync;
run the generation scripts and commit their outputs: execute "./build-schema.sh
core --swagger" and "./build-schema.sh gcp --swagger", verify the generated
openapi.yaml files appear under schemas (and ensure tsp-output/ if applicable),
do not edit generated files by hand, then add and commit the updated generated
files and confirm "git diff -- schemas/" is clean before merging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2b78aa9d-41eb-4b86-bf54-2746578cdebe
📒 Files selected for processing (2)
models/common/model.tspschemas/core/openapi.yaml
553eeb6 to
cf019d7
Compare
cf019d7 to
b77a76e
Compare
|
Consumed by: openshift-hyperfleet/hyperfleet-api#127 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
main.tsp (1)
23-23:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCI blocker: adjust
@infolayout so version extraction succeedsLine 23 is still tied to a failing validation step (
Failed to extract version from main.tsp). Please switch to the extractor-friendly canonical layout before merge.Proposed fix
-@info(#{ version: "1.0.12", contact: #{ name: "HyperFleet Team" }, license: #{ name: "Apache 2.0" ,url: "https://www.apache.org/licenses/LICENSE-2.0"} }) +@info(#{ + version: "1.0.12", + contact: #{ name: "HyperFleet Team" }, + license: #{ name: "Apache 2.0", url: "https://www.apache.org/licenses/LICENSE-2.0" } +})🤖 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 `@main.tsp` at line 23, The `@info` block must be rewritten into the extractor-friendly canonical layout so the version parser can find a top-level version field; update the `@info`(...) invocation (symbol: `@info`) to expose version as a plain top-level key with the exact string "1.0.12" (symbol: version) and remove or flatten nested maps for contact and license (symbols: contact, license) so the extractor can read version reliably.
🤖 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 `@models/common/model.tsp`:
- Around line 247-251: The new reason strings (ExampleLastKnownReconciledReason,
ExampleReadyReason, ExampleReconciledReason) introduce breaking values; update
the model to be backward-compatible by preserving the previous reason constants
or providing canonical-to-legacy mapping—e.g., add legacy aliases for
"AllAdaptersLastKnownReconciled" and any other prior values (keep
ExampleLastKnownReconciledMessage and ExampleReadyMessage as-is), or implement a
mapping function in the model that returns both new and legacy reason strings so
consumers like hyperfleet-sentinel continue to work during rollout; reference
the constants ExampleLastKnownReconciledReason, ExampleReconciledReason, and
ExampleReadyReason to add the legacy names or mapping.
---
Duplicate comments:
In `@main.tsp`:
- Line 23: The `@info` block must be rewritten into the extractor-friendly
canonical layout so the version parser can find a top-level version field;
update the `@info`(...) invocation (symbol: `@info`) to expose version as a plain
top-level key with the exact string "1.0.12" (symbol: version) and remove or
flatten nested maps for contact and license (symbols: contact, license) so the
extractor can read version reliably.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ed371805-ab89-4381-b74f-34d23f58de72
📒 Files selected for processing (6)
main.tspmodels/common/model.tspschemas/core/openapi.yamlschemas/core/swagger.yamlschemas/gcp/openapi.yamlschemas/gcp/swagger.yaml
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ma-hill 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 |
98cfd7e
into
openshift-hyperfleet:main
Summary
hyperfleet-apiExampleLastKnownReconciledReason→AllAdaptersReconciledExampleReadyReason/ExampleReconciledReason→ReconciledAllaliases.tspsymlink convention and document it in CLAUDE.mdContext
The
LastKnownReconciledcondition type and enum value were already added tomain. This PR aligns the example payload reason/message values with the actual output fromcomputeLastKnownReconciled()andcomputeReconciled()in the API code.Test plan
./build-schema.sh core— TypeSpec compiles successfullyopenapi.yamlhas correct reason values (AllAdaptersReconciled,ReconciledAll)