sync: reapply 135 OP commits onto current upstream/main#38
Conversation
Add Go zone rules for generated file patterns (.generated., _gen.go, .pb.go, _string.go, _enumer.go) and config files (go.mod, go.sum). Improve --import-run error to explain what's missing and suggest next steps. Closes peteromallet#402, addresses peteromallet#401. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
test_reset_runtime_state_clears_registry_and_hooks called reset_runtime_state() without saving/restoring the registry, causing 5 downstream tests (erlang, ocaml, fsharp, javascript, bash) to fail when the language plugins couldn't re-register (already imported). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tterns
Per project policy ("no backward compat for import paths — remove
re-export facades, wrapper shims, compat layers"), delete all thin
wrapper files left behind by recent package reorganizations:
- 13 _framework/ wrappers (commands_base, generic, registry_state, etc.)
- 8 context_holistic/ wrappers (budget_*, selection_contexts)
- 2 helpers/ wrappers (runtime.py, persist.py)
Replace SimpleNamespace fake-module pattern in override_misc.py and
commit_log/dispatch.py with direct function calls.
Update 27 import sites and 4 test monkeypatch targets to use canonical
paths. All 5193 tests pass.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add suggestion and evidence fields to show command output and cluster member display so triage stages can investigate issues without JSON exports. Add investigation command hints to compact summaries (self_record mode only), forward observe assessments to sense-check, and update enrich/sense-check instructions to reference the new data paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…wers Add deferred/triaged_out to Status enum so state is always authoritative for issue disposition. Previously temporary and triaged_out skips left issue.status as "open", causing overcounting and misleading displays. Part A — Status unification: - Add DEFERRED, TRIAGED_OUT to Status enum and _CANONICAL_ISSUE_STATUSES - Update FAILURE_STATUSES_BY_MODE (all 3 scoring modes) - Map temporary skip → deferred, triaged_out skip → triaged_out in state - Backlog/unskip reopen deferred/triaged_out back to open - Triage dismiss sets state status to triaged_out - Reconcile migrates existing open+skipped → correct status on scan - Treat deferred/triaged_out as alive in reconcile (not superseded) - Add status icons (⏸ deferred, △ triaged_out) - Update plan header and summary_lines for new status buckets Part B — Surface history to reviewers: - Default --retrospective to True (--no-retrospective to opt out) - Rewrite render_historical_focus with status grouping and CLI hints - Add render_dimension_deferral_context for stale dimension warnings - Wire both into batch and external review prompt paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…acteristics Split holistic review into Phase 1 (observe: collect characteristics and defects) and Phase 2 (judge: synthesize dimension_character, then score). Positive observations now persist as context insights with positive: true and full provenance (added_at, source), replacing ephemeral strengths. judgment.strengths is backfilled from positive insights after import. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The anti-gaming safeguard for subjective dimensions is working as designed, but agents weren't discovering the blind-review workflow because: 1. The first penalty message said only "Re-review objectively" with no pointer to the blind packet or agent overlay docs 2. The blind packet hint only appeared after repeated penalties (streak >= 2) 3. SKILL.md's anti-gaming note didn't reference the overlay docs Now the first penalty immediately surfaces: - The blind packet path (.desloppify/review_packet_blind.json) - Pointers to docs/CLAUDE.md and docs/HERMES.md for the full workflow https://claude.ai/code/session_01RqpTiawULymfeVXW8X8ySq
The fork's history was reinitialized as a snapshot import that shares no git ancestor with peteromallet/desloppify. A plain merge would produce 1,500+ pseudo-conflicts on overlapping paths. Instead, this branch is built from upstream/main (3f40fbf, "chore: prepare 1.0 release") with OP's 121 touched files reapplied on top using the fork's initial snapshot (5937528) as the content-level merge base. Categorization: 26 OP-only adds (copied from origin/main) 28 OP-edits, upstream untouched (copied from origin/main) 4 upstream-deleted, OP kept (re-added from origin/main) 26 both-evolved merges (18 patched cleanly, 8 manually resolved) 1 OP+upstream both-added (took upstream, more comprehensive) 15 OP-deleted (PR #23 shims) (re-applied; rationale still holds) 21 OP-deleted+upstream-deleted (no-op) Notable resolutions, full details in REAPPLY_LOG.md: - Catalog + javascript/__init__.py: took OP wholesale to preserve the advocacy scoring dimensions / phase appends upstream had stripped. Those are OP's reason for forking. - attempts.py, transition_messages.py: took upstream's larger refactor, layered OP's nosec annotations on top. - .gitignore: upstream tail (dev/* rename), plus OP's .desloppify/ comment and .claude/ agent-state entries. - README.md: took OP wholesale (fork branding is OP-shape, preserve). - test_coverage.py: upstream's richer impl satisfies OP __init__.py's module reference; OP alias js_test_coverage_hooks retained. Excluded from this reapply (by design): - feature/dehallucination-gate tip (ff34082, external contributor). Anchored at archive/feature-dehallucination-gate-2026-05-14. Should go through its own PR post-reapply. - .claude/rules/*.md and .claude/skills/*.md removed by PR #30. Those moved to org-canonical structured-coding-with-ai; reapply preserves OP's removal. Recovery: archive/pre-sync-2026-05-14 (99b4442) is the rollback point.
Sync branch and main share no git ancestor (fork was reinitialized as a snapshot import, see PR body). This merge commit uses -s ours strategy: the tree comes entirely from the reapply commit, with origin/main added as a second parent purely to make the PR creatable. When this PR is merged, main becomes the merge commit's tree (= the reapply tree). The pre-sync history (99b4442 backwards) remains reachable via the archive/pre-sync-2026-05-14 tag for rollback.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (300)
📝 WalkthroughWalkthroughReplaces MIT with OSNL v0.2. Removes advocacy/persona-qa features. Adds OpenCode and Rovo Dev runners end-to-end (review and triage), with stage-runner override and recovery. Introduces per-language scan/status. Expands state/plan repair/reconcile, reflect token accounting, suppression fingerprints. Broad language plugin updates (Rust, JS/TS, R). Numerous docs/tests added/updated. ChangesRunners and Triage Pipeline (OpenCode + Rovo Dev)
Scan/Status by Language and Helpers
State/Plan lifecycle, reconcile, and recovery
Suppression, attestation, and CLI cleanups
Language plugin and tooling updates
Policy, registry, security, docs, and packaging
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (review/triage)
participant Orchestrator as Orchestrator
participant Runner as Runner (codex/opencode/rovodev)
participant FS as Filesystem
participant State as State/Plan
User->>CLI: desloppify review --run-batches --runner opencode
CLI->>Orchestrator: build deps (run_batch_fn)
Orchestrator->>Runner: run_batch(prompt, output_file, log_file)
Runner-->>FS: stream stdout, recover JSON payloads
Runner-->>FS: write log sections
Runner-->>Orchestrator: exit code
Orchestrator->>State: import results (trusted policy)
Orchestrator-->>User: summary and next-step hints
User->>CLI: desloppify plan triage --run-stages --runner rovodev
CLI->>Orchestrator: set stage_runner_override(rovodev)
Orchestrator->>Runner: run_triage_stage(prompt, files)
Runner-->>FS: write outputs/logs
Orchestrator->>State: persist stage dispositions
Orchestrator->>CLI: clear override
Orchestrator-->>User: stage result and follow-ups
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
The initial reapply filtered on files OP touched post-snapshot, which
missed files OP kept verbatim from the snapshot but that don't exist on
upstream. These have to be ported because they ARE fork-specific content
even though OP didn't modify them.
CI confirmed the gap:
- tests-core/tests-full: ModuleNotFoundError on phases_advocacy.py
(imported by javascript/__init__.py)
- ci-contracts: FileNotFoundError on docs/ci_plan.md
(test_ci_plan_required_checks_match_ci_workflow reads it)
Added:
- desloppify/languages/_framework/phases_advocacy.py
- desloppify/engine/detectors/advocacy_common.py
- desloppify/engine/detectors/advocacy_tool_presence.py
- desloppify/engine/detectors/frontend_detection.py
- desloppify/engine/detectors/advocacy_rules/*.yaml (8 files)
- desloppify/engine/detectors/advocacy_rules/context-rules/*.yaml (3 files)
- desloppify/app/commands/persona_qa/{__init__,browser_check,cmd,defaults,findings}.py
- .pre-commit-config.yaml, .semgrep.yml, .vale.ini
- docs/{ci_plan,QUEUE_LIFECYCLE,DEVELOPMENT_PHILOSOPHY,commit-summary-since-0.7.0,work-batches-since-0.7.0-ticket-digest}.md
- desloppify-fork-architecture.md, fork-verification-report.md
- integration-investigation.md, persona-qa-architecture.md
- website/* (landing page assets)
- dev/release/release-notes-drafts/v0.9.11.md
REAPPLY_LOG.md updated with the gap explanation.
|
Status update — gap-fix committed Initial CI run flagged 3 failures, all from the same root cause: my reapply filter (files OP touched post-snapshot) missed 41 files OP carries from the initial snapshot but never modified — including
One finding worth flagging while CI runs — OP main has Recommendation: keep upstream's OSNL since OP's fork sources from peteromallet/desloppify, the upstream changed it deliberately, and reverting to OP's older MIT version while taking the rest of upstream's evolution would mismatch the upstream's intent. But this is a fork-policy decision that's worth your call before merge. If you want OP to stay on MIT, say so and I'll restore. |
…d OP intent
CI from the previous round flagged failing tests in:
- test_transitive_modules + test_update_skill_cmd_direct (5 tests)
Root cause: upstream's update_skill/cmd.py removed _read_local_docs_file
- test_javascript_test_coverage (4 tests)
Root cause: API signature drift (set vs list return type)
- test_holistic_review + test_review_context_selection (5 tests)
Root cause: policy/core.py's DIMENSIONS list lost 6 advocacy entries
- test_bash_unused_imports (3 tests)
Root cause: analysis/unused_imports.py behavior changed
All four files are in the "OP-untouched-since-snapshot but upstream evolved
in a way that strips OP intent" category. The original reapply filter
(files OP touched post-snapshot) missed them.
Also restored OP versions of 6 more files in the same pattern, identified
by scanning every OP-untouched-but-different file for advocacy references
on OP side that upstream dropped:
- desloppify/app/commands/next/render_nudges.py (4 advocacy refs)
- desloppify/app/commands/show/scope.py (1)
- desloppify/base/registry/catalog_entries.py (11 — registry of advocacy detectors)
- desloppify/base/registry/catalog_models.py (3)
- desloppify/languages/_framework/review_data/dimensions.json (13)
- desloppify/languages/go/__init__.py (4)
These were silently losing OP's advocacy customization in the first pass.
…_coverage Upstream's tests/test_test_coverage.py imports BARREL_BASENAMES from javascript/test_coverage.py — a symbol upstream added in their evolved version of that module. The previous commit restored OP's test_coverage.py (simpler API, set-return semantics) because OP's own test suite tests OP's API. With OP's version canonical, upstream's BARREL_BASENAMES test is testing functionality that doesn't exist. Removing the upstream test rather than re-merging the two test_coverage.py files. Follow-up issue: if upstream's richer JS test-coverage impl is wanted later, unify the two modules properly + reintegrate this test.
scope.py (OP version, restored in previous commit) imports build_work_queue_for_visibility from desloppify.engine.work_queue. Upstream's work_queue.py doesn't export that function; OP's does. Without the restoration, 7 test_cmd_show tests fail at import-time. This is the same cascading-dependency pattern as the previous restore: when an OP-flavored module is restored, modules it depends on may also need to be restored if upstream stripped the called API.
|
Ready for sam-gate review — caveats below. Final CI state9 of 11 checks green:
What happened during CI iterationFour fix-up commits after the initial reapply, each driven by a CI failure:
The 5 remaining test failuresAll are "OP's frozen-snapshot test contract" vs "upstream's evolved behavior" mismatches. Not broken code — evolutionary drift between two unrelated histories.
Per your standing reapply plan: "Test failures are findings, not merge blockers — log them in REAPPLY_LOG.md and link follow-up issues." That's done. Each failure points at a specific OP-flavored module that upstream's evolution drifted from. What is and isn't readyReady:
Not ready (needs your call):
Open question on LICENSE: Currently OSNL v0.2 (upstream's evolution). My earlier comment surfaced this; awaiting your call. If you want MIT, say so and I'll restore. Standing by for your call to merge or request more iteration on specific failures. |
* fix: root-cause CI failures inherited from sync PR #38 merged with 5 failing tests. Each had a real root cause now fixed, verified locally before push. 1. tree-sitter-language-pack 1.6.3 breaks 3 bash detector tests 1.6.3 (released 2026-04-23) is a deliberate breaking refactor by upstream (kreuzberg-dev): the public module moved from `tree_sitter_language_pack` to `_native`, removing the top-level package that desloppify's parser bootstrap imports. ImportError gets caught by PARSE_INIT_ERRORS in analysis/unused_imports.py and returns empty findings, masking the regression as test assertion failures rather than import failures. Verified: on a clean Python 3.12 venv with 1.6.3, all 3 bash tests fail with `[] == ['helpers']` etc. Pin to 1.6.2 → all 5 bash tests pass. Fix: cap at <1.6.3 in pyproject.toml (treesitter + full extras) and update test_treesitter_language_pack_is_capped_below_incompatible_release to assert the new cap. 2. test_successful_dedicated_install_rovodev: missing _read_local_docs_file mock OP's update_skill/cmd.py prefers local docs/ files over the download path (sensible: local-first when running from a checkout). Test mocked only `_download`, so _read_local_docs_file('SKILL.md') and ('ROVODEV.md') returned real local content (the actual ROVODEV.md shipping with the package), bypassing the mock. The test class already had this mock pattern on test_download_failure and test_bad_content; the three "successful_install" tests were inconsistent. Added @patch on `_read_local_docs_file, return_value=None` to all three to consistently exercise the download path the tests intend to test. Verified all 25 tests in the file pass. 3. test_show_structural_loads_medium_confidence_matches: scope.py regression The sync's per-file restoration of OP versions reverted scope.py's load_matches to a stale variant that routes everything through build_work_queue, which applies standalone_threshold filtering (medium structural < high threshold → filtered). Upstream's evolved load_matches has an explicit "show should surface persisted matching issues even when a detector has a higher standalone confidence threshold" path that calls build_issue_items directly with forced_ids, bypassing the filter. OP's only difference in scope.py was a docstring example mentioning advocacy_language — not load-bearing code. Upstream's load_matches handles advocacy scopes identically. Fix: take upstream's scope.py + revert engine/work_queue.py to upstream (no longer needs build_work_queue_for_visibility export, which only OP's old scope.py used). Verified all 42 tests in test_cmd_show.py pass. Excluded from this PR: ~30 additional latent failures visible in Python 3.12 local runs that don't appear in CI on Python 3.11. Separate triage. * fix: skip bash_unused_imports tests when tree-sitter unavailable tests-core runs via `make tests` → `install-ci-tools` which installs the core package without [full] extras (no tree-sitter-language-pack). The new test_bash_unused_imports.py module (came in via the sync from upstream) lacked the skipif guard that every other test_treesitter module uses: pytestmark = pytest.mark.skipif( not is_available(), reason="tree-sitter-language-pack not installed", ) Without the guard, the parser-init catch in detect_unused_imports returns empty findings on missing parser, and the test assertions fail with "assert [] == ['helpers']" — exactly the symptom the previous commit's 1.6.3 pin fixed for tests-full but couldn't fix for tests-core (parser not installed at all there). Verified locally: tests still pass when tree-sitter is installed; will skip cleanly when it isn't. --------- Co-authored-by: Original Gary <276612211+OpenGaryBot@users.noreply.github.com>
Summary
Brings Open-Paws/desloppify in line with
peteromallet/desloppify@main(currently3f40fbfd, 918 commits). The fork's git history shares no common ancestor with upstream (the fork was reinitialized as a squashed snapshot import), so agit merge --allow-unrelated-historieswould produce 1,500+ pseudo-conflicts. This branch is instead built from upstream/main with OP's 121 touched files reapplied on top, using the fork's initial snapshot (5937528f) as the content-level merge base.Result: future
git fetch upstream && git merge upstream/mainbecomes trivial — the fork tracks upstream from this point forward.Branch shape
Two commits on this PR:
bd156337— the reapply. Parent is3f40fbfd(upstream/main HEAD). Contains the 97-file diff that brings the reapplied tree into existence.2105ca56— bridge merge (-s ours). Adds99b44426(fork main HEAD) as a second parent so GitHub can render the PR. Tree is unchanged — none of main's content was pulled in. This commit exists solely to give GitHub a structural relationship to compute the PR against.When this PR is merged,
mainbecomes the bridge commit's tree, which is the reapply tree. The pre-sync history remains reachable viaarchive/pre-sync-2026-05-14for rollback.What's in the reapply diff
97 files: 31 added, 51 modified, 15 deleted.
.claude/rules/*, advocacy detectors, persona_qa, CodeQL workflow, scorecard, …)origin/mainorigin/mainCLAUDE.md,advocacy_language.py,advocacy_security.py,persona_qa/profiles.py)origin/mainjavascript/test_coverage.py)__init__.pyuses by module ref only)REAPPLY_LOG.md is the full per-file paper trail with detailed rationale for each manually-resolved conflict and the collision.
Conflict resolutions worth a second look
Real semantic clashes where upstream and OP both moved on the same code with different intent:
desloppify/base/subjective_dimension_catalog.py— took OP. Upstream removed all six Open Paws advocacy dimensions; OP wanted them retained and rebranded one label. Honoring upstream's removal would gut the fork's reason for existing.desloppify/languages/javascript/__init__.py— took OP. Same shape: upstream stripped the advocacy phase imports + appends; OP kept them.README.md— took OP (fork branding is OP-shape, preserve)..gitignore— surgical merge: upstream'sreview/* → dev/*directory rename +/CLAUDE.md, plus OP's.desloppify/ tracked intentionallycomment and.claude/agent-memory//worktrees/excludes.attempts.py,transition_messages.py— took upstream's bigger refactor, layered OP's# nosec B404/# nosec B310 — localhost onlyannotations on top.agent_context.py,test_treesitter.py— took upstream wholesale (strict superset of OP intent in both cases).What's NOT in this PR (by design)
feature/dehallucination-gatebranch tip (ff34082d) by external contributor LarytheLord. Anchored atarchive/feature-dehallucination-gate-2026-05-14. Should get its own PR post-merge..claude/rules/{testing,security,privacy,…}.mdand.claude/skills/*.mdremoved by PR #30 (canonical-rule-set sync — those moved to org-widestructured-coding-with-ai). Reapply preserves OP's intentional removal.Recovery
If anything's wrong-shaped after merge, rollback is one push:
The tag
archive/pre-sync-2026-05-14points at the pre-syncmain(99b44426) and was created before any of this work began.Test plan
make ci-fast(lint + typecheck + arch + contracts + tests)desloppify scan --path .— confirm self-scan still produces a scoredesloppify scan --path desloppify/engine/detectors/advocacy_*desloppify persona-qa --helpTest failures introduced by the merge are findings, not merge blockers per the reapply plan — log them as follow-up issues against this PR.
Summary by CodeRabbit
New Features
--by-languageflag for mixed-language repositories.--runner rovodev).--runner opencode).--show-requirementsflag displays triage stage validation requirements.Bug Fixes
Documentation
License