Skip to content

Commit bfc1129

Browse files
committed
docs(gh62): drop preemptive sub-issues — matrix is the live backlog
The previous version of task 7.4 opened one GitHub issue per Fix-now and Follow-up ledger entry at archive time, intended as live tracking after the change directory is archived. That creates a parallel source of truth that must be kept in sync with the matrix, and preemptively scaffolds N issues for closures that may never be scheduled (violating P1 simplicity and the "no speculative work" principle). The matrix at docs/aspectj_grammar_coverage.md is sufficient on its own: any row with Verdict = SILENT-GAP is by definition outstanding work. Future closures are scheduled by opening one OpenSpec change per closure (with its own GitHub issue at that point) — not by maintaining a backlog of issues created up-front. Changes: - tasks.md: drop task 7.4 (open sub-issues); renumber 7.5..7.8 -> 7.4..7.7; add explicit note that future closures open their own issues when scheduled - design.md: D4 rationale rewritten — rejects the GitHub-issues alternative on simplicity grounds; architecture diagram updated; risks/mitigations reframed - spec.md: Requirement "Scope Ledger" reframed as one-shot snapshot; scenario "opening a sub-change" no longer references issues opened by task 7.4 refs #62
1 parent b7aeabc commit bfc1129

3 files changed

Lines changed: 18 additions & 18 deletions

File tree

