refactor(observability): make project run dispatch and run id parsing easier to follow - #4607
Conversation
… easier to follow Move the project run dispatch out of the project_run.execute span callback into executeProjectRun, read the run id from a regex capture group instead of a path segment index, and drop an error branch identical to its fallthrough.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Warning Review limit reachedNext included review available in 26 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
Review: 90/100 — clean, well-scoped readability refactor with no behavior changeTraced all three changes by hand against the old code; each is a faithful, behavior-preserving simplification. Strengths
Minor / optional
Given the size, clarity, and correctness of this refactor, this is a good example of a readability-only PR — nothing here blocks merge. Generated by Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
1 participant
Readability pass over the final veryfront-code observability code from #4600–#4604, read as a newcomer rather than as diffs. No behavior change: span names and attributes, log fields and response shapes are unchanged. Files touched by the open clean-up #4605 (run-stream, metrics, proxy logger, route registry) are left alone.
Changes
project-run-execute.handler.ts: dispatch moved out of the span callback. Theproject_run.executespan callback held a seven-way nested ternary that picked the task, eval or workflow executor, so the span's own logic (mark failed onsuccess: falseor on a throw) was buried under it. The dispatch now lives inexecuteProjectRunas anif/switchin the same order, and the span callback reads as: run, mark failed if needed, respond.project-run-execute.handler.ts: removed a branch identical to its fallthrough. The request-errorcatchhadif (error instanceof SyntaxError || error instanceof Error) return 400 "Invalid project run execute request"followed by the samereturn. A reader had to check whether the two differed.control-plane-routes.ts: run id comes from a capture group.controlPlaneRunIdFromPathtested the route regex, then readpathname.split("/")[4], so the reader had to count path segments to confirm index 4 is the run id. Both route regexes now capture the run id as group 1 (documented), and the function returnsroute?.exec(pathname)?.[1]. The regexes have nog/yflag, soexecis stateless; other callers only use.test.Checks:
deno fmt,deno lint,deno check, targeted tests (control-plane-routes, control-plane, project-run-execute.handler, proxy handler),deno task lint:testing-front-door,TEST_SEMANTIC_AUDIT_BASE_REF=origin/main deno task lint:test-semantic-dispositions.Part of veryfront/veryfront-issue-inbox#1860