feat(feedback): add structured feedback reporting#31
Conversation
Add privacy-aware /feedback submissions with redacted session context, GitHub fallback handling, and worker support for structured payloads. Also keep the welcome/update refinements and Dependabot config changes currently present on the branch.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a structured feedback submission flow with redaction and async backend submission (with GitHub fallback), integrates a welcome-banner update/what's-new chip, expands tests for both features, and tightens Dependabot configuration. ChangesFeedback and Update Notification System
🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/feedback.py`:
- Around line 148-154: The try/except around Path.home() is too broad and
silently swallows errors; replace the bare except with a narrow catch for the
specific exceptions that Path.home() can raise (e.g., RuntimeError and OSError)
and log the failure instead of passing silently — update the block that calls
Path.home() (and modifies the redacted variable) to except (RuntimeError,
OSError) as e: and call the module/logger (e.g., logger.warning or
processLogger) with a short message including the exception before continuing to
return redacted.
In `@src/pythinker_code/ui/shell/update.py`:
- Around line 387-391: The _read_last_seen_version function currently catches
all OSError and returns None, which hides real failures; change the exception
handling so that FileNotFoundError (or PermissionError due to a missing file)
returns None but other OSError cases are not silently swallowed—either re-raise
them or log and re-raise. Specifically, update the try/except around
LAST_SEEN_VERSION_FILE.read_text(...) so it catches FileNotFoundError and
returns None, and lets other OSError exceptions propagate (or log via the module
logger before raising) to preserve actionable errors.
In `@tests/ui_and_conv/test_shell_feedback_slash.py`:
- Around line 94-150: The test uses SimpleNamespace(...) as the mocked return
for submit_mock in test_submits_structured_payload; replace that with an actual
FeedbackSubmission instance to make the test robust to type changes: import
FeedbackSubmission from pythinker_code.feedback (or the module where it’s
defined), create FeedbackSubmission(number=42, html_url="https://issue/42") for
the AsyncMock return (the submit_mock used via
monkeypatch.setattr("pythinker_code.feedback.submit_feedback_payload",
submit_mock)), and keep the rest of the assertions the same so
submit_mock.assert_awaited_once() and payload inspection continue to work.
In `@tests/ui_and_conv/test_shell_update.py`:
- Around line 1010-1020: The test only checks the baseline file is non-empty;
instead assert it contains the actual recorded version so a wrong value can't
slip through: after calling update.consume_whats_new() in
test_consume_whats_new_baseline_on_first_launch, read
last_seen_file.read_text(...).strip() and assert it equals the expected version
exported by the module (e.g. update.__version__ or the module's canonical
version symbol), referencing update.consume_whats_new and
update.LAST_SEEN_VERSION_FILE to locate the logic to validate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 05bb6b68-e18a-474f-ac54-f34d9ee3ea32
📒 Files selected for processing (9)
.github/dependabot.ymlexamples/feedback-worker/src/index.tssrc/pythinker_code/feedback.pysrc/pythinker_code/ui/shell/__init__.pysrc/pythinker_code/ui/shell/slash.pysrc/pythinker_code/ui/shell/update.pytests/ui_and_conv/test_shell_feedback_slash.pytests/ui_and_conv/test_shell_update.pytests/ui_and_conv/test_shell_welcome_info.py
- redact_text: narrow blind `except Exception` to (RuntimeError, OSError) and log at debug instead of silently swallowing home-resolution failures - _read_last_seen_version: return None for missing file but log real OSError (e.g. permission) instead of masking it - test_shell_feedback_slash: use real FeedbackSubmission dataclass instead of duck-typed SimpleNamespace for both submit mocks - test_consume_whats_new_baseline: assert the baseline equals the current VERSION rather than merely non-empty
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
/feedbacksupport with feedback types, redacted session context, opt-in transcript/diff/tool details, and default send confirmationVerification
make check-pythinker-codemake test-pythinker-codeuv run pytest tests/ui_and_conv/test_shell_feedback_slash.py -qcd examples/feedback-worker && npm exec --package typescript@5.9.3 -- tsc --noEmit --strict --target ES2022 --module ESNext --moduleResolution Bundler src/index.tsgit diff --checkgraphify update .Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
Chores