rv-android/openspec/changes/gh62-aspectj-grammar-coverage/design.md

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ Relevant PRD references: **FR02** (APK instrumentation — this change does not
5858
│ ┌──────────────────────────────────────┐ │
5959
│ │ openspec/changes/gh62-.../ledger.md │ ← scope ledger │
6060
│ │ (Fix-now / Follow-up / Deferred) │ │
61-
│ │ snapshot at archive; live tracking │ │
62-
│ │ via GitHub issues labelled `gh62` │ │
61+
│ │ one-shot snapshot at archive time; │ │
62+
│ │ matrix SILENT-GAP rows = live state │ │
6363
│ └──────────────┬───────────────────────┘ │
6464
│ │ schedules │
6565
│ ▼ │
@@ -89,7 +89,7 @@ Relevant PRD references: **FR02** (APK instrumentation — this change does not
8989
|-------------|----------------|------|
9090
| AspectJ Grammar Coverage Matrix as Contract (R1) | `docs/aspectj_grammar_coverage.md` | `MatrixIntegrityTest.{testEveryDesignatorHasMatrixRow,testVerdictsAreValid,testCoveredRowsCiteEnabledPassingTests,testSilentGapRowsHaveDisabledTestAndLedgerEntry,testEnabledTestsResolveToCoveredOrExplicitNoOpRow,testDisabledTestsResolveToSilentGapRow,testSkipCountEqualsSilentGapCount,testDemandCountsReproducible}` |
9191
| Grammar Tests Maven Submodule (R2) | `rvsec-android/rvsec-instrumentation-dexlib2/grammar-tests/pom.xml` + `src/test/java/.../grammar/*.java` | `mvn -pl grammar-tests test` SHALL pass with 0 failures and `Skipped == SILENT-GAP count` |
92-
| Scope Ledger for Future Closures (R3) | `openspec/changes/gh62-.../ledger.md` + GitHub issues labelled `gh62` | `MatrixIntegrityTest.testSilentGapRowsHaveDisabledTestAndLedgerEntry` cross-checks every SILENT-GAP row against ledger |
92+
| Scope Ledger for Future Closures (R3) | `openspec/changes/gh62-.../ledger.md` (snapshot at archive) | `MatrixIntegrityTest.testSilentGapRowsHaveDisabledTestAndLedgerEntry` cross-checks every SILENT-GAP row against ledger |
9393
| INV-INS-88 (closed enumeration) | Matrix + `AspectJDesignators.DESIGNATORS` | `testEveryDesignatorHasMatrixRow` (asserts set equality, not subset) |
9494
| INV-INS-89 (verdict ∈ 4-value set; NOT-NEEDED requires zero demand AND no impl) | Matrix structure | `testVerdictsAreValid` |
9595
| INV-INS-90 (COVERED → enabled passing test, no inherited `@Disabled`) | Matrix `Evidence` column + `grammar-tests/` | `testCoveredRowsCiteEnabledPassingTests` (reflection walks superclasses for inherited `@Disabled`) |
@@ -105,7 +105,7 @@ Relevant PRD references: **FR02** (APK instrumentation — this change does not
105105
- Every matrix row carries a verdict from `{COVERED, SILENT-GAP, EXPLICIT-NO-OP, NOT-NEEDED}` anchored to a `file:line` in the dexlib2 source and to a named test in `grammar-tests/`.
106106
- A new Maven submodule `grammar-tests/` provides executable coverage of every matrix row, with `@Disabled` failing tests for every `SILENT-GAP`. `MatrixIntegrityTest` enforces both directions of the matrix↔tests link and asserts `Skipped == SILENT-GAP count`.
107107
- `DemandCounter` (portable Java) replaces shell `grep` for demand counts; `MatrixIntegrityTest.testDemandCountsReproducible` invokes it directly.
108-
- A scope ledger at `openspec/changes/gh62-aspectj-grammar-coverage/ledger.md` classifies every `SILENT-GAP` into `Fix-now`, `Follow-up`, or `Deferred-by-design`, with `Owner` + `Target milestone` per entry. Ongoing tracking moves to GitHub issues labelled `gh62`.
108+
- A scope ledger at `openspec/changes/gh62-aspectj-grammar-coverage/ledger.md` classifies every `SILENT-GAP` into `Fix-now`, `Follow-up`, or `Deferred-by-design`, with `Owner` + `Target milestone` per entry. The ledger is a one-shot snapshot archived with the change; the matrix is the live backlog.
109109
- A PR-check GitHub Action blocks production changes in `pointcut-engine`/`advice-emitter`/`dex-mutator`/`coverage-weaver` without matching matrix updates (operational enforcement of closure atomicity).
110110
- Future closures (`gh-XX`) cite gh62, name the matrix rows they flip, and update the matrix and the test annotation atomically in the same commit; the PR-check fails the PR otherwise.
111111

@@ -158,16 +158,17 @@ Relevant PRD references: **FR02** (APK instrumentation — this change does not
158158
- *Comment out the failing tests.* Rejected: hides the gap from the test report entirely.
159159
- *Use `Assumptions.assumeFalse(true)` inside the test body.* Rejected: visually identical to a passing test in the report; removes the gap from `Skipped` count.
160160

161-
### D4 — Ledger is a snapshot in the change directory; live tracking via GitHub issues
161+
### D4 — Ledger is a one-shot snapshot; the matrix itself is the live backlog
162162

163-
**Choice:** The matrix records *what is true today* (verdict + evidence). The ledger records *what we plan to do about it* at the time of merging gh62 (Fix-now / Follow-up / Deferred-by-design with rationale + `Owner` + `Target milestone`). They live in two files: `docs/aspectj_grammar_coverage.md` (matrix, persistent) and `openspec/changes/gh62-aspectj-grammar-coverage/ledger.md` (ledger snapshot — archived with the change). Live tracking after archive moves to GitHub issues, one per `Fix-now` and `Follow-up` entry, labelled `gh62` (created by task 7.4).
163+
**Choice:** The matrix records *what is true today* (verdict + evidence). The ledger records *what we plan to do about it* at the time of merging gh62 (Fix-now / Follow-up / Deferred-by-design with rationale + `Owner` + `Target milestone`). They live in two files: `docs/aspectj_grammar_coverage.md` (matrix, persistent) and `openspec/changes/gh62-aspectj-grammar-coverage/ledger.md` (ledger snapshot — archived with the change). The ledger is **not** kept alive after archive; the matrix is the live source of truth — any row with `Verdict = SILENT-GAP` is by definition outstanding work.
164164

165-
**Why:** The matrix is permanent — every future closure updates it but the document persists across milestones. The ledger is a planning artefact — it makes sense in the context of the current scheduling decision. The cross-LLM review flagged "ledger becomes dead document post-archive" as a top risk; the mitigation is *not* to keep the ledger live (it would drift) but to delegate live tracking to the issue tracker, which the team already uses.
165+
**Why:** The matrix is permanent — every future closure updates it but the document persists across milestones. The ledger is a planning artefact — it makes sense in the context of the current scheduling decision. The cross-LLM review flagged "ledger becomes dead document post-archive" as a top risk; an earlier draft of this design tried to mitigate that by creating one GitHub issue per `Fix-now`/`Follow-up` entry (task 7.4) for live tracking. That mitigation was rejected on simplicity grounds: it creates a parallel source of truth that must be kept in sync with the matrix, multiplies the artefacts a closure must update, and preemptively opens N issues for closures that may never be scheduled. The matrix already tells you what is open (count the `SILENT-GAP` rows). Future closures are scheduled by opening one OpenSpec change per closure (with its own GitHub issue at that point) — not by maintaining a backlog of issues created up-front.
166166

167167
**Alternatives considered:**
168168

169169
- *Embed Fix-now/Follow-up/Deferred columns in the matrix.* Rejected: blurs "current state" with "future plan"; matrix becomes a planning document and stops being a contract.
170-
- *Place the ledger in `docs/` so it survives archive.* Rejected: nobody updates `docs/<ledger>.md` after merge; it would drift silently. Issues with state machines and labels are the right tool for live tracking.
170+
- *Place the ledger in `docs/` so it survives archive.* Rejected: nobody updates `docs/<ledger>.md` after merge; it would drift silently.
171+
- *Open one GitHub issue per `Fix-now`/`Follow-up` entry at archive time.* Rejected: see "Why" above — parallel source of truth, preemptive scaffolding for hypothetical work.
171172

172173
### D5 — Pair the matrix with a `smali-dexlib2` 3.0.8 → 3.0.9 bump
173174

@@ -292,7 +293,7 @@ No runtime errors are introduced into the production code path — this change i
292293
- **[Risk] Matrix drift in sub-changes** — historically, ad-hoc fixes ship without updating docs. → **Mitigation**: `MatrixIntegrityTest` runs on every CI invocation and fails on every structural divergence; the PR-check GitHub Action additionally blocks PRs that modify production code in the relevant submodules without touching the matrix.
293294
- **[Risk] DemandCounter regex drift** — a designator whose regex is too loose (substring match) or too tight (missing form) corrupts the demand baseline. The original draft's 356/158 `get/set` count was exactly this failure mode. → **Mitigation**: each designator's `Pattern` is reviewed against a known-good sample from the corpus and quoted inline in the matrix; `testDemandCountsReproducible` runs at every CI invocation; the matrix row's `Demand` column is the visible value, the regex is the audit trail.
294295
- **[Risk] AspectJ Programming Guide is a moving reference** — new AspectJ versions could add pointcut designators. → **Mitigation**: the matrix anchors to the AspectJ Programming Guide §"Pointcuts" + AspectJ 5 quick reference at a specific URL with a snapshot date in its header; bumping the reference is a new sub-change with explicit matrix amendment.
295-
- **[Risk] Ledger goes stale post-archive**`openspec/changes/gh62-.../ledger.md` is archived after merge. → **Mitigation**: live tracking moves to GitHub issues labelled `gh62` (task 7.4); the ledger is explicitly treated as a snapshot, not an evolving document.
296+
- **[Risk] Ledger goes stale post-archive**`openspec/changes/gh62-.../ledger.md` is archived after merge. → **Mitigation**: the ledger is explicitly treated as a one-shot snapshot, not an evolving document; the matrix at `docs/aspectj_grammar_coverage.md` is the live backlog (`SILENT-GAP` rows = outstanding work). Future closures are scheduled by opening one OpenSpec change per closure when the work starts, not by maintaining a parallel issue tracker.
296297
- **[Trade-off] No code changes ship in gh62** — the value is realised only when sub-changes consume the ledger and close gaps. → **Mitigation**: gh62 itself can be merged independently of any closure; the ledger's `Fix-now` bucket schedules the first follow-on sub-changes immediately.
297298
- **[Trade-off] `grammar-tests/` is a new Maven module** — every developer running `mvn package` now builds an extra module. → **Mitigation**: test-only module, small incremental build cost; excluded from the shaded `instr-cli.jar` so the production artefact is unaffected.
298299
- **[Risk] `smali-dexlib2` 3.0.8 → 3.0.9 bump regression** — no announced breaking changes, but the reactor build is not a behavioural oracle. → **Mitigation**: §0.3a re-instruments 5 APKs from the INV-INS-31 baseline pre/post-bump and `dexdump`-diffs the output. Any non-trivial divergence reverts the property change inside gh62.

rv-android/openspec/changes/gh62-aspectj-grammar-coverage/specs/instrumentation/spec.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ For every row in `docs/aspectj_grammar_coverage.md`, the module SHALL contain ex
9898

9999
### Requirement: Scope Ledger for Future Closures
100100

101-
The change directory `openspec/changes/gh62-aspectj-grammar-coverage/` SHALL contain a `ledger.md` document that classifies every `SILENT-GAP` matrix row into exactly one of three buckets. Because the change directory is archived after merge, the ledger SHALL be treated as a snapshot of the bucket assignments at archive time; ongoing tracking moves to GitHub issues (one issue per Fix-now and Follow-up entry, labelled `gh62`) opened by task 7.4. The archived `ledger.md` remains discoverable via `git log --follow` and the issue tracker is the live source of truth for closure progress.
101+
The change directory `openspec/changes/gh62-aspectj-grammar-coverage/` SHALL contain a `ledger.md` document that classifies every `SILENT-GAP` matrix row into exactly one of three buckets. The ledger is a **snapshot** of bucket assignments at archive time — it is not maintained as a live document after the change is archived. The live source of truth for outstanding work is `docs/aspectj_grammar_coverage.md` itself: any row with `Verdict = SILENT-GAP` is by definition open work. Future closures are scheduled by opening one OpenSpec change per closure (with its own GitHub issue), not by maintaining a parallel issue list.
102102

103103
- **Fix-now** — closures recommended for scheduling against the current milestone, with rationale (active demand or otherwise high-value). Each entry names: AspectJ syntax / matrix row(s) it flips, demand summary, planned sub-change identifier (`gh-XX-<kebab>`), `Owner: @user`, `Target milestone: vX.Y`.
104104
- **Follow-up** — real work but no current demand to schedule. Each entry names matrix rows + a one-sentence rationale for deferral + `Owner` + `Target milestone: TBD`.
@@ -116,9 +116,9 @@ The ledger SHALL NOT contain implementation detail for the planned closures —
116116

117117
#### Scenario: opening a sub-change consumes a Fix-now entry
118118

119-
- **WHEN** a developer opens a sub-change (e.g. `gh-XX`) listed in the `Fix-now` bucket
119+
- **WHEN** a developer opens a sub-change (e.g. `gh-XX`) implementing a closure
120120
- **THEN** the sub-change's `proposal.md` SHALL cite gh62 issue #62 and the specific matrix rows it intends to flip
121-
- **AND** upon the sub-change's archive, the linked GitHub issue (opened by task 7.4) SHALL be closed with a reference to the sub-change's PR
121+
- **AND** upon archive of the sub-change, the matrix rows SHALL be flipped from `SILENT-GAP` to `COVERED` and the corresponding `@Disabled` annotations removed in the same commit (closure atomicity enforced by the PR-check GitHub Action)
122122

123123
## Invariants
124124

rv-android/openspec/changes/gh62-aspectj-grammar-coverage/tasks.md

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,12 +148,11 @@
148148

149149
- [ ] 7.1 Validate the openspec change: `openspec validate --changes gh62-aspectj-grammar-coverage --strict`. SHALL return PASS.
150150
- [ ] 7.2 Invoke `/rv-code-reviewer` via the Skill tool against the gh62 diff (matrix, ledger, `grammar-tests/` module, PR-check workflow). Address review findings inline.
151-
- [ ] 7.3 Update `MEMORY.md` with a `project_gh62_grammar_coverage` entry capturing the row count (per category), the SILENT-GAP count by corpus, the `Fix-now` sub-change list, and the corrected `get/set` demand baseline (zero across all corpora).
152-
- [ ] 7.4 Open the GitHub sub-issues for each `Fix-now` AND `Follow-up` ledger entry (one issue per planned sub-change identifier or `Follow-up` group), labelled `gh62`. Each issue references `#62` and names the matrix rows it will flip. This is the live tracking surface post-archive — the ledger.md snapshot is frozen at archive time.
153-
- [ ] 7.5 Run `/opsx:verify` against the change.
154-
- [ ] 7.6 Run `/opsx:archive` (`openspec archive gh62-aspectj-grammar-coverage --yes`). Delta spec for `instrumentation` SHALL auto-merge.
155-
- [ ] 7.7 Commit on `origin/modules`: `chore(gh62): archive change + index sub-issues (closes #62)`. Push.
156-
- [ ] 7.8 Close issue #62 via `gh issue close 62 --repo PAMunb/rvsec --comment "..."` referencing the matrix, the ledger snapshot, the `grammar-tests/` module, the PR-check workflow, and the opened sub-issues.
151+
- [ ] 7.3 Update `MEMORY.md` with a `project_gh62_grammar_coverage` entry capturing the row count (per category), the SILENT-GAP count by corpus, the initial Fix-now bucket, and the corrected `get/set` demand baseline (zero across all corpora).
152+
- [ ] 7.4 Run `/opsx:verify` against the change.
153+
- [ ] 7.5 Run `/opsx:archive` (`openspec archive gh62-aspectj-grammar-coverage --yes`). Delta spec for `instrumentation` SHALL auto-merge.
154+
- [ ] 7.6 Commit on `origin/modules`: `chore(gh62): archive change (closes #62)`. Push.
155+
- [ ] 7.7 Close issue #62 via `gh issue close 62 --repo PAMunb/rvsec --comment "..."` referencing the matrix, the ledger snapshot, the `grammar-tests/` module, and the PR-check workflow. Future closures open their own issues and OpenSpec changes when scheduled — the matrix's `SILENT-GAP` rows are the live backlog; no preemptive issue creation.
157156

158157
## 8. Out-of-scope cross-cutting checks
159158

0 commit comments

Comments
 (0)