Add roadmaps - #93
Add roadmaps#93
Conversation
Signed-off-by: Erick Bourgeois <erick@jeb.ca>
ebourgeois
left a comment
ebourgeois
left a comment
There was a problem hiding this comment.
Post-merge review of the roadmap docs, checked against the code at main (87d1d22). The three design issues in roadmap 01 (dropped validations/references, the $ref target, apiVersion) need a decision before convert is built. Renumbering to 00/01 is being fixed in a follow-up.
|
|
||
| ### 2.1 Full field mapping | ||
|
|
||
| | Legacy | New | Notes | |
There was a problem hiding this comment.
No home for validations or references. Both were added in addfbbb. The roadmap counts 15 top-level keys; main has 16. The stop condition and definition of done name only five generators and omit validations and server.
Following §2.1, Resource.from_legacy() and firestone convert would drop the validations.rules block, so a converted addressbook.yaml silently loses its CEL rules and its x-firestone-validations output. The Phase 1 raw-dict count (59 reads in 5 modules) skips validations.py. references (addressbook.yaml:171) would also fail the proposed additionalProperties: false schema, since it gets no x-firestone-* mapping the way expose does.
| | Silent output drift during the Phase 1 refactor | Golden-file tests captured **before** any change; byte-comparison in CI | | ||
| | firestone-lib coupling (`validate()` reaches into `firestone.schema`) | Keep `resource.yaml` in place and unchanged; dispatch in firestone (§3.5) | | ||
| | Downstream projects with many legacy resource files | `firestone convert`, plus a long dual-read window | | ||
| | `$ref` across resource files (`person.yaml#/schema`) breaks | The `$ref` target path changes to `#/spec/versions/0/schema/openAPIV3Schema`; `convert` must rewrite intra-repo `$ref`s, and this needs its own test — `jsonref` resolves these at load time in `firestone_lib.get_resource_schema()` | |
There was a problem hiding this comment.
$ref rewrite target does not match the original. person.yaml#/schema points at the whole collection wrapper (type: array, key, items, descriptions). openAPIV3Schema is only the old schema.items.
convert would rewrite addressbook.yaml:167 to #/spec/versions/0/schema/openAPIV3Schema, but the nested-resource path code (openapi.py:420-424) reads prop_schema["schema"]["items"] and the key. The nested person paths then break or disappear, contradicting the byte-identical promise in §1.2 and definition-of-done item 4.
| plural: addresses | ||
| ``` | ||
|
|
||
| **`apiVersion` is not an API version.** `apiVersion: v1` is only consulted |
There was a problem hiding this comment.
apiVersion description is wrong. The code builds f"v{rsrc['apiVersion']}/", so the documented apiVersion: v1 produces /vv1/, not /v1.0/. asyncapi.py:438 also reads it, so openapi.py is not the only reader.
Phase 1's byte-identical golden tests would lock that bug in, and the §2.1 mapping to spec.versions[0].name carries it forward instead of fixing it.
|
|
||
| **`kind` is overloaded four ways.** It is the plural URL segment, the OpenAPI | ||
| tag, the generated module name, and the input to | ||
| `spec_base.to_singular()` ([`_base.py:33`](../../firestone/spec/_base.py)) for |
There was a problem hiding this comment.
Line references were stale on commit. _base.py:33 is now pascal, not to_singular. openapi.py:156, :417-420 and :538 have each moved down 4 lines. openapi.py:602 is now a docstring (the versionInPath read is at :662). README.md:243 is line 247 at HEAD (245 even at c301292).
A reader following the links in §1.1, §2 and Phase 5 (and ROADMAPS.md:25) lands on the wrong code; the Phase 5 README fix would hit the apiVersion paragraph instead of the metadata.description claim.
|
|
||
| ## Index | ||
|
|
||
| Statuses were verified against `main` @ `c301292` on 2026-09-16. |
There was a problem hiding this comment.
The verification pin matches neither document. It says verified against main @ c301292, but roadmap 02's inventory counts validations.py and server_rust.py, which arrived later in addfbbb and 83f5bd5. Roadmap 02's status line (02:14-16) still calls that work uncommitted. Roadmap 01's line numbers, meanwhile, are off against HEAD. Neither document is consistent with any single commit.
|
|
||
| ### 1.1 What is kept | ||
|
|
||
| - **`firestone/schema/**/*.jinja2` — 4,126 lines across 30 templates.** More |
There was a problem hiding this comment.
Counts are off. There are 29 .jinja2 templates, not 30 (also at :191 and :253). __main__.py defines six generate subcommands (openapi, asyncapi, cli, streamlit, validations, server), not seven as :22 and ROADMAPS.md:26 say.
Phase 1's MiniJinja spike checklist asks for all 30 templates, so it can never be ticked off.
| xtask/ differential harness + corpus runner | ||
| ``` | ||
|
|
||
| `firestone-templates` is separate on purpose: it keeps templates loadable both |
There was a problem hiding this comment.
--template does not load from a directory. It takes a single file, exists only on cli and streamlit, and renders through a bare jinja2.Environment with no yaml_pretty/pascal filters and no autoescape setting (cli.py:345, cli_rust.py:831, streamlit.py:333).
The planned firestone-templates crate and the §4.5 compatibility survey assume one shared environment. A single-environment port changes behaviour for --template users, and the corpus diff will not catch it unless custom templates are in the corpus.
| @@ -0,0 +1,425 @@ | |||
| # 01 — CRD-Aligned Resource Schema | |||
There was a problem hiding this comment.
Numbering should start at 00. The convention is a zero-padded two-digit prefix, contiguous from 00. These should be 00-crd-aligned-resource-schema.md and 01-rust-rewrite.md, with ROADMAPS.md, the community README and 02's "roadmap 01" prose renumbered in the same commit.
| ## Numbering | ||
|
|
||
| Numbers are sequential and stable once assigned: the next roadmap is the next | ||
| free number, regardless of subject. firestone is small enough that grouping |
There was a problem hiding this comment.
Numbering policy contradicts the convention. Lines 11-15 say numbers are stable once assigned and suggest themed bands past a dozen documents while renumbering nothing. The convention is no bands, and renumber the run (fixing every reference in the same commit) when a roadmap is inserted or retired.
| @@ -0,0 +1,431 @@ | |||
| # 02 — Port the firestone CLI to Rust | |||
There was a problem hiding this comment.
Em-dashes throughout (80 across the four files: 38 in 02, 31 in 01, 7 in ROADMAPS.md, 4 in README.md), including the titles. Titles should read # NN: Title, e.g. # 01: Port the firestone CLI to Rust.
2 participants
Add roadmaps