fix(insights): validate repairs against the inspected definition - #726
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
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: Team 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Architecture diagram
sequenceDiagram
participant Agent as Insights Agent
participant Tools as Tool Registry
participant Inspect as Definition Inspection
participant Validate as validateDefinitionOutcome
participant Repair as insightRepairError
participant Apply as Transactional Apply
Note over Agent,Apply: Runtime flow for validated definition repairs
Agent->>Tools: list_goals/list_funnels
Tools->>Inspect: Read saved definitions
Inspect-->>Tools: Definitions with exact IDs
Tools-->>Agent: Tool results (latest successful read)
Agent->>Agent: Propose outcome (act/edit/delete/resolve)
Agent->>Validate: validateAgentOutcome(outcome, results)
Validate->>Validate: Find latest successful read for signal entity ID
alt Goal/Funnel entity
Validate->>Repair: insightRepairError(entity, current, changes)
end
alt Valid proposal
Repair-->>Validate: null (pass)
Validate-->>Agent: Accept proposal
else Invalid proposal
Repair-->>Validate: Error message
Validate-->>Agent: Throw error - reject before publication
Agent->>Agent: Retry with correction feedback
end
alt Cannot verify exact subject
Validate->>Repair: insightRepairError(entity, current)
Repair-->>Validate: "Not inspected" error
alt Proposal would publish/claim cause
Validate-->>Agent: Throw - must resolve privately
else Private resolution
Validate-->>Agent: Accept resolve with rootCause null
end
end
Note over Agent,Apply: Accepted proposal proceeds to Apply
Agent-->>Apply: Execute accepted repair (edit/delete)
Apply->>Repair: insightRepairError(entity, current, changes)
alt Apply validates pass
Apply-->>Agent: Publish result
else Apply rejects
Apply-->>Agent: badRequest error
end
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
|
The latest updates on your projects. Learn more about Unkey Deploy
|
izadoesdev
left a comment
izadoesdev
left a comment
There was a problem hiding this comment.
Reviewed and addressed all three findings in adb177c:
- Goal no-op validation now compares effective analytics types, so EVENT and CUSTOM are equivalent. Added a real API regression that verifies the goal and reply queue remain unchanged.
- Definition inspection now records attempted tool calls separately from successful evidence reads. Both error-valued and thrown lookup failures must resolve privately without a claimed cause; both paths have correction-loop tests.
- The funnel evaluation now compares the complete ordered replacement steps, conditions, and effective filters. Earlier saved funnel outputs were also inspected against this stronger requirement.
Validation: 265 insights tests pass, repository lint and all 33 type/build tasks pass, full pre-push tests pass. CI is rerunning the now-21 Apply integration tests on this commit. Focused synthetic model runs preserve valid repairs while closing missing and failed definition reads privately. Gateway interruptions remain in the evaluation artifacts rather than being scored as quality passes.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes insight repair proposals being validated against the wrong definition by matching the inspected ID; includes regression tests and rejects no-op or condition-dropping patches. No schema, config, or policy changes.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes insight repair validation to match the inspected definition by exact ID and share measurement-change checks with Apply, with regression tests. No schema, config, or policy changes; behavior is bounded and clearly beneficial.
Re-trigger cubic
Greptile SummaryThis PR validates proposed goal and funnel repairs against the exact inspected definition before publication and reuses the same measurement-equivalence checks at transactional Apply.
Confidence Score: 4/5The behavioral changes appear sound, but the explicit repository typing requirement must be satisfied before merging. No correctness or security failure remains in the exact-ID lookup, failed-read handling, proposal correction, or transactional Apply paths; the sole accepted issue is the newly introduced use of forbidden Files Needing Attention: packages/rpc/src/routers/insight-repairs.ts, apps/insights/src/agent.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Goal or funnel signal] --> B[Inspect current definitions]
B --> C{Exact entity ID found?}
C -- No or read failed --> D[Resolve privately<br/>publish false, rootCause null]
C -- Yes --> E[Model proposes repair]
E --> F[Shared repair validation]
F --> G{Valid measurement change?}
G -- No --> H[Return correction feedback]
H --> E
G -- Yes --> I[Publish executable action]
I --> J[Transactional Apply]
J --> K[Revalidate against locked current definition]
K --> L[Apply and queue recheck]
Reviews (1): Last reviewed commit: "docs(insights): document expanded evalua..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /** Shared by proposal validation and the transactional Apply boundary. */ | ||
| export function insightRepairError( | ||
| entity: { id: string; type: "goal" | "funnel" }, | ||
| current: unknown, |
There was a problem hiding this comment.
The exported repair validator accepts current: unknown, which violates the repository directive to avoid any, unknown, and never in favor of explicit types. The same untyped value is propagated by apps/insights/src/agent.ts:374, and the filter schemas at lines 13 and 18 introduce additional unknown values. Please define an explicit goal/funnel inspection input union before merging to satisfy the repository requirement.
Context Used: Basic guidelines for the project so vibe coders do... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Fresh model evaluations found the agent proposing a goal repair after reading a different goal with the same name. It also offered ineffective patches that Apply rejected only after a customer clicked them.
Match the inspected goal or funnel by the signal's exact ID, then validate measurement changes using the same function as transactional Apply. Invalid proposals receive correction feedback before publication. Missing and failed definition reads must resolve privately without a claimed cause, including thrown tool errors. Preserve stored funnel conditions and reject cosmetic, already-correct, or equivalent EVENT/CUSTOM patches.
Validation:
Review addressed goal type equivalence, failed lookup attempts, complete funnel patch comparisons, and empty-condition equivalence. The final rubric was also reapplied to all 97 saved outcomes; it exposes three intermediate-version coverage diagnoses that the first repair-only rubric had missed.
Scope: proposal validation, shared Apply checks, and synthetic evals. Based on staging after #723, #724, and #725; no unmerged dependency. No schema or model change. Remaining product issues include repetitive briefs, weak recovery conditions, low-value missing-connector notices, and semantic claim validation.