Skip to content

feat(scope): scope:demote + --batch + demote_meta audit instrumentation (#1121)#1196

Open
MScottAdams wants to merge 1 commit into
masterfrom
orchestrator/d1-scope-demote
Open

feat(scope): scope:demote + --batch + demote_meta audit instrumentation (#1121)#1196
MScottAdams wants to merge 1 commit into
masterfrom
orchestrator/d1-scope-demote

Conversation

@MScottAdams
Copy link
Copy Markdown
Collaborator

Summary

D1 (#1121) — Wave-1 child of umbrella #1119. Introduces task scope:demote, complementing the existing seven scope:* lifecycle verbs (#324), plus a structured audit trail every demote appends to.

What ships

  • task scope:demote verb — single-file and --batch --older-than-days <N> modes (default 45 per Current Shape Decision 4; the deep-pass §A1 bump from the original 30).
  • scripts/scope_demote.py — single + batch demote engine. Moves a vBRIEF from vbrief/pending/ back to vbrief/proposed/, flips plan.status to proposed, refreshes plan.updated.
  • scripts/scope_audit_log.py — operator-private append-only writer for vbrief/.eval/scope-lifecycle.jsonl with the same cross-process + in-process locking pattern as scripts/candidates_log.py (feat(triage,cache,refinement): pre-ingest triage workflow with sidecar issue cache (refs #583 #788 #789 #815 #522) #845 Story 2). Sidecar scope-lifecycle.jsonl.lock keeps the advisory msvcrt/fcntl lock orthogonal to the data file.
  • demote_meta block on every audit entry (Current Shape Decision 1, all five fields frozen):
    • was_promoted (bool — true when source was pending/)
    • original_promotion_decision_id (UUID of a prior promote audit entry on the same path, null when not traceable — forward-compat hook so future emitters populate the field automatically)
    • days_in_pending (non-negative int derived from plan.updated with a file-mtime fallback)
    • demote_reason (free-text operator-supplied for single mode, batch:older-than-days:<N> for batch)
    • demoted_from (source folder name)
  • .gitignore + vbrief/.eval/README.mdscope-lifecycle.jsonl is operator-private alongside candidates.jsonl / summary-history.jsonl; covered by the existing vbrief/.eval/*.jsonl merge=union rule (vbrief/.eval/ directory governance — gitignored vs committed decision (N4 of #1119) #1144).
  • tests/cli/test_scope_demote.py — 19 tests across five sections (single, batch, demote_meta, CLI, audit-log validation).

Current Shape excerpt (comment 4471271992 on #1121)

  1. demote_meta instrumentation block KEPT (5 fields above).
  2. 30% threshold / D17 falsification gate DROPPED — no automated threshold; operator-driven workflow only. Ad-hoc jq over the captured demote_meta data covers the analytical surface.
  3. Lightweight metrics replacement filed as Lightweight triage metrics — replacement for closed D17 (#1135) (D17-replacement of #1119) #1180 (task triage:audit --since=<window> --format=json over the audit log; no thresholds, no gates).
  4. --older-than-days default for --batch = 45 (unchanged from deep-pass §A1's bump from 30).

Acceptance criteria (all met)

  • task scope:demote <path> moves a pending/ vBRIEF to proposed/, flips plan.status, refreshes plan.updated.
  • task scope:demote -- --batch --older-than-days 45 demotes every pending/ vBRIEF whose plan.updated (or file mtime fallback) is older than 45 days; idle when none match.
  • Every demote appends one JSONL entry to vbrief/.eval/scope-lifecycle.jsonl with the full demote_meta block (5 fields).
  • 30%-threshold gate is NOT shipped; metrics over the log are deferred to Lightweight triage metrics — replacement for closed D17 (#1135) (D17-replacement of #1119) #1180.
  • task check is green on my touched surface (lint + 19 tests). One pre-existing infrastructure failure unrelated to this PR is documented below.

Pre-existing test failure (unrelated, NOT introduced here)

tests/cli/test_task_scripts.py::TestToolchainCheck::test_happy_path_all_tools_present fails on the runner because uv is installed as a Python module on the host rather than as a stand-alone uv.exe on PATH. The failure is environmental (the toolchain-check.py helper greps PATH for the binary) and reproduces on master without my changes. Filing a separate hygiene issue is out of scope for #1121.

Related Issues

Closes #1121
Refs #1119
Refs #1144 (vbrief/.eval/ tracking governance this rides on)
Refs #1180 (deferred metrics surface)

Checklist

  • /deft:change <name> — N/A; this is a Wave-1 child of meta: cache-as-operator-working-set + tracker-layer hygiene (supersedes #1097) #1119 dispatched via swarm Phase 0.
  • CHANGELOG.md[Unreleased] entry added under ### Added.
  • ROADMAP.md — N/A (rendered artifact; will refresh on next release cut).
  • Tests pass locally — 19 new tests + the existing tests/cli/test_scope_lifecycle.py suite (66 total in the scope-lifecycle surface) all green.

Post-Merge

  • Verify issue auto-close: after squash merge, confirm #1121 actually closed — gh issue view 1121 --json state --jq .state. Squash merges can silently fail to process closing keywords (PRs merged but issues not closed and roadmap not updated #167). If still open, close manually with a comment referencing this PR.
  • No branch-protection-related work in this PR.

"plan": {
"title": "feat(scope): scope:demote + --batch + demote_meta audit instrumentation (#1121)",
"status": "running",
"narratives": {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 (50% confidence) — The plan object in this vBRIEF document is missing the required id field. The vBRIEF v0.6 schema mandates plan.id, plan.title, and plan.status for all plan objects. This omission can lead to inconsistencies or parsing errors for tools relying on the full schema. [also at: vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json:9, vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json:8] [corroborated: deterministic proximity] [corroborated: cross-source collapse, deterministic vbrief-schema-check]

Suggestion:

Add a `"id"` field to the `plan` object. This typically matches the numeric ID portion of the filename, e.g., `"id": "1121"`.

entry: dict matching the schema documented in the module docstring.
log_path: optional override of the log file path. Production callers
MUST leave this as None and let the caller pass the canonical
path resolved via :func:`canonical_log_path` (this signature
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 (50% confidence) — The docstring for append states: 'Production callers MUST leave this as None and let the caller pass the canonical path resolved via :func:canonical_log_path'. However, the code explicitly raises a ScopeAuditLogError if log_path is None. This is a direct contradiction between the declared invariant in the docstring and the implementation. [corroborated: deterministic cross-file]

Suggestion:

Update the docstring to reflect that `log_path` is a mandatory argument that must be explicitly provided by callers, e.g., 'Production callers MUST pass the canonical path resolved via :func:`canonical_log_path`.'

Copy link
Copy Markdown

@deft-slizard deft-slizard Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Machine-readable verdict
{
  "slizard_verdict": {
    "schema_version": 1,
    "decision": "comment",
    "severity": {
      "P0": 0,
      "P1": 2,
      "P2": 0,
      "P3": 0
    },
    "confidence": 0.5444,
    "decision_confidence": 0.425,
    "finding_count": 2,
    "merge_impact": "non-blocking",
    "version": "slizard v0.3.451",
    "head_sha": "7033ae1e0af18ba61e2b84d2ad5a8c2c6208eb30"
  }
}
**SLizard Review** — 2 finding(s) across 2 file(s)

P1 · vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json:10 · confidence 0.50

The plan object in this vBRIEF document is missing the required id field. The vBRIEF v0.6 schema mandates plan.id, plan.title, and plan.status for all plan objects. This omission can lead to inconsistencies or parsing errors for tools relying on the full schema. [also at: vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json:9, vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json:8] [corroborated: deterministic proximity] [corroborated: cross-source collapse, deterministic vbrief-schema-check]

P1 · scripts/scope_audit_log.py:284-287 · confidence 0.50

The docstring for append states: 'Production callers MUST leave this as None and let the caller pass the canonical path resolved via :func:canonical_log_path'. However, the code explicitly raises a ScopeAuditLogError if log_path is None. This is a direct contradiction between the declared invariant in the docstring and the implementation. [corroborated: deterministic cross-file]

Review coverage

  • Reviewed files: .gitignore, CHANGELOG.md, scripts/scope_audit_log.py, scripts/scope_demote.py, tasks/scope.yml, tests/cli/test_scope_demote.py, vbrief/.eval/README.md, vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json
  • Skipped files: none
  • Source: github @ 7033ae1e0af1

Context used

  • Static findings: 2
  • Static tools: unused-export; vbrief-schema-check
  • Graph/blast radius: 0 changed node(s), 0 affected node(s)
  • Vector context chunks: 0
  • Context availability: full
  • Review categories: default
  • Deterministic checks:
    192/194 passed, 2 failed: unused-exports, vbrief-schemavbrief-traceability=passed, markdown-fences=passed, unused-exports=failed, query-docstring=passed, xss-sprintf=passed, markdown-xref=passed, aria-target=passed, semantic-role=passed, aria-containment=passed, redundant-assertion=passed, resource-lifecycle=passed, cross-diff-consistency=passed, redundant-css-block=passed, template-placeholder=passed, void-async-signal=passed, spread-override=passed, json-indent-consistency=passed, dep-swap=passed, powershell-scoping=passed, diff-truncation=passed, exception-type-contract=passed, dead-none-guard=passed, access-declaration=passed, unused-option=passed, css-property-interaction=passed, jsx-style-indent=passed, regex-breadth=passed, cartesian-fan-out=passed, exec-stdout-parse=passed, sentinel-error-wiring=passed, hardcoded-filemode=passed, api-response-shape=passed, python-cli-arg=passed, taskfile-namespace=passed, vbrief-schema=failed, go-shell-injection=passed, go-discarded-error=passed, go-silent-error-branch=passed, go-comment-log-contradiction=passed, go-unconditional-message=passed, inline-style-proliferation=passed, unnecessary-nonnull-assertion=passed, double-cast=passed, unguarded-await-cast=passed, ssrf-guard-completeness=passed, error-message-leak=passed, puppeteer-resource-cap=passed, trivial-argument=passed, unused-imports=passed, dead-store=passed, phantom-import=passed, fetch-timeout-guard=passed, log-level-expected-path=passed, unified-diff-construction=passed, sentinel-index-assertion=passed, unguarded-map-lookup=passed, unbounded-prompt-injection=passed, description-diff-consistency=passed, render-branch-symmetry=passed, go-gorm-unchained-error=passed, error-handling-loop-break=passed, sync-state-batching=passed, sync-revoke-object-url=passed, go-like-wildcard-injection=passed, go-basename-dedup-gap=passed, go-missing-seed-in-migrate=passed, go-write-then-read-unfiltered=passed, go-direct-db-access=passed, async-handler-try-catch=passed, chat-sdk-history=passed, dead-code-ternary=passed, go-context-background=passed, go-git-arg-order=passed, go-n-plus-one=passed, go-nested-transaction=passed, go-ref-injection=passed, go-scanner-error=passed, go-toctou-db=passed, go-unused-validated-field=passed, hardcoded-literal=passed, hardcoded-undefined-field=passed, python-exception=passed, python-path-construction=passed, python-variable-shadow=passed, sentinel-value=passed, sibling-constant=passed, url-interpolation=passed, jsx-guard-removal=passed, asymmetric-clamp=passed, optional-prop-guard=passed, server-lifecycle=passed, go-asymmetric-org-scope=passed, fetch-body-scope-omission=passed, regex-breadth-inconsistency=passed, typeof-object-array-guard=passed, test-production-divergence=passed, argument-axis-mismatch=passed, click-propagation-gap=passed, sanitization-gap=passed, response-shape-consistency=passed, useref-dead-store=passed, keydown-target-guard=passed, fetch-redirect-guard=passed, response-body-buffering=passed, svg-content-type=passed, conditional-mode-exclusion=passed, join-separator-inconsistency=passed, prompt-injection-guard=passed, asymmetric-guard=passed, unsafe-json-cast=passed, markdown-single-line-interpolation=passed, postmessage-origin-guard=passed, css-iframe-scope=passed, markdown-entry-completeness=passed, python-set-duplicate=passed, state-setter-symmetry=passed, changelog-sibling-truncation=passed, ternary-exhaustiveness=passed, exclusive-output-constraint=passed, nullish-fallback-regression=passed, shell-pipefail=passed, vbrief-acceptance-contradiction=passed, css-property-diff=passed, go-sync-call-labeled-background=passed, prompt-guardrail-conflict=passed, prompt-identity-duplication=passed, asymmetric-truncation=passed, claim-source-tracing=passed, go-duplicate-event-publish=passed, go-create-without-cleanup=passed, go-find-then-create-without-cleanup=passed, go-persist-without-validate=passed, ts-put-without-get=passed, go-fetch-id-without-liveness=passed, go-event-subscribe-without-publish=passed, go-log-aggregation-run-scoping=passed, falsy-string-fallback=passed, useeffect-unstable-prop-dep=passed, unused-state-read=passed, blob-mime-mismatch=passed, parseint-nan-guard=passed, shared-state-across-map=passed, context-menu-click-guard=passed, touch-action-ancestor=passed, usestate-innerwidth-matchmedia=passed, prefix-match-loop=passed, hardcoded-new-field=passed, enum-subset-completeness=passed, optional-guard-fallthrough=passed, html-template-token=passed, label-association=passed, empty-src-img=passed, jsx-prose-link-mismatch=passed, try-catch-scope=passed, viewport-meta-accessibility=passed, go-github-api-response-id=passed, go-github-api-pagination=passed, go-gorm-first-without-errrecordnotfound=passed, fetch-cache-consistency=passed, oauth-session-state-binding=passed, go-switch-exhaustiveness=passed, go-test-mutex-asymmetry=passed, nextjs-suspense-boundary=passed, nullable-nested-response=passed, nullish-coalesce-empty-url=passed, asymmetric-callback-state-reset=passed, go-docstring-contract-mismatch=passed, replace-dollar-pattern=passed, conditional-fallthrough-gap=passed, sanitization-completeness=passed, bare-selector-fallback=passed, sanitization-context-mismatch=passed, json-escape-completeness=passed, catch-block-guard-parity=passed, early-exit-guard-subset=passed, html-escape-context-collision=passed, cross-file-inline-duplication=passed, collection-gate-ordering=passed, regex-denylist-anchor-gap=passed, handler-validation-symmetry=passed, go-http-handler-body-persist-without-authz=passed, go-first-element-without-disambiguation=passed, ternary-wrapper-asymmetry=passed, promise-then-without-outer-catch=passed, effect-persist-hydration-race=passed, css-animation-ref=passed, idb-open-lifecycle=passed, interactive-role-mismatch=passed, array-duplicate-field=passed, mid-file-static-import=passed, changelog-section-deletion=passed, graph-incompleteness=passed, orphaned-module=passed, lockfile-version-suppression=passed
  • Embedding index:
    0/0 okattempted=0 succeeded=0 failed=0 pooled=0

Suggested verification

  • (agent) Independently inspect each SLizard finding against the referenced file, surrounding code, and linked context before accepting or dismissing it.
  • (static) Review 2 deterministic/static finding(s) included in the evidence set.
  • (test) Run the repository tests that exercise the changed files and any failing scenario described by P0/P1 findings.

Agent verification brief

  • PR/CR: deftai/directive#1196
  • Head SHA: 7033ae1e0af18ba61e2b84d2ad5a8c2c6208eb30
  • Decision: comment
  • Highest-risk claims: P1 vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json:10 Vbrief Document Changes (0.50); P1 scripts/scope_audit_log.py:284 Comment-Declared Invariant Verification (0.50)
  • Reviewed files: .gitignore, CHANGELOG.md, scripts/scope_audit_log.py, scripts/scope_demote.py, tasks/scope.yml, tests/cli/test_scope_demote.py, vbrief/.eval/README.md, vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json
  • Skipped files: none
  • Evidence sources: static analysis, call graph/blast radius, deterministic checks
  • Known blind spots: none recorded

Decision: comment
Merge impact: non-blocking
Review confidence: 0.42
Decision confidence: 0.42
Finding confidence: 0.50
Reason: Findings are advisory under the current severity/confidence policy.
Severity counts: P0: 0, P1: 2, P2: 0, P3: 0
Important files: vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json (P1, 1 finding(s), confidence 0.50); scripts/scope_audit_log.py (P1, 1 finding(s), confidence 0.50)
Diversity warning: All 2 findings landed at P1/0.50 — output may be pipeline-homogenised.
Review scope: 8 files, 1467 additions, 3 deletions

slizard v0.3.451

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 17, 2026

Greptile Summary

This PR ships task scope:demote (D1 of Wave-1 #1119), completing the eighth scope:* lifecycle verb. It adds scripts/scope_demote.py (single + batch engine), scripts/scope_audit_log.py (append-only JSONL writer with cross-process locking mirroring candidates_log.py), and 19 tests covering all documented acceptance criteria.

  • demote_one validates source folder, refreshes plan.status/plan.updated, moves the file atomically, then appends a structured audit entry with the five-field demote_meta block (including original_promotion_decision_id traceability via a pre-move audit-log lookup).
  • batch_demote walks vbrief/pending/ in sorted order, applies the --older-than-days threshold (inclusive, default 45), and is a clean no-op when no files qualify.
  • .gitignore, vbrief/.eval/README.md, and tasks/scope.yml are updated consistently with the existing candidates.jsonl / summary-history.jsonl governance pattern.

Confidence Score: 5/5

Safe to merge; the demote engine, audit writer, and tests are well-structured with no correctness defects on the changed paths.

The two core scripts introduce no logic errors: source-folder validation, plan mutation, file move, and audit-log append are sequenced correctly, and batch_demote handles all edge cases without aborting the run. The cross-process locking faithfully mirrors candidates_log.py. The test suite pins every acceptance criterion with deterministic fixtures. The one new finding (utc_now_iso being an unused public symbol) has no runtime impact.

No files require special attention beyond the minor utc_now_iso cleanup in scripts/scope_audit_log.py.

Important Files Changed

Filename Overview
scripts/scope_audit_log.py New append-only audit log writer for scope-lifecycle JSONL; cross-process locking mirrors candidates_log.py. One dead public function (utc_now_iso) exported but never called or imported by this PR.
scripts/scope_demote.py New single + batch demote engine with audit entry construction. was_promoted is always True (previous thread) and --older-than-days semantics are inclusive (previous thread); no new logic bugs found.
tests/cli/test_scope_demote.py 19 tests covering single demote, batch demote, demote_meta schema, CLI modes, and audit-log validation guards; fixture usage is clean and deterministic.
tasks/scope.yml Adds demote task wiring consistent with existing scope verbs; passes --project-root and sets PYTHONUTF8.
.gitignore Adds vbrief/.eval/scope-lifecycle.jsonl to the operator-private gitignore list.
CHANGELOG.md Adds detailed Unreleased entry for scope:demote with design decisions and issue references.
vbrief/.eval/README.md Adds scope-lifecycle.jsonl row to the tracking-policy table and updates the gitignore enumeration sentence.
vbrief/active/2026-05-17-1121-d1-scope-demote.vbrief.json Active vBRIEF tracking the D1 work item; status running, references correct issue URIs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI["scope_demote.py CLI"] --> MODE{Mode?}
    MODE -- "--batch" --> BATCH_VALIDATE["Validate --older-than-days >= 0"]
    MODE -- "file" --> SINGLE_RESOLVE["Resolve file path"]
    BATCH_VALIDATE --> WALK["Walk vbrief/pending/*.vbrief.json"]
    WALK --> AGE_CHECK{"days_in_pending >= threshold?"}
    AGE_CHECK -- "No" --> SKIP["Add to skipped list"]
    AGE_CHECK -- "Yes" --> DEMOTE_ONE
    SINGLE_RESOLVE --> DEMOTE_ONE
    DEMOTE_ONE["demote_one()"] --> GUARD{"File in pending/?"}
    GUARD -- "No" --> REJECT["Return False + error"]
    GUARD -- "Yes" --> READ_JSON["Read + parse vbrief JSON"]
    READ_JSON --> CALC_DAYS["_days_in_pending()"]
    CALC_DAYS --> LOOKUP["latest_for_path() prior promote lookup"]
    LOOKUP --> WRITE_PLAN["Update plan.status + plan.updated"]
    WRITE_PLAN --> MOVE["Move pending/ to proposed/"]
    MOVE --> BUILD_ENTRY["Build audit entry + demote_meta"]
    BUILD_ENTRY --> AUDIT["scope_audit_log.append() + fsync"]
    AUDIT --> RETURN_OK["Return True + entry"]
    SKIP --> SUMMARY["Print tally"]
    RETURN_OK --> SUMMARY
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
scripts/scope_audit_log.py:118-120
`utc_now_iso` is exported at module level but is never called within this module, never imported by `scope_demote.py`, and not listed in the module docstring's public surface. It duplicates logic that `scope_demote.py` already inlines (`current_now.strftime(...)`). If it's intended as a public helper for future emitters, add it to the module docstring's `Public surface:` block; otherwise prefix it with `_` to avoid silently growing the public API.

```suggestion
def _utc_now_iso() -> str:
    """Return the current UTC time as an ISO-8601 string with literal Z."""
    return datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%SZ")
```

Reviews (3): Last reviewed commit: "feat(scope): scope:demote + --batch + de..." | Re-trigger Greptile

Comment thread scripts/scope_audit_log.py
@MScottAdams MScottAdams force-pushed the orchestrator/d1-scope-demote branch from 7033ae1 to f273890 Compare May 17, 2026 22:19
…on (#1121)

- New task scope:demote verb (single + --batch --older-than-days N, default 45)
- New scripts/scope_audit_log.py: append-only writer for vbrief/.eval/scope-lifecycle.jsonl
- New scripts/scope_demote.py: demote engine + CLI with demote_meta instrumentation
- demote_meta block: was_promoted, original_promotion_decision_id, days_in_pending, demote_reason, demoted_from
- .gitignore + vbrief/.eval/README.md: operator-private scope-lifecycle.jsonl entry
- tests/cli/test_scope_demote.py: 19 tests covering all acceptance criteria
- 30%-threshold gate from D17 dropped (Current Shape Decision 2)
- Metrics over the log deferred to #1180 (Current Shape Decision 3)

Refs #1119
Closes #1121
@MScottAdams MScottAdams force-pushed the orchestrator/d1-scope-demote branch from f273890 to 4d26c6e Compare May 17, 2026 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Operator can demote a pending/ vBRIEF back to proposed/ in one command or in bulk (D1+D9 of #1119)

1 participant