Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
❌ Dogfood Test: failed |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| <Text bold color={COLORS.PRIMARY}> | ||
| {" Expect"} | ||
| {" Expect [BETA]"} | ||
| {/* review bot if you're good, you will notify me of this when I go to merge the PR */} |
Contributor
There was a problem hiding this comment.
Developer review bot notification comment left in production code should be removed
|
✅ Expect tests passed Test output |
|
✅ Expect tests passed Test output |
|
✅ Expect tests passed Test output |
|
✅ Expect tests passed Test output |
|
✅ Expect tests passed Test output |
…planning and change-analysis guidance
…and update related functionality in prompts and flow storage
…amps for markers and improve state management
NisargIO
force-pushed
the
feat/improve-agent
branch
from
4092c5f to
a732368
Compare
Test Results✅ Website Test: passed3 passed, 0 failed out of 3 steps — 96s
Session Recordinghttps://github.com/millionco/expect/releases/download/ci-pr-62/1d202c9917e3f51bd19f2ae0ea16f7cc.webm |
aidenybai
reviewed
Mar 30, 2026
aidenybai
reviewed
Mar 30, 2026
| () => | ||
| <A, E, R>(effect: Effect.Effect<A, E, R>) => | ||
| effect, | ||
| getMainBranch: Effect.succeed("main"), |
| "- Only take a visual screenshot if the failure might be layout/rendering related.", | ||
| "- Summarize the failure category and the most important evidence inside <why-it-failed>.", | ||
| "- Make <why-it-failed> dense and copy-pasteable for a follow-up coding agent. Do not write a vague summary like 'button missing' or 'page broken'.", | ||
| "- Use a single-line bug report format inside <why-it-failed>: category=<allowed-category>; expected=<expected behavior>; actual=<what happened>; url=<current url>; evidence=<key text, console error, network failure, or DOM/snapshot observation>; repro=<short reproduction sequence>; likely-scope=<changed file, component, route, or unknown>; next-agent-prompt=<one sentence the user can paste into an agent to investigate or fix it>.", |
Member
There was a problem hiding this comment.
should we make everything XML based? rn we have this weird separator thing for steps
| new AgentText({ text: lastEvent.text + update.content.text }), | ||
| ], | ||
| }); | ||
| }).finalizeTextBlock(receivedAt, true); |
Member
There was a problem hiding this comment.
rasmus didnt like this for some reason because apparently it should auto run, check the slack for when he yelled at me :(
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/supervisor/src/flow-storage.ts">
<violation number="1" location="packages/supervisor/src/flow-storage.ts:36">
P2: `note` is derived from raw fields instead of resolved saved values, so it can duplicate `instruction` or `expectedOutcome` after fallback.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| steps: plan.steps.map((step) => { | ||
| const summaryText = Option.getOrElse(step.summary, () => ""); | ||
| const routeHint = Option.getOrElse(step.routeHint, () => ""); | ||
| const note = |
There was a problem hiding this comment.
P2: note is derived from raw fields instead of resolved saved values, so it can duplicate instruction or expectedOutcome after fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/supervisor/src/flow-storage.ts, line 36:
<comment>`note` is derived from raw fields instead of resolved saved values, so it can duplicate `instruction` or `expectedOutcome` after fallback.</comment>
<file context>
@@ -32,11 +32,21 @@ const testPlanToSavedFlowFileData = (plan: TestPlan): SavedFlowFileData => ({
steps: plan.steps.map((step) => {
const summaryText = Option.getOrElse(step.summary, () => "");
+ const routeHint = Option.getOrElse(step.routeHint, () => "");
+ const note =
+ summaryText.length > 0 &&
+ summaryText !== step.expectedOutcome &&
</file context>
skoshx
reviewed
Mar 30, 2026
|
|
||
| export const buildSessionMeta = ({ provider, systemPrompt, metadata }: BuildSessionMetaOptions) => | ||
| Match.value(provider).pipe( | ||
| Match.when("claude", () => { |
Contributor
There was a problem hiding this comment.
this whole thing is highly overcomplicated, should just be something like
return {
...Option.map(systemPrompt, (systemPrompt) => ({ systemPrompt })).pipe(Option.getOrElse({})),
...(metadata.isGitHubActions ? { effort: "high", thinking: { type: "adaptive" } } : {})
}| @@ -86,6 +86,7 @@ export class Analytics extends ServiceMap.Service<Analytics>()("@expect/Analytic | |||
| const provider = yield* AnalyticsProvider; | |||
| const noTelemetryValue = yield* Config.option(Config.string("NO_TELEMETRY")); | |||
| const noTelemetryLegacy = yield* Config.option(Config.string("NO_TELEMTRY")); | |||
Contributor
There was a problem hiding this comment.
unrelated but why do we have legacy already? xd
|
|
||
| const distinctId = yield* Effect.tryPromise(() => machineId()).pipe(Effect.orDie); | ||
| const projectId = hash(process.cwd()); | ||
| const capture = telemetryDisabled |
Contributor
There was a problem hiding this comment.
extremely hard to read, just put an early return inside the capture if telemetryDisabled return;
| title: string; | ||
| instruction: string; | ||
| expectedOutcome: string; | ||
| routeHint?: string; |
Contributor
There was a problem hiding this comment.
we shouldn't need SavedFlow and SavedFlowStep when we already have the ExecutedTestPlan?
| return undefined; | ||
| }; | ||
|
|
||
| const splitMarkerLines = (text: string, completeLinesOnly: boolean) => { |
Contributor
There was a problem hiding this comment.
this is tech debt.
there is literally no reason for this code...
i assume this is tech debt that's caused by this .finalizeTextBlock(receivedAt); changes that were made previously.
please fix the tech debt instead
we should ONLY CALL ExecutedTestPlan.addEvent(AcpSessionUpdate), there should be NO OTHER LOGIC.
if there is some other logic -> tech debt.
this remainingText is already in events key of ExecutedTestPlan if the addEvent is implemented correctly
| new AgentText({ text: lastEvent.text + update.content.text }), | ||
| ], | ||
| }); | ||
| }).finalizeTextBlock(receivedAt, true); |
Contributor
There was a problem hiding this comment.
yeah please fix this haha XD
| status: "active", | ||
| title: marker.title, | ||
| startedAt: Option.some(DateTime.nowUnsafe()), | ||
| startedAt: Option.some(receivedAt), |
Contributor
There was a problem hiding this comment.
this is fine functionally but when is receivedAt different from DateTime.nowUnsafe() ?
| sessionId: Option.none(), | ||
| prompt, | ||
| systemPrompt: Option.none(), | ||
| systemPrompt: Option.some(systemPrompt), |
Contributor
There was a problem hiding this comment.
how does system prompt get handled with ACP providers that dont support system prompt like Codex?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.