docs: reconcile demote-path surface removed by #814 / #820 (#822) - #824
Conversation
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Important Review skippedIgnore keyword(s) in the title. ⛔ Ignored keywords (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
|
[claim:review:bagheera:2026-05-14T21:45:11Z] |
|
[claim:review:feynman:2026-05-14T21:45:13Z] |
|
[release:review:feynman:2026-05-14T21:45:18Z] |
|
[claim:review:prince:2026-05-14T21:47:09Z] |
|
[release:review:prince:2026-05-14T21:47:13Z] |
robotrocketscience
left a comment
There was a problem hiding this comment.
Independent review — substantive coverage good; CHANGELOG conflict needs rebase
Four atomic commits, 38 additions / 56 deletions across 8 files. Net negative on a removal-sweep PR is the right shape.
1. Acceptance grep — passes
git grep -nE 'pressured_locks|demoted_locks|--pressured|demotion_pressure|_pressure_and_maybe_demote|propagate=(False|True)' github/docs/issue-822-demote-path-reconciliation -- 'docs/' 'CHANGELOG.md' returns four hits, all in the allowed scope:
CHANGELOG.md:19— the new[Unreleased] / Removedentry (intended).CHANGELOG.md:872, 873, 934, 936— historical pre-v3.0 release-section entries describing the original#20/#5/#7shipped mechanism. Issue acceptance criterion explicitly permits "locked release sections."
Zero remaining hits under docs/ — every doc reference to the removed surface has been swept.
2. User-facing surface drops — correct
Spot-checked the four user-facing doc changes against the removed code surface:
docs/COMMANDS.md:20—locked [--pressured]→locked. Matches what's incli.pypost-#820 (no--pressuredargparser entry).docs/COMMANDS.md:28—feedbackrow's "contradiction → demotion_pressure ≥ 5 ⇒ auto-demote" sentence replaced with a clean removal note + a pointer to PHILOSOPHY #605. Honest.docs/COMMANDS.md:109— defaults table drops the "Demotion threshold: 5 contradicting events" row. Correct.docs/MCP.md:63(aelf:locked) — dropspressuredarg; opportunistically documentslimit, offset, response_formatwhich were already implemented but undocumented. Bonus accuracy fix.docs/MCP.md:68-69(aelf:feedback/aelf:confirmreturn shapes) — dropspressured_locksanddemoted_locksfrom the return-dict shape. Matches the newFeedbackResultpost-#820.docs/MCP.md:86(worked example) —pressured_locks: []anddemoted_locks: []stripped from the example confirm return.docs/MCP.md:179, 220(class diagrams) —tool_locked/aelf_lockedsignatures droppressured. Consistent.docs/SLASH_COMMANDS.md:17— drops--pressuredrow column. Matches CLI.
3. Hibernation reconciliation — issue acceptance criterion is misframed, but the PR's choice is sound
Acceptance criterion #2 in #822 says: "docs/hibernation_lifecycle.md reconciled with current hibernation logic (hibernate_eligible in store.py)." But hibernate_eligible does not exist in store.py on github/main — and neither does list_hibernated (referenced in the PR's TODO comment). The only hibernation surface in production code is the hibernation_score: REAL column (added v2.0 #196). Nothing writes to it: git grep 'hibernation_score\s*=' src/ returns only the row→model load at store.py:661.
So docs/hibernation_lifecycle.md is a design document for an unimplemented feature, not API documentation. The acceptance criterion was written against a function that doesn't exist.
The PR's actual change is correct given that reality: it drops the demotion_pressure >= 1 predicate from the trigger description (the design no longer plans to key off that signal post-#820) and rewrites the "Why this and not the others" bullet to describe harmful-feedback pressure rather than demotion_pressure. The SQL block at line 49 was left with a TODO comment pointing at list_hibernated (a function that doesn't exist) — recommend retargeting that TODO at the actual surface, e.g., <!-- TODO(#822 follow-up): example was sketched against demotion_pressure; rewrite once hibernation eligibility is implemented (only schema column exists today) -->. Non-blocking; the substance of the predicate-drop is correct.
4. Internal-surface drops (ARCHITECTURE, hook-injection-audit, promotion_path) — correct
docs/ARCHITECTURE.md— drops the "Contradicting positive feedback accumulatesdemotion_pressure; ≥5 ⇒ auto-demote" line from rule #5; dropsdemotion_pressurefrom theBeliefrecord schema list; rewrites step 2 of the feedback flow.docs/hook-injection-audit.md— drops thepropagate=Falsereference at lines 60-61.docs/promotion_path.md— substantial trim (36 deletions): drops thedemotion_pressurerow from the schema columns table, drops theaelf locked --pressuredreference, drops the dead link tofeedback._pressure_and_maybe_demote, and drops the locked-belief auto-demote section at line ~563. All correct.
5. CHANGELOG.md — merge conflict with just-merged PR #810
git merge-tree --merge-base=$(git merge-base github/main github/docs/issue-822-demote-path-reconciliation) github/main github/docs/issue-822-demote-path-reconciliation flags one conflict file: CHANGELOG.md. The other 7 files merge clean.
Root cause: PR #810 (merged 21:43:35Z, FF to 4eb7df4f) added a [Unreleased] / Fixed entry for #809 immediately above the line where this PR adds a [Unreleased] / Removed entry. Both edits land in the same [Unreleased] section.
Fix is mechanical: rebase docs/issue-822-demote-path-reconciliation onto 4eb7df4f, resolve the CHANGELOG conflict by keeping both the ### Fixed (from #810) and the new ### Removed section, push, re-add ready-to-merge. The merge-train will refuse the current state because mergeStateStatus is DIRTY/CONFLICTING.
6. Discretion
Diff vs github/main clean — no public-boundary issues.
Net
Substantive coverage is correct. Only blocker is the CHANGELOG merge conflict introduced by PR #810 landing first. Rebase + re-label and merge-train will ship it.
|
[release:review:bagheera:2026-05-14T21:47:48Z] |
|
[claim:review:bagheera:2026-05-14T21:50:08Z] |
|
[claim:review:prince:2026-05-14T21:50:49Z] |
|
[release:review:prince:2026-05-14T21:50:54Z] |
robotrocketscience
left a comment
There was a problem hiding this comment.
Content-correct, content-verified, blocked on rebase.
Verification matches PR body claim. Ran the body's grep against branch HEAD:
docs/: zero hitsCHANGELOG.md: 5 hits — line 19 (the new[Unreleased] / Removedentry, intentional past-tense) and lines 872 / 873 / 934 / 936 (historicalv0.4.0/v0.1.0release sections, correctly untouched)
Cross-checked doc removals against github/main source:
src/aelfrice/cli.py— no--pressuredarg,_cmd_lockedcarries nopressuredparameter.COMMANDS.mdrow matches.src/aelfrice/mcp_server.py—tool_locked(store, *, limit, offset, response_format)(line 416) andaelf_locked(limit, offset, response_format)(line 1264). The PR'sMCP.mdrow + both class-diagram entries now match these signatures exactly.src/aelfrice/store.py— only survivingdemotion_pressurereference is theALTER TABLE ... DROP COLUMNmigration at line 577, which is expected and not user-facing.
The four atomic commits factor cleanly. Commit 4 (CHANGELOG ### Removed) is information-dense but lists all four breaking surfaces with the specific exception each raises — fine.
Blocker 1 — CHANGELOG conflict (mergeStateStatus: DIRTY). github/main gained an ### Fixed entry for #809 (subfloor noise gate) under [Unreleased] after this branch forked. The two sections are independent (### Fixed vs ### Removed), so the conflict is trivial — sit ### Removed underneath ### Fixed on rebase.
Blocker 2 — operator decision on the <!-- TODO(#822 follow-up) --> HTML comment in hibernation_lifecycle.md. The PR body flags this and explicitly defers to the operator: "TBD pending whether the operator considers the TODO acceptable for this PR or wants the followup issue captured." This isn't a content problem — it's an unresolved decision. Either path is defensible:
- Accept the HTML comment as a marker, ship the PR as-is content-wise, no follow-up issue. The TODO is honest about scope.
- File a follow-up issue capturing the
hibernation_lifecycle.md§ Candidate A SQL example rewrite against the currenthibernation_score-basedstore.py::list_hibernatedcontract. The TODO comment then carries the issue number.
Pinging operator: which?
No code concerns; no discretion-grep concerns. Diff is doc-only and contains nothing that would trip the public/private boundary.
Next steps for author:
git fetch github main && git rebase github/main(CHANGELOG section reorder, nothing more).- Once operator answers the TODO question above, either add the issue number to the HTML comment or leave as-is and proceed.
- After rebase, re-request review; the
ready-to-mergelabel flow becomes viable once the conflict clears.
|
[release:review:bagheera:2026-05-14T21:52:17Z] |
91a93dc to
cb885eb
Compare
Operator decision — hibernation TODOOperator picked option C on the Follow-up filed as #825 (scopes the dispose-of / rewrite / implement Author action on rebaseIn addition to the mechanical CHANGELOG → Dropping |
cb885eb to
6c1c609
Compare
Per operator decision on PR #824 (option C): #825 is the issue that scopes the dispose-of / rewrite / implement question for the aspirational hibernation surface in docs/design/hibernation_lifecycle.md. Update the TODO comment to cite #825 directly and capture the design-doc-for-unimplemented-feature framing.
|
Rebased onto current Conflicts:
Operator decision applied (commit Verification:
Per |
|
[claim:review:clarke:2026-05-14T23:16:40Z] |
Approve-equivalent — content correct; needs rebase (caught next merge-train cycle)All 5 commits signed ( Both prior decisions addressed:
Verification grep on the rebased tip: Returns 4 historical hits in CHANGELOG.md (v0.4.0 / v0.1.0 locked release sections — correctly untouched) + 1 new hit in the [Unreleased] / Removed entry (intentional past-tense reference). Zero hits under Blocker: branch NOT-FF on current main. PR #826 (ζ rerank, 6 commits) merged in the gap. Swapping |
|
[release:review:clarke:2026-05-14T23:17:38Z] |
f6f021a to
965a7da
Compare
Per operator decision on PR #824 (option C): #825 is the issue that scopes the dispose-of / rewrite / implement question for the aspirational hibernation surface in docs/design/hibernation_lifecycle.md. Update the TODO comment to cite #825 directly and capture the design-doc-for-unimplemented-feature framing.
|
Rebased onto current
Labeled |
Remove stale references to the demote-path surface removed by #814/#820: - COMMANDS.md: drop `--pressured` from `aelf locked` row; rewrite `feedback` row to note contradiction-driven demotion was removed under #814 with pointer to PHILOSOPHY #605; remove stale "Demotion threshold" row from the Defaults table. - SLASH_COMMANDS.md: drop `optional --pressured` from `/aelf:locked` row. - MCP.md: remove `pressured_locks` / `demoted_locks` from the `aelf:feedback` and `aelf:confirm` Returns columns and from the example JSON block; align `aelf:locked` optional-params column with the actual paginated signature (limit/offset/response_format); update the class diagram stubs to match. Closes the two consumer-facing contract gaps in #822: MCP clients coding to the old payload shape would have hit KeyError on every feedback/confirm call; CLI users passing `--pressured` get an unrecognised-arg error since #820.
Remove stale references to the auto-demote machinery removed by #814: - ARCHITECTURE.md: rewrite principle §5 to note lock correction is an explicit user act per PHILOSOPHY #605, not auto-demote; drop `demotion_pressure` from the Belief field list; strip the CONTRADICTS-walk paragraph from the Bayesian-update step list and remove the stale "Walk is 1-hop only" note; remove the "Drives demotion-pressure + auto-demote" clause from the feedback.py module row; clean up the "No pressure walk" phrase from the negative-valence step. - hook-injection-audit.md: remove the stale `propagate=False` kwarg citation — `record_retrieval` calls `apply_feedback` positionally with no `propagate` argument since #820.
Remove demotion_pressure dependencies from both docs: hibernation_lifecycle.md: - Recommendation summary: drop the `demotion_pressure >= 1` trigger clause; the column no longer exists (#814). - Candidate A SQL example: remove `AND demotion_pressure >= 1` and prefix the block with a TODO comment noting the example needs rewrite against current hibernation_score-based criteria. The three remaining predicates (unlocked, cold, low-posterior) are correct but the conjunction is now looser than the original spec intended — a follow-up is flagged rather than invented here. - Rationale bullet: reword "Pure demotion_pressure" to "Pure negative-feedback" since the column is gone. promotion_path.md: - Drop `demotion_pressure` from the v1.0.x schema field table (§1). - Drop the `aelf locked --pressured`-style listing TBD (§1). - Drop dead link to `feedback._pressure_and_maybe_demote` from the one-tier-per-demote-call paragraph (§5). - Drop the "TBD: softer auto-demote threshold" TBD bullet (§7) — the mechanism it references no longer exists. - Rewrite "Why not C" rationale to remove the pressure-walk argument (no longer applicable); the irreversibility argument stands. - Rewrite "Why not A" to remove the `DEMOTION_THRESHOLD=5` citation.
…ased Add ### Removed section to [Unreleased] covering the four breaking changes shipped in PR #820: apply_feedback(propagate=) kwarg, FeedbackResult.pressured_locks/.demoted_locks fields, aelf locked --pressured CLI flag, and pressured_locks/demoted_locks keys in the aelf:feedback/aelf:confirm MCP payloads. Cites the empirical rationale (zero CONTRADICTS edges against locked beliefs in production lifetime; 14 locks vs 63,861 positive feedback rows) and references locked PHILOSOPHY #605 as the design principle that motivated clean removal over a deprecation cycle.
Per operator decision on PR #824 (option C): #825 is the issue that scopes the dispose-of / rewrite / implement question for the aspirational hibernation surface in docs/design/hibernation_lifecycle.md. Update the TODO comment to cite #825 directly and capture the design-doc-for-unimplemented-feature framing.
965a7da to
4339951
Compare
|
merge-train: merged 4339951 → |
The hibernation_lifecycle memo describes a trigger / predicate-grammar / sweeper design that has never been implemented. Storage half (#196 / PR #282 — hibernation_score + activation_condition columns) sits unused on github/main; no scorer populates the score column, no list_hibernated / hibernate_eligible function exists, and the original trigger spec keyed off demotion_pressure which was removed by #814 / PR #820. Operator decision on #825 (option A): rewrite the doc to current reality rather than implement the eligibility surface (option B) or delete the file (option C). Changes: - Rewrite Status section with an UNIMPLEMENTED banner that names the empty src/ surface and points at the column-only state. - Replace the SQL block in Candidate A with a paragraph noting that the original demotion_pressure-based trigger no longer applies and no replacement signal has been chosen. - Update the Decision ask to mark ratification as deferred until an implementation pass is scheduled. - Drop the TODO(#825) marker that PR #824 left behind; this commit IS the disposition. Net effect: the memo no longer claims any behavior that exists in src/; remaining references to demotion_pressure are historical context explaining why each section is unimplemented, not claims about current behavior. Closes #825.
Summary
Closes #822. Reconciles documentation and CHANGELOG with the surface PR #820 (
fed650ca) removed fromsrc/but did not sweep fromdocs/. No code changes; pure docs cleanup +[Unreleased] / Removedentry.The seven doc files cited in #822 plus a few sibling lines surfaced during the sweep (the Defaults table in
COMMANDS.md, theaelf:lockedrow + class-diagram stubs inMCP.mdthat still showed thepressuredarg, and two stale rationale paragraphs inpromotion_path.md) are updated to matchgithub/mainas offed650ca.Verification
After this PR returns only:
CHANGELOG.mdline 19: the new[Unreleased] / Removedentry (intentional — references the removed surface in past-tense).CHANGELOG.mdlines 872, 873, 934, 936: historical release-note entries fromv0.4.0/v0.1.0. These are locked release sections and are not touched.Zero hits in
docs/.uv run pytest tests/ -x -q --ignore=tests/bench_gate -k "doc or changelog": 243 passed, 4 skipped.Commit shape (atomic / bisectable)
docs: drop user-facing surface refs (COMMANDS, SLASH_COMMANDS, MCP)— the consumer-facing contracts. Removes--pressuredflag references,pressured_locks/demoted_locksfrom MCP payload schema + example, and updates theaelf:lockedrow + both class-diagram tool signatures to reflect the actual current shape (limit, offset, response_format).docs: drop internal-surface refs (ARCHITECTURE, hook-injection-audit)—ARCHITECTURE.md§5 lock-correction now cites v3.0 PHILOSOPHY: natural-language-relatedness gate — deterministic vs embedding #605/refactor(feedback): remove unreachable CONTRADICTS-edge auto-demote path (per #605) #814 for the removed auto-demote semantics; field list dropsdemotion_pressure; positive-valence walk paragraph removed.hook-injection-audit.mddrops thepropagate=Falsekwarg citation.docs: reconcile hibernation_lifecycle + promotion_path—hibernation_lifecycle.mddrops thedemotion_pressure >= 1predicate from the Candidate A doctor-pass sketch (adds a<!-- TODO(#822 follow-up) -->flag on the SQL example since current hibernation logic ishibernation_score-based, beyond docs: reconcile demote-path surface removed by #814 (CHANGELOG + 7 doc files) #822's scope to fully rewrite).promotion_path.mddrops thedemotion_pressurefield-table row, the deadfeedback._pressure_and_maybe_demotelink, and rephrases two "Why not C / Why not A" rationale paragraphs that depended on the removed mechanism.docs(changelog): note #814 / #820 removed demote surface under Unreleased— single information-dense### Removedentry covering all four breaking changes (kwarg, dataclass fields, CLI flag, MCP payload keys) with consequence-per-surface (TypeError,AttributeError, unrecognised-arg error,KeyError), the empirical justification (14 locks vs 63,861 positive feedback events, zero CONTRADICTS edges against locked beliefs), and the locked PHILOSOPHY v3.0 PHILOSOPHY: natural-language-relatedness gate — deterministic vs embedding #605 framing.All four commits SSH-signed.
Special handling notes
docs/hibernation_lifecycle.md's SQL example at the Candidate A section now carries a<!-- TODO(#822 follow-up) -->HTML comment marking that the example predicates need a real rewrite against currenthibernation_score-based logic instore.py::list_hibernated. That rewrite is genuinely out-of-scope for a removal sweep — it requires understanding the current hibernation contract end-to-end, not just deleting one line. Filing a follow-up: TBD pending whether the operator considers the TODO acceptable for this PR or wants the followup issue captured.aelf:lockedMCP tool row's "Optional" column onMCP.mdline 62 was stale beyond justpressured(it also missedlimit,offset,response_formatthat the tool actually accepts) — corrected.Refs
fed650ca) — the refactor whose docs sweep this PR completes.c06f8d575fad71fb) — rationale.Closes #822.