From a0b3380654e6a0b0f4fd473f41c593c5db51b29f Mon Sep 17 00:00:00 2001 From: hanghang zhang Date: Fri, 22 May 2026 20:18:21 +0100 Subject: [PATCH] feat(schedule_type): minute-precision windows + variable maintenance length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lifts the two hard restrictions in PR #18: * window bounds were `int hour` (0-23) → now `int minutes-since-UTC-midnight` (0-1439) * maintenance window was exactly 1 hour → now any duration in [1, 180] minutes ((maint_to - maint_from) mod 1440) ## Schema migration (additive) `_migrate_schema()` detects legacy "hours" rows (any row where MAX of the 6 window columns is ≤ 23) and multiplies each column by 60 to convert to minutes. Idempotent — post-conversion values are well above 23 so the guard never fires twice. ## Touched surfaces - `models/schedule_type.py` — column comments updated; new `compute_maintenance_duration()` helper (returns 1-1440 min, treats from==to as 1440 which is then rejected by validator) - `schemas/schedule_type.py` — `*_from`/`*_to` upper bound 23 → 1440; `_validate_maintenance_window` accepts 1-180min duration; response includes derived `maintenance_duration_minutes` - `schemas/schedule_type_special_slot.py` — `minute_in_window` max 59→179; `estimated_duration` max 60→180 - `routers/schedule_type.py` — PATCH re-validates merged maintenance pair (partial updates can put the row into an invalid combo the pydantic single-field validator can't catch); `_attach_derived` populates the new response field - `routers/schedule_type_special_slot.py` — `_validate_fits_window` now takes the parent's maintenance duration instead of hard-coded 60 - `routers/calendar.py` — `_scheduled_inside_window` arg renamed hour→min; the maintenance-window guard error message formats HH:MM not HH:00 - `services/special_slot_materialiser.py` — materialised `scheduled_at` derived from `(maint_from_min + tpl.minute_in_window)` with hour/minute split 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) --- app/api/routers/calendar.py | 23 +++--- app/api/routers/schedule_type.py | 36 +++++++-- app/api/routers/schedule_type_special_slot.py | 15 ++-- app/main.py | 29 ++++++- app/models/schedule_type.py | 78 +++++++++---------- app/schemas/schedule_type.py | 56 ++++++++----- app/schemas/schedule_type_special_slot.py | 8 +- app/services/special_slot_materialiser.py | 10 +-- 8 files changed, 162 insertions(+), 93 deletions(-) diff --git a/app/api/routers/calendar.py b/app/api/routers/calendar.py index 2f2edcb..1d00c98 100644 --- a/app/api/routers/calendar.py +++ b/app/api/routers/calendar.py @@ -205,12 +205,14 @@ def create_slot( st.maintenance_from, st.maintenance_to, ): + mf_h, mf_m = divmod(st.maintenance_from, 60) + mt_h, mt_m = divmod(st.maintenance_to, 60) raise HTTPException( status_code=422, detail=( f"slot at {payload.scheduled_at} duration {payload.estimated_duration}min " f"intersects the maintenance window " - f"{st.maintenance_from:02d}:00-{st.maintenance_to:02d}:00 UTC of " + f"{mf_h:02d}:{mf_m:02d}-{mt_h:02d}:{mt_m:02d} UTC of " f"schedule_type '{st.name}' — that window is admin-reserved" ), ) @@ -341,22 +343,21 @@ def _require_agent(db: Session, agent_id: str, claw_identifier: str) -> Agent: def _scheduled_inside_window( scheduled_at, estimated_duration_minutes: int, - window_from_hour: int, - window_to_hour: int, + window_from_min: int, + window_to_min: int, ) -> bool: - """True if [scheduled_at, scheduled_at+duration] intersects [from:00, to:00]. + """True if [scheduled_at, scheduled_at+duration] intersects [from, to). - Handles the 23→0 wrap case (window straddles UTC midnight). + Window bounds are minutes-since-UTC-midnight (0-1439). Handles the + case where the window crosses UTC midnight (e.g. 23:30→01:00). """ start_min = scheduled_at.hour * 60 + scheduled_at.minute end_min = start_min + max(estimated_duration_minutes, 1) - win_start_min = window_from_hour * 60 - win_end_min = window_to_hour * 60 - if win_end_min > win_start_min: + if window_to_min > window_from_min: # normal same-day window - return start_min < win_end_min and end_min > win_start_min - # wrap-around: window = [from..24:00) ∪ [00:00..to) - return (start_min < 24 * 60 and end_min > win_start_min) or end_min > win_end_min + return start_min < window_to_min and end_min > window_from_min + # wrap-around: window = [from..1440) ∪ [0..to) + return (start_min < 1440 and end_min > window_from_min) or end_min > window_to_min # Admin-locked special slots accept only these agent-driven status diff --git a/app/api/routers/schedule_type.py b/app/api/routers/schedule_type.py index 09d4cf3..fd1ab52 100644 --- a/app/api/routers/schedule_type.py +++ b/app/api/routers/schedule_type.py @@ -57,6 +57,18 @@ def _require_schedule_manage(db: Session, user: User) -> User: return user +def _attach_derived(st: ScheduleType) -> ScheduleType: + """Attach derived fields (maintenance_duration_minutes) so the + pydantic ScheduleTypeResponse picks them up via from_attributes. + + Pydantic with from_attributes reads attributes off the ORM object; + setting a transient attr here avoids having to convert through dict. + """ + if st is not None: + st.maintenance_duration_minutes = st.compute_maintenance_duration() + return st + + # --------------------------------------------------------------------------- # Schedule Type CRUD # --------------------------------------------------------------------------- @@ -71,7 +83,7 @@ def list_schedule_types( current_user: User = Depends(get_current_user_or_apikey), ): _require_schedule_read(db, current_user) - return db.query(ScheduleType).all() + return [_attach_derived(st) for st in db.query(ScheduleType).all()] @router.post( @@ -96,11 +108,13 @@ def create_schedule_type( work_to=payload.work_to, entertainment_from=payload.entertainment_from, entertainment_to=payload.entertainment_to, + maintenance_from=payload.maintenance_from, + maintenance_to=payload.maintenance_to, ) db.add(st) db.commit() db.refresh(st) - return st + return _attach_derived(st) @router.patch( @@ -120,12 +134,23 @@ def update_schedule_type( if not st: raise HTTPException(404, "Schedule type not found") - for field, value in payload.model_dump(exclude_unset=True).items(): + update_fields = payload.model_dump(exclude_unset=True) + for field, value in update_fields.items(): setattr(st, field, value) + # Re-validate maintenance after merge (partial updates can put the row + # into an invalid window combo that the pydantic schema couldn't catch + # because it only saw one field). + from app.schemas.schedule_type import _validate_maintenance_window + try: + _validate_maintenance_window(st.maintenance_from, st.maintenance_to) + except ValueError as e: + db.rollback() + raise HTTPException(422, str(e)) + db.commit() db.refresh(st) - return st + return _attach_derived(st) @router.delete( @@ -181,7 +206,8 @@ def get_my_schedule_type( if not agent.schedule_type_id: return None - return db.query(ScheduleType).filter(ScheduleType.id == agent.schedule_type_id).first() + st = db.query(ScheduleType).filter(ScheduleType.id == agent.schedule_type_id).first() + return _attach_derived(st) if st else None @router.put( diff --git a/app/api/routers/schedule_type_special_slot.py b/app/api/routers/schedule_type_special_slot.py index 9e02328..47ef355 100644 --- a/app/api/routers/schedule_type_special_slot.py +++ b/app/api/routers/schedule_type_special_slot.py @@ -75,15 +75,17 @@ def _fetch_schedule_type(db: Session, schedule_type_id: int) -> ScheduleType: def _validate_fits_window( minute_in_window: int, estimated_duration: int, + maintenance_duration_minutes: int, ) -> None: - """Reject special slots that wouldn't fit inside the 1-hour window.""" - if minute_in_window + estimated_duration > 60: + """Reject special slots that wouldn't fit inside the parent's maintenance window.""" + if minute_in_window + estimated_duration > maintenance_duration_minutes: raise HTTPException( 422, ( f"special slot does not fit in maintenance window: " f"minute_in_window={minute_in_window} + " - f"estimated_duration={estimated_duration} > 60" + f"estimated_duration={estimated_duration} > " + f"maintenance window {maintenance_duration_minutes}min" ), ) @@ -127,8 +129,8 @@ def create_special_slot( current_user: User = Depends(get_current_user_or_apikey), ): _require_schedule_manage(db, current_user) - _fetch_schedule_type(db, schedule_type_id) - _validate_fits_window(payload.minute_in_window, payload.estimated_duration) + st = _fetch_schedule_type(db, schedule_type_id) + _validate_fits_window(payload.minute_in_window, payload.estimated_duration, st.compute_maintenance_duration()) dup = ( db.query(ScheduleTypeSpecialSlot) @@ -188,7 +190,8 @@ def update_special_slot( update_fields = payload.model_dump(exclude_unset=True) next_min = update_fields.get("minute_in_window", slot.minute_in_window) next_dur = update_fields.get("estimated_duration", slot.estimated_duration) - _validate_fits_window(next_min, next_dur) + parent = _fetch_schedule_type(db, schedule_type_id) + _validate_fits_window(next_min, next_dur, parent.compute_maintenance_duration()) for field, value in update_fields.items(): setattr(slot, field, value) diff --git a/app/main.py b/app/main.py index d668211..d85a282 100644 --- a/app/main.py +++ b/app/main.py @@ -400,9 +400,9 @@ def _migrate_schema(): db.execute(text("ALTER TABLE agents ADD COLUMN schedule_type_id INTEGER NULL")) # --- schedule_types: add maintenance_from / maintenance_to --- - # Default 8:00–9:00 UTC for existing rows; the 1-hour-window - # invariant is enforced at the schema level for any NEW rows by - # the pydantic ScheduleTypeCreate validator. + # Default 8:00–9:00 UTC for existing rows; the maintenance + # duration invariant (1-180min) is enforced at the schema + # level for any NEW rows by ScheduleTypeCreate validator. if _has_table(db, "schedule_types"): if not _has_column(db, "schedule_types", "maintenance_from"): db.execute(text( @@ -413,6 +413,29 @@ def _migrate_schema(): "ALTER TABLE schedule_types ADD COLUMN maintenance_to INT NOT NULL DEFAULT 9" )) + # --- minutes-since-midnight migration (PR #21+) --- + # The 6 schedule_type window columns used to hold *hours* + # (0-23). PR #21 changed semantics to *minutes since UTC + # midnight* (0-1439). Detect the legacy regime by checking + # if ANY row has all 6 values ≤ 23 — if so, multiply each + # by 60 to convert. Idempotent: post-conversion values are + # all ≥ 0 and usually well above 23, so guard never fires + # twice. + row = db.execute(text( + "SELECT MAX(GREATEST(work_from, work_to, entertainment_from, entertainment_to, maintenance_from, maintenance_to)) AS m " + "FROM schedule_types" + )).fetchone() + if row is not None and row.m is not None and row.m <= 23: + db.execute(text( + "UPDATE schedule_types SET " + " work_from = work_from * 60, " + " work_to = work_to * 60, " + " entertainment_from = entertainment_from * 60, " + " entertainment_to = entertainment_to * 60, " + " maintenance_from = maintenance_from * 60, " + " maintenance_to = maintenance_to * 60" + )) + # --- time_slots: admin-locked + special_slot pointer --- if _has_table(db, "time_slots"): if not _has_column(db, "time_slots", "is_admin_locked"): diff --git a/app/models/schedule_type.py b/app/models/schedule_type.py index 5691676..4498396 100644 --- a/app/models/schedule_type.py +++ b/app/models/schedule_type.py @@ -1,9 +1,17 @@ """ScheduleType model — defines work/entertainment/maintenance time periods. Each ScheduleType defines the daily work, entertainment, and maintenance -windows. Agents reference a schedule_type to know when they should be -working, when they can engage in entertainment, and when the system -requires them to surrender control for admin-scheduled special slots. +windows for agents who reference this type. All bounds are stored as +**minutes-since-UTC-midnight** (0-1439 inclusive) so half-hour and other +sub-hour boundaries are exact. + +Maintenance window length is variable (1-180 minutes) and admin-owned; +agent slots cannot intersect it (see `app/api/routers/calendar.py`). + +Historical note: pre-PR #21 the columns held *hours* (0-23) and the +maintenance window was hard-fixed at exactly 1 hour. The additive +migration in `_migrate_schema()` multiplies legacy values by 60 so +existing rows convert transparently. """ from sqlalchemy import Column, Integer, String, DateTime @@ -26,52 +34,26 @@ class ScheduleType(Base): comment="Human-readable schedule type name (e.g., 'standard', 'night-shift')", ) - work_from = Column( - Integer, - nullable=False, - comment="Work period start hour (0-23, UTC)", - ) + # Minutes since UTC midnight, 0-1439 inclusive. + work_from = Column(Integer, nullable=False, comment="Work period start (minutes since UTC midnight)") + work_to = Column(Integer, nullable=False, comment="Work period end (minutes since UTC midnight)") - work_to = Column( - Integer, - nullable=False, - comment="Work period end hour (0-23, UTC)", - ) + entertainment_from = Column(Integer, nullable=False, comment="Entertainment start (minutes since UTC midnight)") + entertainment_to = Column(Integer, nullable=False, comment="Entertainment end (minutes since UTC midnight)") - entertainment_from = Column( - Integer, - nullable=False, - comment="Entertainment period start hour (0-23, UTC)", - ) - - entertainment_to = Column( - Integer, - nullable=False, - comment="Entertainment period end hour (0-23, UTC)", - ) - - # ----------------------------------------------------------------- - # Maintenance window — every agent on this schedule_type must - # surrender work/entertainment slots during this hour. Admin-created - # special slots tied to this schedule_type can only be scheduled - # inside this window. The window is always exactly 1 hour. - # - # Default (when columns are added via additive migration to existing - # rows) is 8:00–9:00 UTC so deployments stay sane until an operator - # picks proper hours per schedule_type. - # ----------------------------------------------------------------- + # Maintenance window — admin-owned, variable length (1-180 min). + # Default 8:00–9:00 UTC = 480–540 minutes for existing rows. maintenance_from = Column( Integer, nullable=False, - server_default="8", - comment="Maintenance window start hour (0-23, UTC). Window is exactly 1h.", + server_default="480", + comment="Maintenance start (minutes since UTC midnight, default 480 = 8:00 UTC).", ) - maintenance_to = Column( Integer, nullable=False, - server_default="9", - comment="Maintenance window end hour (0-23, UTC). Must equal (maintenance_from + 1) % 24.", + server_default="540", + comment="Maintenance end (minutes since UTC midnight, default 540 = 9:00 UTC). Duration ((to-from) mod 1440) must be in [1, 180].", ) created_at = Column(DateTime(timezone=True), server_default=func.now()) @@ -83,3 +65,19 @@ class ScheduleType(Base): back_populates="schedule_type", cascade="all, delete-orphan", ) + + # --------------------------------------------------------------- + # Convenience methods used by the API layer + materialiser. + # --------------------------------------------------------------- + + def compute_maintenance_duration(self) -> int: + """Maintenance window length in minutes (handles 23→0 wrap).""" + return (self.maintenance_to - self.maintenance_from) % 1440 or 1440 + + def window_contains(self, start_min: int, end_min: int, win_from: int, win_to: int) -> bool: + """True if [start_min, end_min) intersects [win_from, win_to) (handles wrap).""" + # Normalise into [0, 1440) — same logic as the helper in calendar.py. + if win_to > win_from: + return start_min < win_to and end_min > win_from + # wrap window crosses midnight: [win_from..1440) ∪ [0..win_to) + return start_min < win_to or end_min > win_from diff --git a/app/schemas/schedule_type.py b/app/schemas/schedule_type.py index acdc695..e507884 100644 --- a/app/schemas/schedule_type.py +++ b/app/schemas/schedule_type.py @@ -1,27 +1,44 @@ -"""Schemas for ScheduleType CRUD.""" +"""Schemas for ScheduleType CRUD. + +All `*_from` / `*_to` values are **minutes since UTC midnight** (0-1439). +A maintenance window of variable length is allowed (1-180 minutes, +handles 23→0 wrap). +""" from pydantic import BaseModel, Field, model_validator from typing import Optional -def _validate_maintenance_window(maintenance_from: int, maintenance_to: int) -> None: - """Maintenance window must be exactly 1 hour (handles 23→0 wrap).""" - expected_to = (maintenance_from + 1) % 24 - if maintenance_to != expected_to: +_MAX_MIN = 1440 # 24 * 60 — exclusive upper bound + + +def _maintenance_duration(maint_from: int, maint_to: int) -> int: + """Maintenance window length in minutes; treats from==to as 24h (invalid).""" + return (maint_to - maint_from) % _MAX_MIN or _MAX_MIN + + +def _validate_maintenance_window(maint_from: int, maint_to: int) -> None: + dur = _maintenance_duration(maint_from, maint_to) + if dur < 1 or dur > 180: raise ValueError( - f"maintenance window must be exactly 1 hour: " - f"expected maintenance_to={expected_to}, got {maintenance_to}" + f"maintenance window duration must be in [1, 180] minutes; " + f"got {dur} (from={maint_from}, to={maint_to})" ) +def _validate_minute_field(name: str, value: int) -> None: + if value < 0 or value >= _MAX_MIN: + raise ValueError(f"{name} must be in [0, {_MAX_MIN}); got {value}") + + class ScheduleTypeCreate(BaseModel): name: str = Field(..., min_length=1, max_length=64) - work_from: int = Field(..., ge=0, le=23) - work_to: int = Field(..., ge=0, le=23) - entertainment_from: int = Field(..., ge=0, le=23) - entertainment_to: int = Field(..., ge=0, le=23) - maintenance_from: int = Field(8, ge=0, le=23, description="Maintenance window start hour UTC (default 8)") - maintenance_to: int = Field(9, ge=0, le=23, description="Maintenance window end hour UTC; must equal (maintenance_from+1) % 24") + work_from: int = Field(..., ge=0, lt=_MAX_MIN, description="Work start (minutes since UTC midnight, 0-1439)") + work_to: int = Field(..., ge=0, lt=_MAX_MIN) + entertainment_from: int = Field(..., ge=0, lt=_MAX_MIN) + entertainment_to: int = Field(..., ge=0, lt=_MAX_MIN) + maintenance_from: int = Field(480, ge=0, lt=_MAX_MIN, description="Maintenance start (default 480 = 8:00 UTC)") + maintenance_to: int = Field(540, ge=0, lt=_MAX_MIN, description="Maintenance end; (to-from) mod 1440 in [1,180]") @model_validator(mode="after") def _check_maintenance(self): @@ -31,12 +48,12 @@ class ScheduleTypeCreate(BaseModel): class ScheduleTypeUpdate(BaseModel): name: Optional[str] = Field(None, min_length=1, max_length=64) - work_from: Optional[int] = Field(None, ge=0, le=23) - work_to: Optional[int] = Field(None, ge=0, le=23) - entertainment_from: Optional[int] = Field(None, ge=0, le=23) - entertainment_to: Optional[int] = Field(None, ge=0, le=23) - maintenance_from: Optional[int] = Field(None, ge=0, le=23) - maintenance_to: Optional[int] = Field(None, ge=0, le=23) + work_from: Optional[int] = Field(None, ge=0, lt=_MAX_MIN) + work_to: Optional[int] = Field(None, ge=0, lt=_MAX_MIN) + entertainment_from: Optional[int] = Field(None, ge=0, lt=_MAX_MIN) + entertainment_to: Optional[int] = Field(None, ge=0, lt=_MAX_MIN) + maintenance_from: Optional[int] = Field(None, ge=0, lt=_MAX_MIN) + maintenance_to: Optional[int] = Field(None, ge=0, lt=_MAX_MIN) @model_validator(mode="after") def _check_maintenance(self): @@ -56,6 +73,7 @@ class ScheduleTypeResponse(BaseModel): entertainment_to: int maintenance_from: int maintenance_to: int + maintenance_duration_minutes: Optional[int] = None # derived; populated by router class Config: from_attributes = True diff --git a/app/schemas/schedule_type_special_slot.py b/app/schemas/schedule_type_special_slot.py index f3efcf0..62bc1e0 100644 --- a/app/schemas/schedule_type_special_slot.py +++ b/app/schemas/schedule_type_special_slot.py @@ -9,8 +9,8 @@ from pydantic import BaseModel, Field class SpecialSlotCreate(BaseModel): name: str = Field(..., min_length=1, max_length=64) description: Optional[str] = Field(None, max_length=512) - minute_in_window: int = Field(0, ge=0, le=59, description="Minute offset (0-59) inside the schedule_type maintenance window") - estimated_duration: int = Field(15, ge=1, le=60, description="Duration in minutes; must fit inside the 1-hour maintenance window") + minute_in_window: int = Field(0, ge=0, le=179, description="Minute offset (0-179) inside the schedule_type maintenance window") + estimated_duration: int = Field(15, ge=1, le=180, description="Duration in minutes; must fit inside the maintenance window (1-180min)") priority: int = Field(50, ge=0, le=99) event_data: Optional[dict[str, Any]] = Field(None, description="JSON payload merged into every materialised slot's event_data") is_active: bool = True @@ -19,8 +19,8 @@ class SpecialSlotCreate(BaseModel): class SpecialSlotUpdate(BaseModel): name: Optional[str] = Field(None, min_length=1, max_length=64) description: Optional[str] = Field(None, max_length=512) - minute_in_window: Optional[int] = Field(None, ge=0, le=59) - estimated_duration: Optional[int] = Field(None, ge=1, le=60) + minute_in_window: Optional[int] = Field(None, ge=0, le=179) + estimated_duration: Optional[int] = Field(None, ge=1, le=180) priority: Optional[int] = Field(None, ge=0, le=99) event_data: Optional[dict[str, Any]] = None is_active: Optional[bool] = None diff --git a/app/services/special_slot_materialiser.py b/app/services/special_slot_materialiser.py index c9aa8fc..17eaa17 100644 --- a/app/services/special_slot_materialiser.py +++ b/app/services/special_slot_materialiser.py @@ -145,11 +145,11 @@ def _build_time_slot_from_template( schedule_type: ScheduleType, template: ScheduleTypeSpecialSlot, ) -> TimeSlot: - scheduled_at = time_type( - hour=schedule_type.maintenance_from, - minute=template.minute_in_window, - second=0, - ) + # schedule_type.maintenance_from is minutes-since-UTC-midnight; the + # template's minute_in_window is an offset inside that window. Combined + # offset must fit in [0, 1440) and produce a wall-clock time_type. + total_min = (schedule_type.maintenance_from + template.minute_in_window) % 1440 + scheduled_at = time_type(hour=total_min // 60, minute=total_min % 60, second=0) # Merge admin-supplied event_data with bookkeeping pointers so the # agent (and ARD) can identify the template at wake time. merged_event_data = dict(template.event_data or {})