feat: add milestone_type field to distinguish start/finish milestones
Add milestone_type field to milestone queries that indicates whether
a milestone is a start milestone ('start') or finish milestone ('finish').
Changes:
- Add milestone_type column to activities table schema
- Parse milestone_type from XER TASK table (MS_Start/MS_Finish)
- Include milestone_type in list_milestones response
- Update contract tests for milestone_type field
- Update specs, contracts, and documentation
The milestone_type is determined by:
1. Explicit milestone_type field in XER (MS_Start -> 'start', MS_Finish -> 'finish')
2. Derived from task_type (TT_Mile -> 'start', TT_FinMile -> 'finish')
This commit is contained in:
@@ -1,16 +1,18 @@
|
||||
# Tasks: Add Driving Flag to Relationships
|
||||
# Tasks: Add Milestone Type to List Milestones Tool
|
||||
|
||||
**Input**: Design documents from `/specs/001-schedule-tools/`
|
||||
**Prerequisites**: plan.md (required), spec.md (required), research.md, data-model.md, contracts/
|
||||
|
||||
**Tests**: TDD is mandated by constitution - tests MUST be written and fail before implementation.
|
||||
|
||||
**Scope**: Enhancement to existing implementation - add computed `driving` flag to relationship query responses.
|
||||
**Scope**: Enhancement to existing implementation - add `milestone_type` (start/finish) to milestone query responses per spec clarification.
|
||||
|
||||
**Change Summary**: The spec was updated to require that the `list_milestones` tool return both Start Milestones and Finish Milestones with their `milestone_type` field (start/finish for milestones, null otherwise).
|
||||
|
||||
## Format: `[ID] [P?] [Story] Description`
|
||||
|
||||
- **[P]**: Can run in parallel (different files, no dependencies)
|
||||
- **[Story]**: Which user story this task belongs to (US3 = Query Activity Relationships)
|
||||
- **[Story]**: Which user story this task belongs to (US4 = Query Project Summary)
|
||||
- Include exact file paths in descriptions
|
||||
|
||||
## Path Conventions
|
||||
@@ -23,65 +25,70 @@
|
||||
|
||||
## Phase 1: Setup (Schema Enhancement)
|
||||
|
||||
**Purpose**: Add early date columns needed for driving flag computation
|
||||
**Purpose**: Add milestone_type column needed for distinguishing start vs finish milestones
|
||||
|
||||
- [x] T001 Update activities table schema to add early_start_date and early_end_date columns in src/xer_mcp/db/schema.py
|
||||
- [x] T001 Update activities table schema to add milestone_type column in src/xer_mcp/db/schema.py
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Foundational (Parser Enhancement)
|
||||
|
||||
**Purpose**: Parse early dates from TASK table and store in database
|
||||
**Purpose**: Parse milestone_type from TASK table and store in database
|
||||
|
||||
**⚠️ CRITICAL**: Must complete before relationship queries can compute driving flag
|
||||
**⚠️ CRITICAL**: Must complete before milestone queries can return the type
|
||||
|
||||
### Tests
|
||||
|
||||
- [x] T002 [P] Unit test for TASK handler parsing early dates in tests/unit/test_table_handlers.py (verify early_start_date, early_end_date extracted)
|
||||
- [x] T002 [P] Unit test for TASK handler parsing milestone_type in tests/unit/test_table_handlers.py (verify milestone_type extracted for TT_Mile activities)
|
||||
|
||||
### Implementation
|
||||
|
||||
- [x] T003 Update TASK table handler to parse early_start_date and early_end_date in src/xer_mcp/parser/table_handlers/task.py
|
||||
- [x] T004 Update database loader to store early dates when inserting activities in src/xer_mcp/db/loader.py
|
||||
- [x] T003 Update TASK table handler to parse milestone_type field in src/xer_mcp/parser/table_handlers/task.py
|
||||
- [x] T004 Update database loader to store milestone_type when inserting activities in src/xer_mcp/db/loader.py
|
||||
|
||||
**Checkpoint**: Early dates are now parsed and stored - driving computation can begin
|
||||
**Checkpoint**: milestone_type is now parsed and stored - queries can begin
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: User Story 3 - Add Driving Flag to Relationships (Priority: P2) 🎯 FOCUS
|
||||
## Phase 3: User Story 4 - Add Milestone Type to Queries (Priority: P3)
|
||||
|
||||
**Goal**: Compute and return `driving` flag in all relationship query responses
|
||||
**Goal**: Return `milestone_type` field in milestone query responses
|
||||
|
||||
**Independent Test**: Load XER file, query relationships, verify driving flag is present and correctly computed
|
||||
**Independent Test**: Load XER file, query milestones, verify milestone_type is present and correctly identifies start vs finish milestones
|
||||
|
||||
### Tests for User Story 3
|
||||
### Tests for User Story 4
|
||||
|
||||
> **NOTE: Write these tests FIRST, ensure they FAIL before implementation**
|
||||
|
||||
- [x] T005 [P] [US3] Update contract test to verify driving flag in list_relationships response in tests/contract/test_list_relationships.py
|
||||
- [x] T006 [P] [US3] Update contract test to verify driving flag in get_predecessors response in tests/contract/test_get_predecessors.py
|
||||
- [x] T007 [P] [US3] Update contract test to verify driving flag in get_successors response in tests/contract/test_get_successors.py
|
||||
- [x] T008 [P] [US3] Unit test for driving flag computation logic in tests/unit/test_db_queries.py (test is_driving_relationship function)
|
||||
- [x] T005 [P] [US4] Update contract test to verify milestone_type field in list_milestones response in tests/contract/test_list_milestones.py
|
||||
- [x] T006 [P] [US4] Add test case verifying both start and finish milestones are returned with correct types in tests/contract/test_list_milestones.py
|
||||
|
||||
### Implementation for User Story 3
|
||||
### Implementation for User Story 4
|
||||
|
||||
- [x] T009 [US3] Create is_driving_relationship helper function in src/xer_mcp/db/queries.py (implements date comparison logic from research.md)
|
||||
- [x] T010 [US3] Update query_relationships function to JOIN on activity early dates and compute driving flag in src/xer_mcp/db/queries.py
|
||||
- [x] T011 [US3] Update get_predecessors function to JOIN on activity early dates and compute driving flag in src/xer_mcp/db/queries.py
|
||||
- [x] T012 [US3] Update get_successors function to JOIN on activity early dates and compute driving flag in src/xer_mcp/db/queries.py
|
||||
- [x] T007 [US4] Update query_milestones function to SELECT and return milestone_type in src/xer_mcp/db/queries.py
|
||||
- [x] T008 [US4] Update list_milestones tool docstring to document milestone_type field in src/xer_mcp/tools/list_milestones.py
|
||||
|
||||
**Checkpoint**: Driving flag now included in all relationship responses
|
||||
**Checkpoint**: milestone_type now included in milestone responses
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: Polish & Validation
|
||||
## Phase 4: Documentation & Contracts
|
||||
|
||||
**Purpose**: Verify integration and documentation alignment
|
||||
**Purpose**: Update contracts and documentation to reflect the new field
|
||||
|
||||
- [x] T013 Integration test validating driving flag against sample XER data in tests/integration/test_xer_parsing.py
|
||||
- [x] T014 Run all tests to verify no regressions: pytest tests/
|
||||
- [x] T015 Run quickstart.md validation - verify driving flag examples match actual output
|
||||
- [x] T016 Run ruff check and fix any linting issues: ruff check src/
|
||||
- [x] T009 [P] Update ActivitySummary schema to include optional milestone_type field in specs/001-schedule-tools/contracts/mcp-tools.json
|
||||
- [x] T010 [P] Update Activity entity in data-model.md to include milestone_type field in specs/001-schedule-tools/data-model.md
|
||||
- [x] T011 Update quickstart.md milestone example to show milestone_type in output in specs/001-schedule-tools/quickstart.md
|
||||
|
||||
---
|
||||
|
||||
## Phase 5: Polish & Validation
|
||||
|
||||
**Purpose**: Verify integration and ensure no regressions
|
||||
|
||||
- [x] T012 Run all tests to verify no regressions: uv run pytest tests/
|
||||
- [x] T013 Run ruff check and fix any linting issues: uv run ruff check src/
|
||||
- [x] T014 Validate list_milestones output matches updated contract schema
|
||||
|
||||
---
|
||||
|
||||
@@ -99,107 +106,106 @@ Phase 2 (Parser)
|
||||
Phase 3 (Queries) ← MAIN WORK
|
||||
│
|
||||
▼
|
||||
Phase 4 (Polish)
|
||||
```
|
||||
|
||||
### Task Dependencies Within Phase 3
|
||||
|
||||
```
|
||||
T005, T006, T007, T008 (Tests) ← Write first, verify FAIL
|
||||
Phase 4 (Docs) ← Can run in parallel with Phase 3
|
||||
│
|
||||
▼
|
||||
T009 (Helper function)
|
||||
Phase 5 (Polish)
|
||||
```
|
||||
|
||||
### Task Dependencies Within Phases
|
||||
|
||||
```
|
||||
T001 (Schema)
|
||||
│
|
||||
├── T010 (query_relationships)
|
||||
├── T011 (get_predecessors)
|
||||
└── T012 (get_successors)
|
||||
├── T002 (Parser test)
|
||||
│
|
||||
▼
|
||||
All tests now PASS
|
||||
T003 (Parser impl) ← depends on T001
|
||||
│
|
||||
▼
|
||||
T004 (Loader) ← depends on T003
|
||||
│
|
||||
├── T005, T006 (Contract tests)
|
||||
│
|
||||
▼
|
||||
T007 (Query impl) ← depends on T004
|
||||
│
|
||||
▼
|
||||
T008 (Tool docs)
|
||||
```
|
||||
|
||||
### Parallel Opportunities
|
||||
|
||||
**Phase 2 Tests + Phase 3 Tests** (can write all tests in parallel):
|
||||
```bash
|
||||
# All test tasks can run in parallel:
|
||||
Task T002: "Unit test for TASK handler parsing early dates"
|
||||
Task T005: "Update contract test for list_relationships"
|
||||
Task T006: "Update contract test for get_predecessors"
|
||||
Task T007: "Update contract test for get_successors"
|
||||
Task T008: "Unit test for driving flag computation"
|
||||
Task T002: "Unit test for TASK handler parsing milestone_type"
|
||||
Task T005: "Update contract test for list_milestones"
|
||||
Task T006: "Add test for start/finish milestone types"
|
||||
```
|
||||
|
||||
**Phase 3 Query Updates** (after T009 is complete):
|
||||
**Phase 4 Documentation** (can run in parallel):
|
||||
```bash
|
||||
# These can run in parallel once helper function exists:
|
||||
Task T010: "Update query_relationships"
|
||||
Task T011: "Update get_predecessors"
|
||||
Task T012: "Update get_successors"
|
||||
Task T009: "Update ActivitySummary schema"
|
||||
Task T010: "Update Activity entity in data-model.md"
|
||||
Task T011: "Update quickstart.md example"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Parallel Example: Phase 3
|
||||
## Implementation Details
|
||||
|
||||
```bash
|
||||
# First: Write all tests in parallel
|
||||
Task: "Update contract test for list_relationships in tests/contract/test_list_relationships.py"
|
||||
Task: "Update contract test for get_predecessors in tests/contract/test_get_predecessors.py"
|
||||
Task: "Update contract test for get_successors in tests/contract/test_get_successors.py"
|
||||
Task: "Unit test for driving flag computation in tests/unit/test_db_queries.py"
|
||||
### Milestone Type Determination
|
||||
|
||||
# Verify all tests FAIL (driving flag not yet implemented)
|
||||
In Primavera P6, milestones are identified by `task_type = 'TT_Mile'`. The distinction between Start and Finish milestones is typically:
|
||||
|
||||
# Then: Implement helper function
|
||||
Task: "Create is_driving_relationship helper in src/xer_mcp/db/queries.py"
|
||||
1. **XER Field**: Check for `milestone_type` field in TASK table (values: `MS_Start`, `MS_Finish`)
|
||||
2. **Fallback**: If field not present, can infer from dates:
|
||||
- Start milestone: Has only `target_start_date` (or start = end)
|
||||
- Finish milestone: Has only `target_end_date` (or represents completion)
|
||||
|
||||
# Then: Update all queries in parallel
|
||||
Task: "Update query_relationships in src/xer_mcp/db/queries.py"
|
||||
Task: "Update get_predecessors in src/xer_mcp/db/queries.py"
|
||||
Task: "Update get_successors in src/xer_mcp/db/queries.py"
|
||||
|
||||
# Verify all tests PASS
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Implementation Strategy
|
||||
|
||||
### Driving Flag Computation (from research.md)
|
||||
|
||||
The driving flag is computed at query time by comparing dates:
|
||||
|
||||
```python
|
||||
def is_driving_relationship(
|
||||
pred_early_end: str | None,
|
||||
succ_early_start: str | None,
|
||||
lag_hours: float,
|
||||
pred_type: str
|
||||
) -> bool:
|
||||
"""Determine if relationship is driving based on early dates."""
|
||||
if pred_early_end is None or succ_early_start is None:
|
||||
return False
|
||||
|
||||
# For FS: pred_end + lag = succ_start means driving
|
||||
if pred_type == "FS":
|
||||
# Parse dates, add lag, compare with 1-hour tolerance
|
||||
...
|
||||
|
||||
# Similar logic for SS, FF, SF
|
||||
return False
|
||||
```
|
||||
|
||||
### SQL Query Pattern
|
||||
### Schema Change
|
||||
|
||||
```sql
|
||||
SELECT r.*,
|
||||
pred.early_end_date,
|
||||
succ.early_start_date,
|
||||
-- Compute driving in Python after fetch, or use CASE in SQL
|
||||
FROM relationships r
|
||||
JOIN activities pred ON r.pred_task_id = pred.task_id
|
||||
JOIN activities succ ON r.task_id = succ.task_id
|
||||
-- Add to activities table
|
||||
milestone_type TEXT -- 'start', 'finish', or NULL for non-milestones
|
||||
```
|
||||
|
||||
### Query Change
|
||||
|
||||
```sql
|
||||
SELECT task_id, task_code, task_name,
|
||||
target_start_date, target_end_date,
|
||||
status_code, milestone_type
|
||||
FROM activities
|
||||
WHERE task_type = 'TT_Mile'
|
||||
ORDER BY target_start_date, task_code
|
||||
```
|
||||
|
||||
### Response Format
|
||||
|
||||
```json
|
||||
{
|
||||
"milestones": [
|
||||
{
|
||||
"task_id": "M001",
|
||||
"task_code": "MS-START",
|
||||
"task_name": "Project Start",
|
||||
"milestone_type": "start",
|
||||
"target_start_date": "2026-01-15T08:00:00",
|
||||
"target_end_date": "2026-01-15T08:00:00",
|
||||
"status_code": "TK_Complete"
|
||||
},
|
||||
{
|
||||
"task_id": "M002",
|
||||
"task_code": "MS-END",
|
||||
"task_name": "Project Complete",
|
||||
"milestone_type": "finish",
|
||||
"target_start_date": "2026-06-30T17:00:00",
|
||||
"target_end_date": "2026-06-30T17:00:00",
|
||||
"status_code": "TK_NotStart"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
@@ -210,30 +216,40 @@ JOIN activities succ ON r.task_id = succ.task_id
|
||||
|-------|-------|-------|
|
||||
| Setup | 1 | Schema update |
|
||||
| Foundational | 3 | Parser enhancement |
|
||||
| US3 Implementation | 8 | Driving flag computation |
|
||||
| Polish | 4 | Validation |
|
||||
| **Total** | **16** | |
|
||||
| US4 Implementation | 4 | Milestone type in queries |
|
||||
| Documentation | 3 | Contracts & docs |
|
||||
| Polish | 3 | Validation |
|
||||
| **Total** | **14** | |
|
||||
|
||||
### Task Distribution by Type
|
||||
|
||||
| Type | Count | IDs |
|
||||
|------|-------|-----|
|
||||
| Schema | 1 | T001 |
|
||||
| Tests | 5 | T002, T005-T008 |
|
||||
| Implementation | 6 | T003-T004, T009-T012 |
|
||||
| Validation | 4 | T013-T016 |
|
||||
| Tests | 3 | T002, T005, T006 |
|
||||
| Implementation | 4 | T003, T004, T007, T008 |
|
||||
| Documentation | 3 | T009, T010, T011 |
|
||||
| Validation | 3 | T012, T013, T014 |
|
||||
|
||||
### Independent Test Criteria
|
||||
|
||||
| Verification | How to Test |
|
||||
|--------------|-------------|
|
||||
| milestone_type parsed | Unit test: parse XER with milestones, verify milestone_type extracted |
|
||||
| milestone_type in response | Contract test: call list_milestones, verify milestone_type field present |
|
||||
| Start/Finish distinction | Contract test: verify "Project Start" has type="start", "Project Complete" has type="finish" |
|
||||
|
||||
### MVP Scope
|
||||
|
||||
Complete through T012 for minimal viable driving flag implementation.
|
||||
Complete through T008 for minimal viable milestone_type implementation.
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
- [P] tasks = different files, no dependencies on incomplete tasks
|
||||
- [US3] = User Story 3 (Query Activity Relationships)
|
||||
- [US4] = User Story 4 (Query Project Summary - milestones)
|
||||
- Constitution mandates TDD: write tests first, verify they fail, then implement
|
||||
- Driving flag is COMPUTED at query time, not stored in database
|
||||
- Use 1-hour tolerance for floating-point date arithmetic
|
||||
- milestone_type is stored in database, not computed at query time
|
||||
- For non-milestone activities, milestone_type should be NULL
|
||||
- Commit after each task or logical group
|
||||
|
||||
Reference in New Issue
Block a user