Conversation
|
This change is part of the following stack: Change managed by git-spice. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #737 +/- ##
=======================================
Coverage 55.66% 55.66%
=======================================
Files 126 126
Lines 13759 13760 +1
=======================================
+ Hits 7659 7660 +1
Misses 6100 6100 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
geoffjay
left a comment
There was a problem hiding this comment.
Review: refactor(common): centralize HealthResponse status strings as pub constants
Stack position: issue-721 is directly on main with no parent PR
dependency, but git-spice reports (needs restack) — the branch is behind
main and must be rebased before the conductor can merge it.
Code quality: approved
The change is correct, focused, and complete within its stated scope.
Constants — STATUS_OK = "ok" and STATUS_DEGRADED = "degraded" are
exact replacements for the literals they supersede. Module-level placement
(rather than associated constants on impl HealthResponse) is a valid and
idiomatic choice; callers import them as agentd_common::types::STATUS_OK.
Three use sites updated — ok(), degraded(), and is_healthy() in
types.rs all use the constants consistently. No fourth call site was missed.
pub visibility — correct per the architect guidance stored in memory: downstream
crates (CLI, integration tests) can now import the constants instead of
hardcoding the string. The change is additive — existing callers comparing
against "ok" in JSON assertions are unaffected and can migrate
opportunistically.
No behaviour change — the string values are identical; existing tests pass
without modification.
Scope — single file, single concern, no unrelated changes.
Observation (no action required)
grep across the codebase shows several downstream sites still using "ok"
and "degraded" literals directly against HealthResponse.status (e.g.,
crates/memory/src/api.rs, crates/monitor/src/api.rs). These can migrate
to STATUS_OK / STATUS_DEGRADED in a follow-up, but are out of scope here
and not broken.
Action required before merge
Rebase onto main:
git-spice branch restack
git-spice branch submitOnce restacked, add merge-queue to trigger the conductor.
refactor(common): centralize HealthResponse status strings as pub constants
refactor(common): centralize HealthResponse status strings as pub constants (closes #721)