fix(lib): align Python implementation with v0.6 spec#8
Conversation
Four P1 defects identified in the Greptile review of PR #4: - compat/policy.py: add 'failed' to VALID_STATUSES; was missing despite being the headline v0.6 status, causing validation errors and strict- mode raises for any item with status='failed' - validation.py: introduce SUPPORTED_VERSIONS frozenset {'0.5','0.6'}; update version check to accept both; update _validate_items to traverse nested children under either 'items' (v0.6 preferred) or 'subItems' (legacy compat) using nested_key selection - models.py: add 'items' to _PLAN_ITEM_FIELD_ORDER so v0.6 nested children are not silently dropped into extras; update PlanItem.from_dict to prefer 'items' key over 'subItems'; update _known_item_values to emit 'items' (v0.6 preferred) on write; add PlanItem.failed factory - builder.py: replace hardcoded {'version':'0.5'} with '0.6' in all three document-producing functions (to_document, quick_todo, from_items) Also fix PROJECT.md which referenced TypeScript instead of Python (P2). Tests: 141 passed. Added explicit coverage for v0.6 document acceptance, 'failed' status, 'items' key traversal in validator, PlanItem.failed factory, builder version string, and nested-items serialization key.
Greptile SummaryThis PR correctly resolves all four P1 defects from the PR #4 review: Confidence Score: 5/5Safe to merge — all four prior P1 defects are correctly fixed and covered by 141 passing tests. All P1 issues from the previous review are resolved with correct implementations. The two remaining findings are P2 style observations (dead code and a naming imprecision in an error code) with no runtime impact. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Input document dict] --> B{Item has 'items' key?}
B -->|Yes - v0.6| C[Use 'items' for nested children]
B -->|No - legacy v0.5| D[Fall back to 'subItems']
C --> E[PlanItem.from_dict stores in .subItems field]
D --> E
E --> F[_validate_items validates nested children]
F --> G{to_dict serialize}
G --> H[_known_item_values always emits 'items' key]
H --> I[Output always uses v0.6 'items' key]
J[Builder / quick_todo / from_items] --> K[vBRIEFInfo version: '0.6']
K --> I
L[VALID_STATUSES] --> M[Now includes 'failed']
M --> N[Validator accepts 'failed' status]
M --> O[Builder strict mode accepts 'failed']
P[SUPPORTED_VERSIONS] --> Q[frozenset of '0.5' and '0.6']
Q --> R[Validator accepts both versions]
Prompt To Fix All With AIThis is a comment left during a code review.
Path: libvbrief/validation.py
Line: 207-211
Comment:
**`ISSUE_INVALID_SUBITEMS_TYPE` reused for v0.6 `items` key errors**
When the v0.6 `items` key is present but not a list, the error emitted uses `ISSUE_INVALID_SUBITEMS_TYPE` (`"invalid_subitems_type"`). The `path` and `message` are both correct, but any consumer doing `issue.code == "invalid_subitems_type"` and expecting it to refer to the legacy `subItems` key will see this code for a v0.6 `items` violation too. A dedicated constant like `ISSUE_INVALID_ITEMS_TYPE` in `compat/policy.py` would make the signal unambiguous.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: libvbrief/models.py
Line: 107-133
Comment:
**Inline classmethods are dead code superseded by `_StatusFactory`**
The `@classmethod` definitions for `pending`, `running`, `blocked`, `cancelled`, and `draft` inside the class body (lines 108–133) are unconditionally overwritten by `_StatusFactory` descriptors at module level (lines 499–505). The inline definitions are never called and can be removed without any behavioral change, which would reduce confusion about which path is actually executed.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(lib): align Python implementation wi..." | Re-trigger Greptile |
Summary
Addresses all four P1 defects identified by Greptile in the review of PR #4, plus the P2
PROJECT.mdtypo.Defects fixed
P1:
compat/policy.py—"failed"missing fromVALID_STATUSESThe headline v0.6 status caused validation errors and
ValueErrorin strict-mode builder calls. Added"failed"to the set.P1:
validation.py— validator rejects all v0.6 documentsThe version check compared against the literal string
"0.5". Introduced aSUPPORTED_VERSIONSfrozenset ({"0.5", "0.6"}) and updated the check and error message accordingly.P1:
validation.py—_validate_itemsignores v0.6itemskey on nested childrenThe validator only inspected
subItems. Updated to prefer theitemskey when present, falling back tosubItems, and routing the recursive call and error paths through the detected key name.P1:
models.py—PlanItem.from_dict()silently drops v0.6itemskey"items"to_PLAN_ITEM_FIELD_ORDERso it is not silently captured intoextrasfrom_dictto preferitemsoversubItemswhen both or either are present_known_item_valuesto emit"items"(v0.6 preferred) on serializationPlanItem.failedfactory via_StatusFactoryP1:
builder.py— all three helpers hardcodeversion: "0.5"to_document(),quick_todo(), andfrom_items()all emitted{"version": "0.5"}. Updated to"0.6".P2:
PROJECT.md— references TypeScript instead of PythonFixed header and tech stack line.
Tests
141 passed. New tests added covering:
"failed"status accepted at plan and item levelitemskey onPlanItemtraversed by_validate_itemsPlanItem.failed()factory produces correct status"0.6"across all three helpersitemskey (notsubItems)itemskey preferred oversubItemswhen both present on parse