feat(spec): upgrade vBRIEF to v0.7#20
Conversation
- Add PlanItem type field (task/group/milestone/epic) with auto status rollup (#13) - Add PlanItem summary string field (#14) - Add PlanItem planRefs array for container items (#15) - Add extension properties namespace x-<consumer>/ with round-trip preservation (#12) - Add Source and Confidence recommended narrative keys (#17) - Add canonical x-vbrief/* reference type registry (#18) - Add failed and auto to Status enum - Update Python lib models, validation, and compat - Update examples and README to v0.7 - Close #19 (delta/patch document type — deferred) Closes #13, closes #14, closes #15, closes #17, closes #18 Refs #12
|
| Filename | Overview |
|---|---|
| libvbrief/validation.py | Adds item type and planRefs validation; critical bug: _validate_items never recurses into the new preferred items field, leaving all nested PlanItems under items unvalidated |
| libvbrief/compat/policy.py | Adds VALID_ITEM_TYPES, VALID_VERSIONS, EXTENSION_PROPERTY_PATTERN; adding "auto" to VALID_STATUSES inadvertently allows plan.status = "auto" to pass validation |
| libvbrief/models.py | Adds type, summary, planRefs, and items fields to PlanItem with correct round-trip support; post-constructor assignment for items is functional but inconsistent with idiomatic dataclass usage |
| schemas/vbrief-core-0.7.schema.json | New schema correctly adds type enum, summary, planRefs, auto status, patternProperties for extension namespace, and VBriefReference type registry |
| docs/vbrief-spec-0.7.md | New v0.7 spec document; well-structured with clear normative language covering all new features (type/summary/planRefs/extensions/provenance/registry) |
| libvbrief/compat/init.py | Exports new policy constants; clean addition with no issues |
| README.md | Updates version references to 0.7 and corrects repo URL from visionik/vBRIEF to deftai/vBRIEF |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[validate_document] --> B[_validate_root]
B --> C[_validate_plan]
C --> D{plan.status in VALID_STATUSES?}
D -- "auto now allowed ⚠️" --> E[No error raised]
C --> F[_validate_items - plan.items]
F --> G{For each item}
G --> H[Validate title/status/id/type/planRef/planRefs]
H --> I{item.get subItems?}
I -- present --> J[_validate_items recurse - subItems ✅]
I -- absent --> K[continue - skip item.get items ⚠️]
G --> K
J --> G
H --> L{status: auto?}
L -- "task type or childless" --> M[No error raised ⚠️]
Comments Outside Diff (2)
-
libvbrief/validation.py, line 228-239 (link)Nested
itemsfield not validated recursively_validate_itemsrecurses only intosubItems(line 239). The new v0.7 preferred fielditemson a PlanItem is fetched and parsed by the model (models.pyline 184–186) but is never passed to a recursive_validate_itemscall. Any PlanItems nested underitemsescape all validation checks — missing required fields, invalid statuses, badplanRefURIs, duplicate IDs, etc.Since v0.7 explicitly recommends
itemsoversubItemsand all new examples use it, this leaves the primary nesting path completely unvalidated.Prompt To Fix With AI
This is a comment left during a code review. Path: libvbrief/validation.py Line: 228-239 Comment: **Nested `items` field not validated recursively** `_validate_items` recurses only into `subItems` (line 239). The new v0.7 preferred field `items` on a PlanItem is fetched and parsed by the model (`models.py` line 184–186) but is never passed to a recursive `_validate_items` call. Any PlanItems nested under `items` escape all validation checks — missing required fields, invalid statuses, bad `planRef` URIs, duplicate IDs, etc. Since v0.7 explicitly recommends `items` over `subItems` and all new examples use it, this leaves the primary nesting path completely unvalidated. How can I resolve this? If you propose a fix, please make it concise.
-
libvbrief/validation.py, line 170-201 (link)status: "auto"on task-type or childless items not rejectedThe spec says
status: "auto"MUST NOT be used on items withtype: "task"or items with no children, and that this constitutes a validation error. The added validation (lines 195–201) only checks thattypeis a valid string; there is no cross-field check that catches{"type": "task", "status": "auto"}or{"type": "group", "status": "auto", "items": []}as errors.Prompt To Fix With AI
This is a comment left during a code review. Path: libvbrief/validation.py Line: 170-201 Comment: **`status: "auto"` on task-type or childless items not rejected** The spec says `status: "auto"` MUST NOT be used on items with `type: "task"` or items with no children, and that this constitutes a validation error. The added validation (lines 195–201) only checks that `type` is a valid string; there is no cross-field check that catches `{"type": "task", "status": "auto"}` or `{"type": "group", "status": "auto", "items": []}` as errors. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
libvbrief/validation.py:228-239
**Nested `items` field not validated recursively**
`_validate_items` recurses only into `subItems` (line 239). The new v0.7 preferred field `items` on a PlanItem is fetched and parsed by the model (`models.py` line 184–186) but is never passed to a recursive `_validate_items` call. Any PlanItems nested under `items` escape all validation checks — missing required fields, invalid statuses, bad `planRef` URIs, duplicate IDs, etc.
Since v0.7 explicitly recommends `items` over `subItems` and all new examples use it, this leaves the primary nesting path completely unvalidated.
### Issue 2 of 4
libvbrief/compat/policy.py:13-22
**`"auto"` in `VALID_STATUSES` makes `plan.status = "auto"` pass validation**
`VALID_STATUSES` is shared between plan-level and item-level status checks. Adding `"auto"` here means `_validate_plan` (`validation.py` line 109) will accept `plan.status = "auto"`, which has no defined semantics at the Plan level and directly violates the spec's constraint that `"auto"` is valid **only on container PlanItems** (`group`, `milestone`, `epic`) with children.
Additionally, no validation enforces the per-item constraint (`MUST NOT` on `type: "task"` or childless items), but the plan-level bleed-through is the most concrete present defect since it produces no error on clearly invalid documents.
### Issue 3 of 4
libvbrief/validation.py:170-201
**`status: "auto"` on task-type or childless items not rejected**
The spec says `status: "auto"` MUST NOT be used on items with `type: "task"` or items with no children, and that this constitutes a validation error. The added validation (lines 195–201) only checks that `type` is a valid string; there is no cross-field check that catches `{"type": "task", "status": "auto"}` or `{"type": "group", "status": "auto", "items": []}` as errors.
### Issue 4 of 4
libvbrief/models.py:184-192
**`items` field omitted from `from_dict` constructor call, then reassigned**
`items` is handled by setting `item.items` after the constructor call (lines 184–186), bypassing the dataclass constructor. This means the `items` dataclass field's `default_factory` is invoked by `cls(...)` (creating an empty list), and then immediately overwritten. While functionally correct today, it departs from the idiomatic dataclass pattern. A comment explaining the deliberate split (matching the `subItems` pattern) would prevent confusion in future refactors.
Reviews (1): Last reviewed commit: "feat(spec): upgrade vBRIEF to v0.7" | Re-trigger Greptile
| "running", | ||
| "completed", | ||
| "blocked", | ||
| "failed", | ||
| "cancelled", | ||
| "auto", | ||
| } | ||
|
|
||
| VALID_ITEM_TYPES: Final[set[str]] = { | ||
| "task", |
There was a problem hiding this comment.
"auto" in VALID_STATUSES makes plan.status = "auto" pass validation
VALID_STATUSES is shared between plan-level and item-level status checks. Adding "auto" here means _validate_plan (validation.py line 109) will accept plan.status = "auto", which has no defined semantics at the Plan level and directly violates the spec's constraint that "auto" is valid only on container PlanItems (group, milestone, epic) with children.
Additionally, no validation enforces the per-item constraint (MUST NOT on type: "task" or childless items), but the plan-level bleed-through is the most concrete present defect since it produces no error on clearly invalid documents.
Prompt To Fix With AI
This is a comment left during a code review.
Path: libvbrief/compat/policy.py
Line: 13-22
Comment:
**`"auto"` in `VALID_STATUSES` makes `plan.status = "auto"` pass validation**
`VALID_STATUSES` is shared between plan-level and item-level status checks. Adding `"auto"` here means `_validate_plan` (`validation.py` line 109) will accept `plan.status = "auto"`, which has no defined semantics at the Plan level and directly violates the spec's constraint that `"auto"` is valid **only on container PlanItems** (`group`, `milestone`, `epic`) with children.
Additionally, no validation enforces the per-item constraint (`MUST NOT` on `type: "task"` or childless items), but the plan-level bleed-through is the most concrete present defect since it produces no error on clearly invalid documents.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Upgrades the vBRIEF specification from v0.6 to v0.7, implementing issues #12–#18 (excluding #19, which was closed as deferred).
Changes
Spec (
docs/vbrief-spec-0.7.md)typefield on PlanItem (task/group/milestone/epic) withstatus: "auto"rollup (feat(schema): add type field to PlanItem — task, group, milestone, epic — with auto status rollup #13)summarystring on PlanItem (feat(schema): add summary string field to PlanItem as single-sentence shorthand #14)x-<consumer>/namespace with round-trip preservation MUST, forward collision protection, reserved tokens (RFC: ratify x-<consumer>/ extension namespace with round-trip preservation #12)x-vbrief/*reference type registry (feat(spec): define canonical x-vbrief/* reference type registry #18)failedstatus now normative in spec prose (was schema-only in v0.6)Schema (
schemas/vbrief-core-0.7.schema.json)typeenum,summarystring,planRefsarray on PlanItemautoadded to Status enumpatternPropertiesfor extension namespace at all object levelsSource/Confidencein narrative propertiesPython lib (
libvbrief/)models.py—type,summary,planRefs,itemsfields on PlanItem with round-trip supportcompat/policy.py—failed/autoin VALID_STATUSES; VALID_ITEM_TYPES, VALID_VERSIONS, EXTENSION_PROPERTY_PATTERNvalidation.py— Accepts 0.5/0.6/0.7; validatestypevalues andplanRefsentriesOther
Issues
Closes #13, closes #14, closes #15, closes #17, closes #18
Refs #12
#19 closed separately as deferred
Warp conversation