Conversation
|
@codex review |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36211cf343
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| number: value.number, | ||
| title: value.title?.trim() ?? "", | ||
| body: value.body ?? "", | ||
| state: value.state === "open" ? "open" : "closed", |
There was a problem hiding this comment.
The shallow mapper collapses every non-open PR to closed, so incremental syncs that use listChangedPullRequestsSince can overwrite PRs that transitioned from open to merged as plain closed. upsertPullRequestSummary only keeps merged when the existing row is already merged, so the first incremental sync after a merge loses that state and downstream cluster/readiness logic treats the PR incorrectly. Hydrating changed PRs (or deriving merged status from the API payload) is needed to avoid this data regression.
Useful? React with 👍 / 👎.
| return; | ||
| } | ||
| try { | ||
| await execFileAsync("open", [url]); |
There was a problem hiding this comment.
The TUI hardcodes macOS's open command, so pressing o fails with ENOENT on Linux/Windows environments where this CLI commonly runs. That makes the advertised open-link action non-functional outside macOS; selecting the opener by process.platform (e.g., xdg-open/start) avoids this portability break.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7802c0b170
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const seed = this.getPrRow(params.prNumber); | ||
| if (!seed) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
clusterPullRequest reads seed (and may return early) before calling ensurePullRequestCached, so a --refresh path can still build semantic queries and result metadata from stale title/body/state values, and an uncached PR returns null even when repo/source are supplied for hydration. This makes refreshed cluster output inconsistent with the just-fetched PR data and can cause false “not found” results for cache-miss calls.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d610c5bb1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (commandRaw in COMMAND_USAGE) { | ||
| return commandRaw as Command; |
There was a problem hiding this comment.
parseCommand uses commandRaw in COMMAND_USAGE, which also matches inherited prototype keys such as toString and constructor. Those inputs are then cast to Command and dispatched through commandHandlers[args.command], so invalid commands can execute inherited object methods instead of returning the intended usage error path. This makes CLI behavior unpredictable for malformed input and can surface runtime errors; use an own-property check (for example Object.hasOwn) before accepting the command.
Useful? React with 👍 / 👎.
| isDraft: false, | ||
| author: value.user?.login?.trim() ?? "", | ||
| baseRef: "", | ||
| headRef: "", |
There was a problem hiding this comment.
Incremental sync now relies on issue-style summaries for changed PRs, but this mapper hardcodes isDraft to false and empties both branch refs. When a PR is first seen during incremental sync (no prior row to preserve), the database stores incorrect draft/branch metadata and keeps it until a manual detail refresh or full sync, which skews inbox ranking (draft penalty not applied) and makes branch-based filtering inaccurate for those PRs.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b147f0b9dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const syncedAt = isoNow(); | ||
| params.setMeta(params.metaKeys.lastSyncAt, syncedAt); | ||
| params.setMeta(params.metaKeys.lastSyncWatermark, syncedAt); |
There was a problem hiding this comment.
syncPullRequestsWorkflow fetches incremental changes using the old watermark first, but then stores lastSyncWatermark as isoNow() at the end. If a PR is updated after the changed-list request is made but before this final timestamp is written, that update is not in the current batch and will be older than the new watermark, so the next incremental sync will skip it permanently. This creates a real data-loss window on active repos; persist a watermark captured at sync start (or the max fetched updatedAt) instead. The issues workflow uses the same end-of-sync watermark pattern.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a21c541cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (githubSnapshot.reviewDecision === "CHANGES_REQUESTED") { | ||
| state = "needs_work"; | ||
| summary = "GitHub review decision is CHANGES_REQUESTED."; |
There was a problem hiding this comment.
This logic only downgrades readiness for CHANGES_REQUESTED, failing checks, conflicts, or pending checks, so a PR with reviewDecision = "REVIEW_REQUIRED" and green checks is reported as ready. In GitHub terms that state means review is still required, so this misclassifies merge readiness and can surface unreviewed PRs as merge-ready in triage flows.
Useful? React with 👍 / 👎.
| for (const match of body.matchAll(/\bsource issue\s*#(\d+)\b/gi)) { | ||
| addLinkedIssue(out, Number(match[1]), "source_issue_marker"); | ||
| } | ||
| for (const match of body.matchAll(/\b(?:fix(?:e[sd])?|close[sd]?|resolve[sd]?)\s+#(\d+)\b/gi)) { |
There was a problem hiding this comment.
The body parser only matches one issue number immediately after the closing verb, so text like Fixes #12, #34 records #12 but drops #34. Because derived links feed clustering, cross-references, and priority grouping before/without fact hydration, this causes incomplete linkage and under-groups related PRs.
Useful? React with 👍 / 👎.
| const currentDir = dirname(fileURLToPath(import.meta.url)); | ||
| const entrypoint = resolve(currentDir, "clawlens.ts"); | ||
|
|
||
| const child = spawn(process.execPath, ["--import", "tsx", entrypoint, ...process.argv.slice(2)], { |
There was a problem hiding this comment.
The installed clawlens binary always launches with --import tsx, but tsx is only declared in devDependencies in this commit. In production-style installs that omit dev dependencies (for example pnpm install --prod), invoking the binary fails with module resolution errors, so the shipped command is not reliably runnable outside a dev install.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ae4de5f9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| JSON.stringify(facts.statusChecks), | ||
| facts.fetchedAt, | ||
| ); | ||
| this.clearIssueLinksForSources(facts.prNumber, ["closing_reference"]); |
There was a problem hiding this comment.
recordPullRequestFacts deletes only closing_reference rows before re-inserting facts.linkedIssues, but the imported facts also include derived sources (body_reference, title_reference, source_issue_marker). When a PR’s body/title references change, old derived links remain in pr_linked_issues, so later clustering and linked-issue reasoning can include outdated issues for that PR. Clear the derived link sources (or all sources present in facts.linkedIssues) before inserting the refreshed snapshot.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3bd2ceb30
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const payload of shallowPullRequests) { | ||
| await params.upsertHydratedPullRequest(payload, { indexVectors: false }); |
There was a problem hiding this comment.
This path writes shallow full-sync entries through upsertHydratedPullRequest, but those payloads were created with comments: [], so the write clears pr_comments and comment search docs for any updated PR. In practice, a regular sync --full can silently drop previously cached discussion context for active PRs, and detail/search quality stays degraded until each PR is manually refreshed. Use the summary upsert path for shallow items (or avoid destructive comment/doc deletes in shallow mode) so cache-first browsing is preserved.
Useful? React with 👍 / 👎.
| this.clearIssueLinksForSources( | ||
| facts.prNumber, | ||
| Array.from(new Set(facts.linkedIssues.map((issue) => issue.linkSource))), |
There was a problem hiding this comment.
recordPullRequestFacts only removes link sources that appear in the new facts.linkedIssues set. If a refreshed snapshot has zero linked issues, the source list is empty and no delete runs, leaving old fact-derived links in pr_linked_issues. That produces persistent false issue associations (and therefore incorrect clustering/xref behavior) after a PR body/edit removes its previous closing references.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb03a6bdb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.isListMode(this.mode) && | ||
| this.rows.length >= currentBrowseCapacity(this.mode, this.browseLimit) | ||
| ); |
There was a problem hiding this comment.
canLoadMore gates pagination on rows.length >= currentBrowseCapacity(...), but for cross-search the capacity is limits.pr + limits.issue while loading uses separate per-type caps (search(...limits.pr) and searchIssues(...limits.issue)). If one side underfills (e.g., 10 PR hits and 0 issue hits with many PRs still available), rows.length is below the combined capacity and "More" is disabled even though additional PR results exist, so users cannot page through the remaining results.
Useful? React with 👍 / 👎.
- dev: launch TUI directly (fast, uses existing DB) - dev:sync: incremental sync + TUI - dev:full: full sync for initial setup - gitignore: exclude .claude/ local config
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5602726f9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.render(this.controller.getRenderModel()); | ||
| }); | ||
| this.render(this.controller.getRenderModel()); | ||
| await this.controller.initialize(); |
There was a problem hiding this comment.
run() subscribes and creates the Blessed screen before awaiting controller.initialize(), but if initialization throws (for example, DB open or status fetch errors), execution exits early and never reaches the destroy/shutdown path. In that failure path the terminal can be left in Blessed's alternate/raw state and listeners remain attached, which is a user-visible breakage for any startup error. Wrap startup in a try/finally (or catch) that always calls shutdown()/screen.destroy() when initialization fails.
Useful? React with 👍 / 👎.
| this.replayTimer = setTimeout(() => { | ||
| void this.refreshActiveListPreservingUi(); | ||
| }, REPLAY_DEBOUNCE_MS); |
There was a problem hiding this comment.
scheduleListReplay() fires refreshActiveListPreservingUi() in a detached setTimeout without any rejection handling, and refreshActiveListPreservingUi() awaits resolveRows() without an internal try/catch. If row resolution throws during background replay (e.g., transient store/read error after sync), the promise rejection is unhandled and can terminate the process in modern Node runtimes, interrupting the TUI. Catch and route the error through reportError (or equivalent) in this replay path.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8627c2bd5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "watchlist": | ||
| case "pr-search": | ||
| case "issue-search": | ||
| return rows.length >= currentBrowseCapacity(mode, browseLimit); |
There was a problem hiding this comment.
canLoadMoreRows gates pagination on rows.length >= browseLimit, but Inbox rows can be collapsed clusters that each represent many PRs. Because resolveListRows requests listPriorityInbox with limit = browseLimit, it is possible to get fewer visible rows than browseLimit while still having additional items available at higher limits; this condition disables the More action too early and traps users on a partial Inbox.
Useful? React with 👍 / 👎.
| const repo = this.getMeta(META_REPO) ?? ""; | ||
| const latestReviewFact = repo ? this.getLatestReviewFact(candidate.prNumber, repo) : null; |
There was a problem hiding this comment.
resolveMergeReadiness pulls the repo key from META_REPO instead of the repo being queried. If a shared --db is used across multiple repos, the last sync overwrites META_REPO, so context/cluster views for another repo can read the wrong pr_review_facts row and report incorrect readiness. This should be keyed from the caller’s repo context, not global sync metadata.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Verification