feat(review_hog): default urgency threshold to consider - #71443
Conversation
Switch the default ReviewHog urgency threshold from `should_fix` to `consider`
("All issues"), so out of the box every validated finding reaches the pull
request. Consider-level findings survive validation too, so they are worth
surfacing by default.
Changed in one place each: the `ReviewUserSettings` field `default`/`db_default`
and `DEFAULT_URGENCY_THRESHOLD`; migration 0017 alters the column default and
resets existing rows. Slider copy moves the "default" annotation to "All issues",
frontend fallbacks now fall back to `consider`, and the temporal input dataclass
defaults follow so "defaults match the model's" stays true.
Generated-By: PostHog Code
Task-Id: a33e404d-813b-43f1-b560-5810f21db8dc
|
Hey @sortafreel! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
posthog-app/_parent/products/tracing/frontend/TracingScene.js |
98.2 KiB | 🟢 -11.2 KiB (-10.3%) |
posthog-app/_parent/products/stamphog/frontend/scenes/StamphogScene/StamphogScene.js |
9.9 KiB | 🔺 +9.9 KiB (new) |
render-query/src/render-query/render-query.js |
21.31 MiB | 🔺 +5.9 KiB (+0.0%) |
posthog-app/src/scenes/experiments/Experiment.js |
224.1 KiB | 🔺 +1.7 KiB (+0.8%) |
posthog-app/_parent/products/review_hog/frontend/CodeReviewScene.js |
48.9 KiB | 🔺 +1.2 KiB (+2.5%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.22 MiB · 22 files | 🔺 +714 B (+0.1%) | ███░░░░░░░ 28.4% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.14 MiB · 2,981 files | 🔺 +6.6 KiB (+0.1%) | █████████░ 87.9% of 9.25 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.3 KiB | ../node_modules/.pnpm/posthog-js@1.402.3/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 235.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 222.9 KiB | ../node_modules/.pnpm/posthog-js@1.402.3/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.8 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 92.7 KiB | ../packages/quill/packages/quill/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Dist folder size — 🔺 +3.72 MiB (+0.3%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1316.41 MiB · 🔺 +3.72 MiB (+0.3%)
⚠️ Playwright — 1 failed
🎭 Playwright report · View test results →
❌ 1 failed test:
- See recordings action (chromium)
These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!
⚠️ Django migration SQL — 2 new migrations to review
We've detected new migrations on this PR. Review the SQL output for each migration:
products/review_hog/backend/migrations/0017_alter_reviewusersettings_urgency_threshold.py
BEGIN;
--
-- Alter field urgency_threshold on reviewusersettings
--
ALTER TABLE "review_hog_reviewusersettings" ALTER COLUMN "urgency_threshold" SET DEFAULT 'consider';
COMMIT;products/review_hog/backend/migrations/0018_backfill_urgency_threshold_to_consider.py
BEGIN;
--
-- Raw SQL operation
--
-- migration-analyzer: safe reason=review_hog_reviewusersettings is a tiny per-user settings table (staff-only alpha, no persisted rows yet)
UPDATE review_hog_reviewusersettings SET urgency_threshold = 'consider' WHERE urgency_threshold <> 'consider';
COMMIT;Last updated: 2026-07-16 14:21 UTC (98a79e9)
✅ Django migration risk — migration analysis complete
We've analyzed your migrations for potential risks.
Summary: 0 Safe | 2 Needs Review | 0 Blocked
⚠️ Needs Review
May have performance impact
review_hog.0017_alter_reviewusersettings_urgency_threshold
└─ #1 ⚠️ AlterField
Field alteration may cause table locks or data loss (check if changing type or constraints)
model: reviewusersettings, field: urgency_threshold, field_type: CharField
review_hog.0018_backfill_urgency_threshold_to_consider
└─ #1 ⚠️ RunSQL
RunSQL with UPDATE/DELETE - developer override applied for small table
override_reason: review_hog_reviewusersettings is a tiny per-user settings table (staff-only alpha, no persisted rows yet)
📚 How to Deploy These Changes Safely
RunSQL:
✅ Developer override applied:
Justification: review_hog_reviewusersettings is a tiny per-user settings table (staff-only alpha, no persisted rows yet)
Reviewer checklist:
- Verify table is actually small (<1000 rows typical)
- Confirm justification is valid
- Check no indexes will cause lock contention
- Ensure WHERE clause limits scope appropriately
If this override is incorrect, request batching:
- Batch size: 1,000-10,000 rows per batch
- Add pauses between batches
- Use WHERE clauses to limit scope
- Consider background jobs for very large updates (millions of rows)
See the migration safety guide
Last updated: 2026-07-16 14:21 UTC (98a79e9)
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
|
Reviews (1): Last reviewed commit: "feat(review_hog): default urgency thresh..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR updates ReviewHog’s default publishing strictness so consider-level (validated) findings are surfaced by default, aligning backend defaults, workflow payload defaults, UI fallbacks, and user-facing copy.
Changes:
- Change the default
urgency_thresholdfromshould_fixtoconsideracross model/constant, Temporal dataclasses, and frontend fallbacks. - Add a migration to change the DB default and update any existing
ReviewUserSettingsrows toconsider. - Update copy/docs and adjust tests/fixtures to reflect the new default.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| products/review_hog/frontend/reviewHogSettingsLogic.ts | Frontend fallback threshold now defaults to consider when settings are missing. |
| products/review_hog/frontend/generated/api.zod.ts | Updates generated OpenAPI-derived Zod schema descriptions to reflect new default. |
| products/review_hog/frontend/generated/api.schemas.ts | Updates generated OpenAPI-derived TS types docstrings to reflect new default. |
| products/review_hog/frontend/CodeReviewScene.tsx | Slider stop copy updated; UI fallback now defaults to consider. |
| products/review_hog/backend/tests/test_settings_api.py | Test expectation updated for new default setting row value. |
| products/review_hog/backend/tests/test_constants.py | Constant/default assertion updated to consider. |
| products/review_hog/backend/tests/test_acting_user.py | Default “no row” resolve behavior expectations updated to consider. |
| products/review_hog/backend/temporal/activities.py | Temporal input defaults updated to consider to match model default. |
| products/review_hog/backend/reviewer/tests/test_publish_review.py | Fixture rename to _SHOULD_FIX_PUBLISHED; references updated. |
| products/review_hog/backend/reviewer/tests/test_prepare_validation_markdown.py | Fixture rename to _SHOULD_FIX_PUBLISHED; references updated. |
| products/review_hog/backend/reviewer/prompts/issue_validation/prompt.jinja | Prompt copy updated to avoid claiming consider is always suppressed. |
| products/review_hog/backend/reviewer/constants.py | DEFAULT_URGENCY_THRESHOLD updated to CONSIDER. |
| products/review_hog/backend/models.py | ReviewUserSettings.urgency_threshold default/db_default updated to consider. |
| products/review_hog/backend/migrations/0017_alter_reviewusersettings_urgency_threshold.py | Alters column default and updates existing rows to consider. |
| products/review_hog/backend/management/commands/publish_review.py | Comment updated to reflect new default used by the command. |
| products/review_hog/backend/api/settings.py | Serializer help_text updated to mark consider as default. |
| products/review_hog/ARCHITECTURE.md | Architecture notes updated to document the default flip and related plumbing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🦔 ReviewHog reviewed this pull requestFound no issues worth raising, so no review was posted. |
|
✅ Visual changes approved by @sortafreel — baseline updated in 2 changed. |
2 updated Run: a2944137-cb7e-46d4-8cfb-1caa8387638c Co-authored-by: sortafreel <354488+sortafreel@users.noreply.github.com>
…github.com:PostHog/posthog into posthog-code/review-hog-default-consider-threshold
|
Note 🤖 stamphog reviewed Gates denied this PR (deny-listed migration files, classified T2-never/cross-cutting). It's a data migration resetting existing users' explicit urgency_threshold preferences to a new default, and Greptile flagged this as a real risk (overwriting explicit choices) — the author's claim that no real users exist yet is unverifiable from the diff alone.
Gate mechanics and policy version
|
Twixes
left a comment
Twixes
left a comment
There was a problem hiding this comment.
A little odd that just changing just the default is this many changes, but they do seem relevant ✅
Co-authored-by: posthog[bot] <206114724+posthog[bot]@users.noreply.github.com> Co-authored-by: sortafreel <354488+sortafreel@users.noreply.github.com>
Problem
ReviewHog's urgency threshold defaulted to "Should fix", which drops every
consider-level finding before it reaches the pull request. Butconsiderfindings pass the same validation pass as everything else, so they are real,
worth-surfacing issues. We want the out-of-the-box experience to show all of
them.
Changes
Flip the default
urgency_thresholdfromshould_fixtoconsider("Allissues"). New users now publish every validated finding by default; they can
still move the slider stricter whenever they want.
The change lands in one place per layer:
ReviewUserSettings.urgency_thresholddefault/db_defaultand
DEFAULT_URGENCY_THRESHOLDboth move toconsider.consider(safe, the feature has no persisted rows yet, done for correctness).(
ResolveActingUserResult/BuildBodyInput/PublishInput) follow the modelso "defaults match the model's" stays true.
rewords "Should fix", and the two
?? 'should_fix'fallbacks now fall back toconsider.help_textand the validator prompt no longerclaim
consideris always suppressed (whether it surfaces now depends on thereader's threshold).
The
should_fix-threshold test fixtures were renamed_DEFAULT_PUBLISHED→_SHOULD_FIX_PUBLISHED, since they exercise the publish gate at that threshold andnever meant "the product default".
No screenshot: I could not stand up the app in this environment. The only visual
change is the slider's helper text and which stop reads as the default.
How did you test this code?
I could not run the Django test suite (no Django install) or the frontend
typecheck/Jest (no
node_modules) in this environment, so I did not execute them.I ran
ruff checkandruff format --checkon the touched Python (clean) andbyte-compiled every changed module. I updated the affected assertions to the new
default rather than adding tests, since the behavior these lock in already had
coverage:
test_settings_api/test_acting_user: the auto-created row and the no-rowresolve fallback now expect
consider.test_constants:DEFAULT_URGENCY_THRESHOLDnow mirrorsCONSIDER.test_publish_review/test_prepare_validation_markdown: unchanged behavior,fixtures renamed only.
Please run the review_hog
backend:testscope andhogli build:openapibeforemerge. I hand-synced the two generated type description strings to match the new
help_textbecause codegen was not runnable here.Automatic notifications
Docs update
No user-facing docs reference this default (checked
docs/); the product doc isproducts/review_hog/ARCHITECTURE.md, updated in this PR.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
I (Claude) made this change for Alex, who asked to switch the default urgency
threshold to
consider. Before writing anything I confirmed one open question:the settings row is created eagerly on first tab open, so a default change alone
would not move anyone who had already opened the tab. Alex confirmed there are no
existing users yet (just merged, demoing soon), so I both changed the default and
added a data migration that resets any existing rows (variant 2).
Skills invoked:
/django-migrations(the migration is hand-written because Djangowas not installed to run
makemigrations; I matched theAlterFieldoutput shapeand added a
RunSQLdata reset),/improving-drf-endpoints(serializerhelp_textedit), and
/writing-tests(the test edits are mechanical expectation updates, notnew coverage).
One judgment call worth flagging: I moved the temporal dataclass defaults to
considertoo. They only matter for deserializing pre-field payloads, of whichthere are none, but keeping them at
should_fixwould have made the "defaultsmatch the model's" comment false.
Created with PostHog Code