HYPERFLEET-1232 - docs: move detailed sentinel docs from architecture repo#222
Conversation
📝 WalkthroughWalkthroughAdds a Decision Engine reference covering CEL evaluation, status semantics, adapter update contracts, Sentinel components, publishing behavior, and test scenarios. Updates operator and configuration documentation to use Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Risk Score: 2 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 636 lines (>500) | +2 |
| Sensitive paths | none | +0 |
Computed by hyperfleet-risk-scorer
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/sentinel-operator-guide.md (1)
682-699: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFinish the config-path rename in the troubleshooting table.
The 401 Unauthorized row still refers to
hyperfleet_api, so this section now mixes the old flat schema with the new nestedclients.hyperfleet_api.*schema. Update that row too to keep the operator guide consistent.🤖 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 `@docs/sentinel-operator-guide.md` around lines 682 - 699, The troubleshooting table still uses the old flat `hyperfleet_api` config path in the 401 Unauthorized row, so update that entry to the new nested `clients.hyperfleet_api.*` schema. Make the change in the operator guide’s troubleshooting section and keep the wording aligned with the rest of the config-path renames in that table.
🤖 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 `@docs/decision-engine.md`:
- Around line 346-349: The cross-reference in the Message Data Composition
section points to an outdated config.md anchor, so update the link target to the
current generated heading for the Message Data section. Locate the markdown text
in the decision-engine documentation near the “Message Data Composition (CEL
Expressions)” block and change the reference from the old anchor to the current
`#message-data-cel-expressions` anchor so the link resolves correctly.
- Around line 502-518: The note in the Test 7 example should be tightened to
reflect the actual default-config behavior: when the Reconciled condition is
missing, `has_ref_time` in the decision logic prevents `timestamp(ref_time)`
from evaluating a zero-value condition, so `not_reconciled_and_debounced` is not
the reason this publishes. Update the explanation around
`condition("Reconciled")`, `has_ref_time`, `not_reconciled_and_debounced`, and
`is_new_resource` to state that the decision is PUBLISH solely because
`is_new_resource` is true.
- Around line 481-498: Update the Test 6 note in decision-engine.md to reflect
that `message_decision` evaluates `generation_mismatch` directly via
`resource.generation > condition("Reconciled").observed_generation`, not only
through `not_reconciled_and_debounced`. Adjust the explanation around the
`Decision: PUBLISH` case to mention the `generation_mismatch` path and keep the
rest of the test data intact so the description matches the actual default rule
behavior.
---
Outside diff comments:
In `@docs/sentinel-operator-guide.md`:
- Around line 682-699: The troubleshooting table still uses the old flat
`hyperfleet_api` config path in the 401 Unauthorized row, so update that entry
to the new nested `clients.hyperfleet_api.*` schema. Make the change in the
operator guide’s troubleshooting section and keep the wording aligned with the
rest of the config-path renames in that table.
🪄 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: CHILL
Plan: Enterprise
Run ID: 498839a2-4416-4c7c-9079-b1549ff35542
📒 Files selected for processing (3)
docs/config.mddocs/decision-engine.mddocs/sentinel-operator-guide.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
c44c863 to
6c38af3
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 `@docs/decision-engine.md`:
- Around line 267-280: The broker config source description is incorrect and
points operators to the wrong input path. Update the references around the
LoadBrokerConfig flow to state that broker configuration is loaded from
broker.yaml or the BROKER_CONFIG_FILE environment variable, not from Sentinel
YAML or a ConfigMap, and keep the wording aligned with the hyperfleet-broker
library contract. Also revise any related mentions in the same section so
BuildLabelSelector/Load(configPath) documentation clearly separates Sentinel
config from broker config.
🪄 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: CHILL
Plan: Enterprise
Run ID: 81c7e7db-7f76-49c5-a799-3d11496b3e1b
📒 Files selected for processing (3)
docs/config.mddocs/decision-engine.mddocs/sentinel-operator-guide.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
✅ Files skipped from review due to trivial changes (1)
- docs/config.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/sentinel-operator-guide.md
6c38af3 to
42d027d
Compare
There was a problem hiding this comment.
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 `@docs/decision-engine.md`:
- Around line 181-184: The generation-flow description is inconsistent with the
documented default decision logic in message_decision, which already checks
generation_mismatch. Update the wording in the “How Generation Changes Flow
Through the System” section to say the Sentinel relies on the default rules’
generation_mismatch evaluation (along with Reconciled) rather than implying no
separate generation check exists. Keep the explanation aligned with the result
expression and reference message_decision and generation_mismatch when revising
the text.
- Around line 403-446: The skip examples in the decision-engine docs are
incomplete because they omit the observed_generation check, which affects
whether the default decision stays false. Update the Test 1 and Test 3 scenarios
in the decision logic section to use condition("Reconciled").observed_generation
equal to resource.generation, and keep the existing refs to ref_time,
is_reconciled, is_new_resource, reconciled_and_stale, and
not_reconciled_and_debounced so the outcomes remain SKIP as described.
🪄 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: CHILL
Plan: Enterprise
Run ID: aeddaf1e-5211-43b4-8614-d834ab050e74
📒 Files selected for processing (3)
docs/config.mddocs/decision-engine.mddocs/sentinel-operator-guide.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
✅ Files skipped from review due to trivial changes (2)
- docs/config.md
- docs/sentinel-operator-guide.md
42d027d to
749ddbd
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 `@docs/decision-engine.md`:
- Around line 419-432: Complete the decision breakdowns in the decision-engine
examples by adding the missing generation_mismatch term to the evaluated params
and result expression in the test cases. In the Test 2 and Test 5 sections of
decision-engine.md, update the breakdowns around the message decision logic so
they explicitly show generation_mismatch alongside ref_time, is_reconciled,
is_new_resource, reconciled_and_stale, and not_reconciled_and_debounced. Also
correct Test 5’s value so it reflects that when Reconciled is absent,
observed_generation defaults to 0 and generation_mismatch evaluates to true.
🪄 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: CHILL
Plan: Enterprise
Run ID: 30a36c1d-5901-4350-bf32-fef49ee9d818
📒 Files selected for processing (3)
docs/config.mddocs/decision-engine.mddocs/sentinel-operator-guide.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
✅ Files skipped from review due to trivial changes (1)
- docs/config.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/sentinel-operator-guide.md
749ddbd to
eaa6b02
Compare
99adcf9 to
daec9d5
Compare
Move detailed operational/reference content to the sentinel repo (openshift-hyperfleet/hyperfleet-sentinel#222) and keep only the high-level design document per the component doc standard. Removed sections (now in sentinel repo docs/decision-engine.md): - Message Decision details, CEL Function Reference - Adapter Status Update Contract, Status Tracking - Service Components, Decision Engine Test Scenarios - Configuration YAML examples, Resource Filtering examples Retained sections: What & Why, Architecture (diagrams), Decision Logic (conceptual), Trade-offs, Alternatives Considered, Post-MVP. Also updates cross-references in condition-mapping-design.md, glossary.md, and v0.2.0-to-v1.0.0-upgrade-guide.md to point to the sentinel repo for detailed content.
There was a problem hiding this comment.
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 `@docs/decision-engine.md`:
- Around line 121-126: Clarify the Test 12 missing-condition note: explain that
when the Applied condition is absent, is_applied short-circuits before
timestamp(ref_time) is evaluated, so this example skips publishing rather than
converting a missing timestamp.
- Around line 350-353: Update the “Implementation Requirements” list in
decision-engine.md to describe GCP Pub/Sub support through the hyperfleet-broker
abstraction rather than directly using cloud.google.com/go/pubsub, matching the
existing RabbitMQ requirement’s framing.
🪄 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: CHILL
Plan: Enterprise
Run ID: 47531230-4fc6-41a4-ba5f-8e9ecf170dc1
📒 Files selected for processing (3)
docs/config.mddocs/decision-engine.mddocs/sentinel-operator-guide.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
✅ Files skipped from review due to trivial changes (2)
- docs/config.md
- docs/sentinel-operator-guide.md
daec9d5 to
d85bcfb
Compare
… repo Move operational/reference content from the architecture repo's sentinel.md to the sentinel repo's docs/ directory. The architecture repo now retains only the high-level design document. New file: - docs/decision-engine.md: CEL function reference, status tracking, adapter status contract, service components, test scenarios (1-12) Updated files: - docs/sentinel-operator-guide.md: fix outdated property name hyperfleet_api.endpoint -> clients.hyperfleet_api.base_url - docs/config.md: add cross-reference to decision-engine.md
d85bcfb to
f16a757
Compare
Move detailed operational/reference content to the sentinel repo (openshift-hyperfleet/hyperfleet-sentinel#222) and keep only the high-level design document per the component doc standard. Removed sections (now in sentinel repo docs/decision-engine.md): - Message Decision details, CEL Function Reference - Adapter Status Update Contract, Status Tracking - Service Components, Decision Engine Test Scenarios - Configuration YAML examples, Resource Filtering examples Retained sections: What & Why, Architecture (diagrams), Decision Logic (conceptual), Trade-offs, Alternatives Considered, Post-MVP. Also updates cross-references in condition-mapping-design.md, glossary.md, and v0.2.0-to-v1.0.0-upgrade-guide.md to point to the sentinel repo for detailed content.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tirthct 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 |
a7c4cae
into
openshift-hyperfleet:main
Summary
docs/decision-engine.mdwith all detailed content moved from the architecture repo'ssentinel.md: CEL function reference, status tracking semantics, adapter status update contract, service components, and decision engine test scenarios (Tests 1-12)hyperfleet_api.endpoint→clients.hyperfleet_api.base_urlindocs/sentinel-operator-guide.md(4 locations: YAML config block, Required Fields table, Deployment Checklist, Troubleshooting table) — reported by Martin Liptak during Ignition Daydecision-engine.mdCompanion PR for the architecture repo will trim
sentinel.mdto keep only the high-level design document (What/Why, How, Trade-offs, Alternatives Considered).Test plan
decision-engine.mdresolve correctlydocs/config.mdcross-reference todecision-engine.mdworksdocs/sentinel-operator-guide.mdcross-reference in Additional Resources worksclients.hyperfleet_api.base_urlmatches the actual config struct