Skip to content

fix(BA-5967): make ResourceSlot.to_humanized() resilient to unknown slots#11505

Closed
fregataa wants to merge 2 commits into
mainfrom
fix/BA-5967-to-humanized-graceful-fallback
Closed

fix(BA-5967): make ResourceSlot.to_humanized() resilient to unknown slots#11505
fregataa wants to merge 2 commits into
mainfrom
fix/BA-5967-to-humanized-graceful-fallback

Conversation

@fregataa
Copy link
Copy Markdown
Member

@fregataa fregataa commented May 7, 2026

Summary

  • Resolve each slot independently in ResourceSlot.to_humanized() via per-slot try/except KeyError, falling back to _guess_slot_type() for keys missing from the supplied slot_types map.
  • Eliminates a class of 500 Internal Server Errors triggered when error-message humanizers (e.g. ResourceLimitRule._humanize_slots) ran against partially-populated slot type metadata — a missing display-metadata entry no longer clobbers the underlying user-facing 4xx.

Test plan

  • New unit tests in tests/unit/common/test_types.py cover the per-slot fallback and the empty-slot_types case.
  • pants fmt, pants fix, pants lint --changed-since=origin/main pass locally.
  • Existing callers (ResourceLimitRule._humanize_slots, scheduler predicates) continue to work — verified tests/unit/common/test_types.py and tests/unit/manager/sokovan/scheduling_controller/validators/test_session_spec_rules.py.

Resolves BA-5967

🤖 Generated with Claude Code

…lots

Resolve each slot independently via per-slot try/except so a missing
slot_types entry falls back to _guess_slot_type instead of aborting the
whole formatter. Eliminates 500 Internal Server Errors when error
messages are humanized against partially-populated slot type metadata.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 7, 2026 09:09
@github-actions github-actions Bot added size:M 30~100 LoC comp:common Related to Common component labels May 7, 2026
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Makes ResourceSlot.to_humanized() tolerant of missing slot_types entries by resolving units per-slot and falling back to _guess_slot_type(), preventing formatting-time 500s from masking user-facing 4xx errors.

Changes:

  • Update ResourceSlot.to_humanized() to fall back per key when slot_types lacks an entry
  • Add unit tests covering unknown-slot fallback and empty slot_types
  • Add a changelog entry documenting the behavior change

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/ai/backend/common/types.py Implements per-slot unit resolution with fallback guessing instead of raising on missing keys
tests/unit/common/test_types.py Adds regression tests validating fallback behavior and empty slot_types handling
changes/11505.fix.md Documents the bugfix and its impact on avoiding 500s during error formatting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1225 to +1236
def to_humanized(self, slot_types: Mapping[str, Any]) -> Mapping[str, str]:
try:
return {
k: type(self)._humanize_value(Decimal(v), slot_types[k])
for k, v in self.data.items()
if v is not None
}
except KeyError as e:
raise ValueError(f"Unknown slot type: {e.args[0]!r}") from e
def _resolve_unit(key: str) -> Any:
try:
return slot_types[key]
except KeyError:
return type(self)._guess_slot_type(key)

return {
k: type(self)._humanize_value(Decimal(v), _resolve_unit(k))
for k, v in self.data.items()
if v is not None
}
Comment thread src/ai/backend/common/types.py
Comment thread tests/unit/common/test_types.py
@fregataa fregataa requested a review from a team May 7, 2026 11:59
@@ -1223,14 +1223,17 @@ def from_user_input(
return cls(data)

def to_humanized(self, slot_types: Mapping[str, Any]) -> Mapping[str, str]:
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.

Could we change the parameter type to Mapping[SlotName, SlotTypes]?

@fregataa fregataa marked this pull request as draft May 8, 2026 03:16
@fregataa fregataa closed this May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:common Related to Common component size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants