feat(sentiment-hook): production wire-up of sentiment-feedback into UPS lane (closes #606) - #612
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✨ 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 |
Reviewer's GuideWires the existing v2 sentiment_feedback module into the UserPromptSubmit hook so that sentiment expressed in the current prompt is deterministically detected, applied as feedback to the prior UPS turn’s retrieved beliefs before retrieval runs again, and recorded via a new sentiment_feedback audit tag; implementation is fail-soft, opt-in via TOML/env config, and covered by focused unit/integration tests plus a design memo and changelog entry. Sequence diagram for UserPromptSubmit sentiment-feedback lane integrationsequenceDiagram
actor User
participant UserPromptSubmitHook as UserPromptSubmit_hook
participant SentimentFeedback as sentiment_feedback_module
participant HookAudit as hook_audit_jsonl
participant BeliefStore as belief_store
participant Retriever as retrieval_engine
User->>UserPromptSubmitHook: submit prompt_N (session_id)
activate UserPromptSubmitHook
UserPromptSubmitHook->>SentimentFeedback: apply_sentiment_feedback(prompt_N, session_id)
activate SentimentFeedback
SentimentFeedback->>SentimentFeedback: _load_aelfrice_toml()
SentimentFeedback-->>SentimentFeedback: config_mapping
SentimentFeedback->>SentimentFeedback: is_enabled(config_mapping)
alt sentiment_from_prose_disabled
SentimentFeedback-->>UserPromptSubmitHook: 0 updated_beliefs
else sentiment_from_prose_enabled
SentimentFeedback->>SentimentFeedback: detect_sentiment(prompt_N)
alt no_sentiment_signal
SentimentFeedback-->>UserPromptSubmitHook: 0 updated_beliefs
else sentiment_signal_detected
SentimentFeedback->>HookAudit: read_hook_audit(hook_audit_jsonl + rotated)
HookAudit-->>SentimentFeedback: most_recent_prior_ups_beliefs_ids
alt no_prior_ups_or_no_beliefs
SentimentFeedback-->>UserPromptSubmitHook: 0 updated_beliefs
else prior_belief_ids_found
SentimentFeedback->>BeliefStore: _open_store()
BeliefStore-->>SentimentFeedback: store_handle
SentimentFeedback->>BeliefStore: apply_sentiment_to_pending(store_handle, signal, prior_belief_ids)
BeliefStore-->>SentimentFeedback: results_per_belief
SentimentFeedback->>HookAudit: _write_sentiment_feedback_audit(signal, applied_belief_ids)
HookAudit-->>SentimentFeedback: append_jsonl_row
SentimentFeedback-->>BeliefStore: close_store()
SentimentFeedback-->>UserPromptSubmitHook: n_applied_beliefs
end
end
end
deactivate SentimentFeedback
UserPromptSubmitHook->>Retriever: _retrieve(prompt_N, budget)
activate Retriever
Retriever-->>UserPromptSubmitHook: hits_reflecting_updated_posteriors
deactivate Retriever
UserPromptSubmitHook-->>User: assistant_response_based_on_hits
deactivate UserPromptSubmitHook
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The sentiment lane is now on the hot UPS path and re-parses
.aelfrice.tomlon every prompt; consider caching the parsed config and/orsentiment_feedback.is_enabledresult to avoid repeated filesystem and TOML work when the feature is disabled or unchanged. - _load_prior_ups_belief_ids scans all records in the current and rotated audit files on each sentiment fire; if these JSONL files grow toward the rotation cap this becomes O(N) per corrective prompt, so it may be worth optimizing (e.g., scanning from the end, keeping a lightweight index, or stopping early once you pass a session’s last match).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The sentiment lane is now on the hot UPS path and re-parses `.aelfrice.toml` on every prompt; consider caching the parsed config and/or `sentiment_feedback.is_enabled` result to avoid repeated filesystem and TOML work when the feature is disabled or unchanged.
- _load_prior_ups_belief_ids scans all records in the current and rotated audit files on each sentiment fire; if these JSONL files grow toward the rotation cap this becomes O(N) per corrective prompt, so it may be worth optimizing (e.g., scanning from the end, keeping a lightweight index, or stopping early once you pass a session’s last match).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| from aelfrice.hook import ( | ||
| AUDIT_FILENAME, | ||
| AUDIT_HOOK_SENTIMENT_FEEDBACK, | ||
| AUDIT_HOOK_USER_PROMPT_SUBMIT, | ||
| _audit_path_for_db, | ||
| _load_aelfrice_toml, | ||
| _load_prior_ups_belief_ids, | ||
| apply_sentiment_feedback, | ||
| read_hook_audit, | ||
| user_prompt_submit, | ||
| ) |
|
[claim:review:noether:2026-05-10T23:50:00Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
|
Substance LGTM — flagging Things I checked:
After rebase + green CI, this is FF-mergeable. Re-flag |
|
[release:review:noether:2026-05-10T23:51:40Z] |
d90863f to
bff7dea
Compare
|
[claim:review:Maxwell:2026-05-11T01:46:25Z] |
|
LGTM. Implements the locked v3.0 sentiment-hook decision exactly:
State checked:
Notes (none blocking):
Branch is ready for |
|
[release:review:Maxwell:2026-05-11T01:47:23Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
Specs hook lane (UserPromptSubmit), retrieval window (prior UPS audit row for same session), audit tag (sentiment_feedback), single opt-in flag, and privacy posture re: existing transcript-ingest opt-out. Maps to #606 AC 1-5.
Adds `apply_sentiment_feedback(prompt, session_id)` and the two helpers behind it: `_load_aelfrice_toml` (full-document walker for modules with their own `is_enabled(config)` surface) and `_load_prior_ups_belief_ids` (projects the most-recent prior UPS audit row's beliefs[*].id list). Wired into `user_prompt_submit` before retrieval so the bumped posteriors are reflected in this turn's hits. New constant `AUDIT_HOOK_SENTIMENT_FEEDBACK = "sentiment_feedback"` and a per-fire audit row carrying pattern + matched_text + valence + applied belief_ids. Default off; opt-in via `[feedback] sentiment_from_prose = true` in `.aelfrice.toml` (existing `sentiment_feedback.is_enabled` resolution). Fail-soft on every error path — the lane never raises into the hook.
Covers _load_aelfrice_toml, _load_prior_ups_belief_ids, and apply_sentiment_feedback (disabled gate, no-signal short-circuit, no-prior-UPS short-circuit, posterior demotion + audit-row write, missing-belief skip). Adds integration tests against the UPS hook entry-point: - disabled lane leaves posteriors and audit log untouched - enabled lane bumps prior-turn belief beta on a correction prompt - AC4 two-session fixture: correction in session A demotes the wrong belief, session B retrieval ranks the uncorrected sibling above it - feedback_history row carries the sentiment_inferred source tag
bff7dea to
07d0463
Compare
|
Rebased on
Per the locked v3.0 sentiment-hook decision (#606, ratified 2026-05-10): UPS lane, default-off opt-in, most-recent-window decay — this PR matches. Note: when this gets — leibniz |
|
[claim:review:noether:2026-05-11T04:48:38Z] |
|
Ratifying. Post-rebase state matches the locked v3.0 sentiment-hook decision (#606): UPS lane, default-off opt-in, most-recent-window decay, stdlib-only determinism, audit tag on existing surface. Verified on rebased HEAD (07d0463):
Per leibniz's note re #618, taking the FF-push path rather than the labeled merge-train. — noether |
|
[release:review:noether:2026-05-11T04:50:05Z] |
Closes #606.
What this does
Wires the v2.0
sentiment_feedbackmodule into a liveUserPromptSubmithook lane. Before each turn's retrieval, the lane reads the most-recent prior UPS audit row for the samesession_id, projects itsbeliefs[*].id, and — ifdetect_sentimentmatches the prompt — applies the signal (positive / negative, base / strong, escalated) to those beliefs viaapply_sentiment_to_pending. Bumped posteriors are reflected in this turn's retrieval, so a "no, that's wrong" prompt demotes the offending belief before the next block is built.How the decisions came out
Captured in
docs/v3_sentiment_feedback_hook.md. Summary:UserPromptSubmit. The corrective prompt arrives after the assistant has acted on the prior retrieval block; applying it at the next UPS keeps the bump in lockstep with the retrieval window it's correcting.session_id. Single-session for v3.0; cross-session propagation is explicit follow-up.sentiment_feedbackwritten to the existinghook_audit.jsonl, carryingpattern,matched_text,valence,belief_ids. The per-belieffeedback_historyrows continue to carrysource = sentiment_inferred(unchanged from v2.0).[feedback] sentiment_from_prose = truein.aelfrice.toml(or the existing env var). Default off — no behavior change for users who haven't opted in.Commits
docs(sentiment-hook): v3.0 spec for #606 production wire-up— decision memo.feat(sentiment-hook): wire detect_sentiment into UPS lane—apply_sentiment_feedback+ two helpers (_load_aelfrice_toml,_load_prior_ups_belief_ids) + new audit tag + wiring intouser_prompt_submitbefore retrieval.test(sentiment-hook): unit + two-session bench fixture— 16 tests covering the disabled gate, no-signal short-circuit, no-prior-UPS short-circuit, posterior demotion, audit-row schema, missing-belief skip, UPS-integration paths, and the AC4 two-session ranking fixture.docs(changelog): unreleased entry.#606 acceptance mapping
docs/v3_sentiment_feedback_hook.md_disable_sentimentpath verifiedtest_correction_lowers_subsequent_ranking_across_sessionstest_apply_demotes_prior_turn_beliefs_and_writes_audit+ tagsentiment_feedbackOut of scope (kept per #606)
Tests
uv run pytest tests/ -q --ignore=tests/bench_gate— 3278 passed, 30 skipped.The 16 new tests are in
tests/test_hook_sentiment_feedback.py.Summary by Sourcery
Wire the existing sentiment feedback module into the UserPromptSubmit hook so corrective user prompts can adjust prior belief posteriors before retrieval, with opt-in gating, auditing, and accompanying documentation and tests.
New Features:
Enhancements:
Tests: