Port Founder Weekly Review quality improvements onto current main - #333
Port Founder Weekly Review quality improvements onto current main#333EricLiu2795 wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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: e522570049
ℹ️ 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 citedText = citedSources | ||
| .map(source => `${source.title} ${source.excerpt}`) | ||
| .join(" "); | ||
| if (!/launch|ship|release|deploy|complete|implement/i.test(citedText)) { |
There was a problem hiding this comment.
When a cited document change says something like “the release is planned pending approval” without a future date, this regex treats the word release as an explicit shipping signal, while futureReleaseMentioned returns false because it finds no date. The deterministic evaluation used to gate run-founder-weekly-review-realistic-e2e.ts therefore passes a whatShipped claim supported only by preparation evidence, corrupting the experiment's quality result.
Useful? React with 👍 / 👎.
| function effectiveAnalyzedGroup(success: SuccessfulAnalysis): AnalyzedDocumentChangeGroup | null { | ||
| if (success.result.disposition === "non_material") return null; |
There was a problem hiding this comment.
When any changed fragment exceeds the 150-character per-side analyzer limit, the analyzer sees only a prefix, but a non_material response still removes the entire group here. If the unchanged wording is at the start and the actual business delta occurs later in the chunk, the model cannot observe that delta and the review silently loses material evidence; inputTruncated is recorded only as a warning and is not consulted before accepting this disposition. Truncated inputs should retain the deterministic fallback or otherwise be prevented from producing a destructive non-material decision.
Useful? React with 👍 / 👎.
| rawChanges: [...rawAuditById.values()].sort((a, b) => | ||
| compareOrdinal(a.rawChangeId, b.rawChangeId) | ||
| ), | ||
| groups: [...analyzedGroups].sort(compareFinalOrder).map(({ group, materiality }) => ({ |
There was a problem hiding this comment.
When the retained version pairs contain more than 5,000 raw edits, or more than 2,500 structural groups, these complete arrays exceed DocumentChangeAuditSnapshotSchema's respective maxima in contracts.ts. The collector later passes this audit to FounderWeeklyReviewEvidenceSnapshotSchema.parse, so one sufficiently large document causes the whole evidence collection to fail rather than returning bounded evidence with a truncation warning. The raw and group audit entries need a coordinated limit that preserves group-to-raw references.
Useful? React with 👍 / 👎.
2 participants
Context
#317 rebuilt the core Founder Weekly Review pipeline on the current repository architecture, and #318 landed the scenario framework.
This PR does not merge or restore the historical LAU-9 branch wholesale. It ports only the remaining Founder Weekly Review quality improvements that were developed after the rebuild and were still missing from main.
What this PR adds
Preserved current-main architecture
This PR intentionally does not restore:
Validation
pnpm -r typecheck— PASSgit diff --check— PASSRealistic E2E
The realistic E2E successfully reached:
computed evidence collection
→ Snapshot v2 validation
→ document-change audit/group validation
→ isolated DB workflow setup
→ generation start
It then stopped before any provider request because the current structured-output adapter rejects the existing optional
rationaleschema form (z.string(...).optional()).That schema form is unchanged from origin/main and is not introduced by this PR. This PR intentionally does not expand scope to modify the shared structured-output adapter.
Live provider calls were therefore not made during this final port validation.
Notes for reviewers
This is a selective port onto current main, not a rescue/rebase of #325.
v1 evidence snapshots remain readable while new collection emits the richer v2 audit boundary.