Skip to content

fix: preserve queue and triage scopes - #686

Open
ApexWorm wants to merge 14 commits into
peteromallet:mainfrom
ApexWorm:fix/synthetic-queue-gating
Open

ApexWorm wants to merge 14 commits into
peteromallet:mainfrom
ApexWorm:fix/synthetic-queue-gating

Conversation

@ApexWorm

@ApexWorm ApexWorm commented Aug 1, 2026 •

Copy link
Copy Markdown

Problems

  1. A queue containing only synthetic subjective, strategy, workflow, or triage IDs could hide live objective backlog from desloppify next.
  2. An explicitly frozen empty triage scope, including one exhausted by protected review holds, fell back to every historical manual cluster and demanded unrelated enrichment.
  3. Enrich and sense-check later converted that explicit empty scope to unscoped validation, reintroducing those historic clusters.
  4. The manual strategize stage advertised --attestation as a queue-guard override, but dropped it before ensure_triage_started, making the documented override impossible.

Fixes

  • Treat a synthetic-only queue as lacking substantive planned work so real objective backlog remains executable.
  • Retain an explicit empty active triage scope rather than treating it as an unfrozen legacy flow.
  • Preserve that scope through enrich and sense-check validation, confirmation, and runner paths; an explicit empty scope now excludes historical clusters.
  • Forward --attestation from the manual strategize stage to ensure_triage_started, matching observe, runner, and manual-start paths.

Verification

  • python3 -m pytest desloppify/tests/plan/test_reconcile_pipeline.py -q: 42 passed.
  • python3 -m pytest desloppify/tests/commands/plan/test_triage_split_modules_direct.py -q -k "active_triage_scope or enrich_quality_empty_triage_scope": 3 passed.
  • python3 -m pytest desloppify/tests/commands/plan/test_triage_runner.py -q -k "enrich or sense_check": 14 passed.
  • python3 -m pytest desloppify/tests/commands/plan/test_triage_stage_prompts_flow_direct.py -q: 5 passed.
  • python3 -m pytest -q desloppify/tests/commands/plan/test_strategist.py: 6 passed.
  • python3 -m ruff check the seven changed triage files: passed.
  • Reproduced against the affected MonoRepo: next surfaced live objective work, organize/enrich/sense-check completed, protected review holds remained untouched, and strategize now accepts the documented attested override.

Known unrelated baseline failures: test_lifecycle_ensure_triage_started_uses_plan_aware_backlog_for_workflow_only_queue already fails on the unmodified parent commit because the current backlog guard returns blocked. Ruff reports pre-existing import layout and unused-import findings in strategize.py and test_strategist.py.

@ApexWorm ApexWorm changed the title fix: keep synthetic queue entries from hiding objectives fix: preserve queue and triage scopes Aug 1, 2026
citizenadam added a commit to citizenadam/desloppify that referenced this pull request Sep 3, 2026
@awdemos

awdemos commented Sep 12, 2026

Copy link
Copy Markdown

This is not a random grab-bag — the theme is coherent and the tests are thorough — but as submitted it's three PRs in one and needs splitting before review can be effective:

  1. The 4 stated scope fixes (synthetic-only queue no longer hides objective backlog via executable_objective_ids; explicit empty triage scope preserved; scope threaded through enrich/sense-check validation; strategize --attestation forwarding) — all tested and passing.
  2. A large, mostly undocumented "protected review issue ids" feature — new 148-line module engine/_plan/triage/protection.py threaded through ~30 call sites (resolve/skip/reopen guards, triage apply/dismiss, snapshots, stale policy, sync, auto-cluster, work-queue, review-import preservation). Coherent theme, but the body only alludes to it; this deserves its own review.
  3. Grab-bag extras not in the body: codex_batch exit-0/output-validation retry, runner blocked→CommandError + shlex.quote, queue-guard threshold change, execution_results missing-selected-dimensions gate, organize-policy queued-members fix, plus heavy isort churn (~⅓ of the diff).

Also verified: introduces 3 new test failures (full suite: 5832 passed / 4 failed vs main's 5801/1):

  • test_lifecycle_ensure_triage_started_uses_plan_aware_backlog_for_workflow_only_queue — synthetic-only queue makes objective issues "executable" → triage auto-start blocks demanding --attestation. The body's claim that this "already fails on the unmodified parent commit" is false — it passes on main.
  • test_queue_snapshot_enforces_phase_boundaries — executable_objective_ids flips lifecycle phase scan→execute.
  • test_triaged_review_findings_stay_postflight_while_objective_work_remains — the _review_issue_items predicate change leaks a triaged review finding ahead of objective work.

Cross-PR notes: this exactly contains #684 (identical test blob) — that one can close as superseded; it has a trivial conflict with #685 in reflect.py imports; and it breaks #692's new test (test_override_resolve_cmd_resolves_state_backed_strategy_item monkeypatches append_log_entry/save_plan on resolve_cmd, which this PR deletes) — #692 will need a rebase regardless of merge order. Textually conflicts with #722 in 7 triage files.

Recommend: split (protection / scope fixes / grab-bag), fix or consciously update the 3 failing tests, and correct the body.

@daBOB

daBOB commented Sep 24, 2026

Copy link
Copy Markdown

Confirmed the queue-guard fixes here against a real project's saved plan/config/state, using disposable local copies only. With a configured strict target of 85 and a stale assessment at 87, next excluded that dimension but plan resolve <front-cluster> --confirm rejected the reordered cluster with subjective::mid_level_elegance as the current next item. The guard's queue options still default to a threshold of 100 unless explicitly supplied.

The supported workaround was plan reorder <cluster> top, then plan resolve <exact-live-issue-id> --note '...' --confirm. Cluster expansion adds the cluster name, preventing the existing exact-ID persisted-prefix allowance from applying. With threshold propagation and equivalent live-member prefix matching, resolving the same cluster changed exactly its one intended issue from open to fixed.

Independent regression checks used the real execution queue and next-selection path: configured targets 85/90, one- and two-member front clusters, fixed/missing members, and rejection of later/noncontiguous members. Three cases failed before the patch; all 20 queue-guard tests passed after. Ruff and diff checks passed. Full core suite: 5,814 passed, 5 skipped, 2 pre-existing failures in TestCmdReviewPrepare::test_do_run_batches_dry_run_generates_packet_and_prompts (normal and integration collection; missing “Previously flagged issues” prompt text). Both failures also reproduce with the original queue guard restored.

No original project state or scores were edited, and no project state/source data is attached. Keeping this evidence here instead of opening a duplicate PR.

This branch has not been deployed

No deployments
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.

3 participants