fix(insights): validate tool-supplied retention comparisons - #780
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
@greptileai review this exact head: Please check retention publication without a saved snapshot: exact native get_data references; complete overall populations with at least 50 eligible profiles each; event/namespace/horizon/date/timezone/cutoff binding; sticky earlier conflicts; and preservation of independent findings and complete overall aggregates when daily rows are trimmed. The detector already enforces sample and maturity thresholds; this PR closes the supporting-tool-evidence route. The native-agent regression reproduces the previous head incorrectly publishing 16/20 → 4/20 and verifies rejection on this head. Root lint, 33 package typechecks, all 27 pre-push test tasks and 25 targeted publication tests pass. Fresh model behavior evaluation remains in progress. Please provide an explicit verdict on this commit rather than treating a skipped review check as approval. |
Greptile SummaryThis PR strengthens the Insights publication boundary for tool-supplied retention comparisons.
Confidence Score: 5/5The PR appears safe to merge with no outstanding findings. No changes were made after the previous review, no new actionable issues were identified, and the sole previous finding was resolved after Greptile explicitly withdrew it as an appropriate untrusted-input validation boundary. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Model cites two get_data results] --> B[Validate exact references and retention schema]
B --> C{Scopes and cohort metadata match?}
C -- No --> D[Reject finish for correction]
C -- Yes --> E{Both cohorts complete and eligible?}
E -- No, private outcome --> F[Render eligible and incomplete counts without a rate]
E -- No, public outcome --> D
E -- Yes --> G{Earlier equivalent read conflicts?}
G -- Yes --> D
G -- No --> H[Render code-grounded retention comparison]
H --> I[Publish qualitative finding]
Reviews (3): Last reviewed commit: "fix(insights): avoid redundant retention..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
The latest updates on your projects. Learn more about Unkey Deploy
|
|
@greptileai review the final pushed head Fresh provider runs exposed unnecessary corrections on a private limitation and one lost valid result after generic numeric-copy feedback. This update lets private undersized/incomplete structured comparisons record validated eligibility counts without a retention-rate claim. Public minimum populations, maturity, scope and sticky-conflict checks remain intact. Private authoring counts still undergo ordinary numeric grounding. Public numeric feedback now quotes the exact field to correct while preserving valid evidence/references. Root lint, 33 package typechecks, 683 insights tests (six live-only skipped), and 28 publication boundary tests pass. Independent review checked the private fallback and public protections. Please provide an explicit verdict on this exact head; the prior untrusted-input |
1 participant
A report-use investigation can query retention as supporting evidence without a saved retention snapshot. Previously, numeric grounding could accept a published comparison from fewer than 50 eligible profiles or incomplete follow-up. The retention detector and scheduled rechecks already enforced those limits; the missing check was on tool-supplied evidence.
Published native retention evidence now selects two exact
get_dataresults with{retention: true}. Code validates and renders the overall populations, cohort dates, identity basis, event selectors, horizon, timezone and observation cutoff. An earlier conflicting read of the same population remains binding. A complete overall aggregate remains usable when daily display rows are trimmed, and unrelated uncited retention data does not suppress an independently supported finding. Unsupported structured comparisons resolve privately with code-rendered eligible/incomplete profile counts and no asserted retention rate. Private grounded counts no longer trigger public-copy restrictions. If public prose repeats measured numbers, feedback names the exact field to rewrite and preserves the valid evidence references.Scope: the insights agent's publication boundary and synthetic regression tests. No schema, dashboard or query-executor changes. Validation runs inside the existing finish call; actual model corrections are measured separately. No dependencies. Known overlap: draft #751 also edits the agent and SPEC; this slice starts from staging and does not depend on it.
Validation: 683 insights tests passed (six live-only skipped), all 28 publication tests passed, and root lint and all 33 package typechecks pass. Native-tool argument validation is exercised with synthetic query outputs. Independent review passes; Greptile reviewed final head aee85ea and reports 5/5 with no outstanding findings. All review threads are resolved.
Fresh paired qualification ran 16 unscripted agent invocations (four synthetic cases, two repetitions, both heads) through the native get_data executor with only database execution replaced. The old head published all four undersized/incomplete comparisons; this head published none. Both heads preserved both mature 50-profile comparisons and both independent report-use findings. All attempts completed. Candidate median latency was 17.9s versus 15.5s; total model turns were 30 versus 28, with five versus one rejected finishes. This is a publication-quality improvement, not a latency reduction.
Earlier attempts remain recorded: the first 16-run batch is excluded from retention conclusions because its replay mishandled native date precedence. A corrected intermediate comparison exposed one lost valid result and unnecessary private corrections, which prompted the final fix. Its inconsistent companion event counts were corrected for both arms before the final fresh comparison. Tests do not establish SQL execution, production data quality or customer value.
AI-assisted implementation and review using Codex.