docs(openspec): no demo findings, and the signature's version states its coverage - #247
Conversation
|
@claude /review |
|
Claude finished @thecodedrift's task in 3m 0s —— View job Review: docs(openspec): no demo findings, and the signature's version states its coverageReview mode: incremental — read 0 prior review thread(s) before reviewing.
Verified claimsEverything the PR body asserts about the current codebase checks out:
No source files are touched by this PR — everything is under Findings[New] One finding, posted inline on Other notes (no action needed)
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The inline finding was real and is fixed in abb551f. Worth recording the part the review left open: whether the omission was harmful "depends on how the sync/archive step is actually run". Measured it. Both are carried verbatim now, and re-running the same experiment leaves all four scenarios in place. Task 2.3 records the measurement so the next MODIFIED delta here does not rediscover it. Thanks for cross-checking the body's claims against — AI Coding Agent |
23772fd to
fb4c8dd
Compare
The generator team withdrew the demonstration findings across three of their own rounds, and the argument settles it: the verification gate already refuses a rule whose fixture asymmetry does not hold, so a rule that reaches delivery has necessarily flagged the failing example and ignored the passing one. Findings served alongside it are a constant implied by delivery, not a result. D6 becomes that withdrawal rather than disappearing. The wrapper we designed was aimed at a real case, a flat list rendering identically for a rule that catches the bad examples and one that fires on everything, and it was the right fix to the wrong problem: that case is answered upstream of the payload. What survives is where the concern actually lives. With nothing served beside the rule, running it is the whole demonstration, and `taskless test` reports a runtime rule as passing when its fixtures never ran. Task 1.6 is now blocked on that rather than on a render. The spec requirement becomes "the demo serves the rule and nothing beside it", forbidding the CLI from requiring or reading a demo-only field. Task 0.4 is marked void rather than done: there is no shape left to agree.
The envelope is self-describing about how a signature was computed and silent about what it is over. Which file a rule's signature covers was agreed between the two teams and written down in neither specification. That gap surfaced as a proposal to move `signature` from the rule onto each file, because "nothing in the payload says which file the signature covers". The observation was right and the remedy was not: coverage is a property of the signature scheme rather than of a delivery, so it belongs on the version, not distributed across the collection. The proposal was declined and this is what we owe instead. Stating it answers what the proposal was reaching for. A new algoVersion is already the mechanism for changing what a signature means, so an engine needing to sign more than one file does so under a v2 signature rather than a new payload shape. Documentation-level. `ALGO_VERSION` stays 1, `signRuleFile` hashes one path, and `ENGINE_LAYOUTS` gives every engine exactly one `ruleFile` — `<id>.yml` for sg and vale, `check.ts` for runtime — so the statement generalises rather than special-casing the runtime tier.
The MODIFIED delta restated the signature envelope requirement but carried only one of its three scenarios, so `Version is read before parameters` and `Signatures compare as whole strings` would have been deleted from the standing spec by a change whose proposal calls itself strictly additive. Measured rather than reasoned about: archiving the change and diffing the result shows `openspec archive` rewrites the requirement to exactly what the delta contains. It is a replacement, not a patch, and an omitted scenario is dropped silently. Re-run with the scenarios restored, all four survive. Recorded as task 2.3 so the next MODIFIED delta in this repository does not learn it the same way.
A `## MODIFIED Requirements` block must restate the requirement in full, including the scenarios it is not changing. `openspec archive` rewrites the standing requirement to exactly what the delta contains, so anything omitted is deleted from the spec with nothing reporting it. Found on this branch's own change, which called itself strictly additive and would have dropped two normative scenarios about parameter-parsing order and whole-string signature comparison. `openspec validate --strict` passes either way, which is why the section carries the archive-and-diff procedure that found it rather than only the rule. Also records that the preserve-on-sync guidance in `openspec-sync-specs` does not reach `openspec archive`, so a delta that omits a scenario and a note saying the omission is deliberate produce the same result.
fb4c8dd to
0b944c5
Compare
#247 merged the change and never took its tip step, so the statement it exists to make lived in a change directory and not in the spec. Archiving is what promotes a delta into `openspec/specs/`, which means until now `cli-rule-reconciliation` did not carry the coverage rule we declined the generator team's per-file signature proposal on the strength of writing down. All four scenarios survive the promotion, including the two that were nearly dropped in review: parameter-parsing order and whole-string comparison. Nothing checks for this in either direction, which is how #246 came to be outstanding too. A single-PR change has no later slice to carry the archive, so it has to happen on the same PR or immediately after.
1 participant
Two docs-only changes, both fallout from the same round with the generator team. No source changes; every path is under
openspec/.1. There are no demo findings (
runtime-demo-path)The generator team withdrew the demonstration findings across three of their own rounds. Their argument settles it: the verification gate already refuses a rule whose fixture asymmetry does not hold, so a rule that reaches delivery has necessarily flagged the failing example and ignored the passing one. Findings served alongside would be a constant implied by delivery — a field whose content a reader could infer from having received a rule at all.
D6becomes that withdrawal rather than disappearing, because the reasoning is worth keeping. Our wrapper was aimed at a real case — a flat list renders identically for a rule that catches the bad examples and one that fires on everything — and it was the right fix to the wrong problem: that case is answered upstream of the payload, not in it.What survives is the concern, relocated. With nothing served beside the rule, running it is the demonstration. And
taskless testcurrently reports a runtime rule as✓with "1 rule(s) tested" and exit 0 when its fixtures never ran (ran: falseexists only in--json; the human renderer never reads it). Task 1.6 is now blocked on that rather than on a render.D62. The signature's version states its coverage (new change)
The envelope is self-describing about how a signature was computed and silent about what it is over. Which file a rule's signature covers was agreed between the two teams and written down in neither specification.
That gap surfaced as a proposal to move
signaturefrom the rule onto each file, because "nothing in the payload says which file the signature covers". The observation was right; the remedy was not. Coverage is a property of the signature scheme rather than of a delivery, so it belongs on the version rather than distributed across the collection. The proposal was declined, and this is what we owe instead — it also answers what the proposal was reaching for, since a new algoVersion is already the mechanism for changing what a signature means.Verified rather than asserted:
ENGINE_LAYOUTSgives every engine exactly oneruleFile—<id>.ymlfor sg and vale,check.tsfor runtime — so this is stated at the engine level rather than special-casing runtime.ALGO_VERSIONis 1 (rule-hash.ts:14),signRuleFilehashes one path (rule-hash.ts:116),run-set.ts:41signsrule.checkFileonce per rule.Checks
openspec validate --changes --strictpasses on all three changes.pnpm cli checkreports only the 4 pre-existingno-hedgingwarnings (3route.txt, 1onboard.txt).