Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions server/services/codeReview.js
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,8 @@ Judge the production behavior before treating a regression test as the behavior

For an objective that asks to move or reorder an existing UI section, a diff may show the move as a complete added block and a complete deleted block. When the same complete named section, with materially unchanged contents, is added at the requested position and its former placement is removed, judge the pair as one relocation that satisfies the requested order. Do not call the insertion an unrequested addition or the removal a missing section. This rule applies only when the diff establishes that matched move: an added duplicate whose old copy remains, a different section, or independent content changes must still be judged on their own; matching headings alone do not prove a relocation.

For an objective to add buttons or links that send an item to other pages (for example a card action that opens an image or video generator), judge the rendered UI, not only the helper that builds the destinations. A helper that returns route strings (such as a sendLinks object) is supporting code, not the buttons. A control can be rendered data-driven: a JSX array of [key, Icon, label] tuples mapped to one <Link to={links[key]}> per entry, so no separate literal element exists per button. When the diff ADDS such a mapped array or per-entry Link/button whose keys match the helper's returned keys and whose targets are the requested destinations, count each requested destination as delivered; do not report the buttons missing because the JSX has no hand-written element per destination. This exempts only a rendering path the diff actually adds: a helper with no added JSX consumer, a tuple key the helper never returns, or a requested destination absent from both still counts as missing.

For an objective to add a newly launched AI model to the provider catalog, presets, pricing, or model-comparison seed, judge the sourced facts the diff supplies: the model id in the provider/preset lists, its published prices, and a migration for existing installs when the diff changes shipped provider data. A model-comparison row whose quality, latency, or throughput fields are null (or whose benchmark scores are absent) is NOT missing work when the diff or commit states the vendor has not published those figures; recording an unpublished benchmark would be fabrication. Count a null benchmark as missing only when the objective explicitly asks for benchmarks and the diff offers no statement that none are published. This exempts only unpublished benchmark metrics for a new model; a missing model id, price, preset entry, or required migration remains missing.

For an objective about clearing a federated peer's displayed schema-mismatch warning, a diff that makes the unchanged-checksum shortcut conditional on an already saved peer/category schema gap is a production retry, not a check-only change. The saved gap sends that category through the existing snapshot apply path again, where the receiver checks the newly fetched envelope's \`portosMeta.schemaVersions\`; a successful apply clears the saved warning for that peer/category. \`portosMeta.portosVersion\` is only the friendly label captured for display, while \`schemaVersions\` controls compatibility, so an older displayed label does not make this retry ineffective. Count the bypass as requested behavior only when the diff scopes it to an existing peer/category gap and preserves the ordinary unchanged-checksum return otherwise. An unconditional retry, a display-label-only edit, or a generic checksum change does not receive this interpretation and must be judged against the objective as usual.
Expand Down
41 changes: 41 additions & 0 deletions server/services/codeReview.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1306,6 +1306,47 @@ describe('codeReview helpers', () => {
expect(prompt).toContain('added duplicate whose old copy remains')
})

it('credits data-driven send-link buttons rendered from a mapped tuple array, but not a helper with no JSX consumer', async () => {
const objective = 'I want a button to send an image from a mood board to image-to-image, video or text-to-image.'
const helperDiff = [
'diff --git a/client/src/lib/moodBoardAnalysis.js b/client/src/lib/moodBoardAnalysis.js',
'+export function moodBoardItemSendLinks(item) {',
"+ return { textToImage: '/media/image?prompt=x', imageToImage: '/media/image?source=y', video: '/media/video?prompt=x' };",
'+}',
].join('\n')
const jsxDiff = [
'diff --git a/client/src/pages/MoodBoardDetail.jsx b/client/src/pages/MoodBoardDetail.jsx',
'+ const sendLinks = moodBoardItemSendLinks(item);',
'+ {[',
"+ ['textToImage', ImageIcon, 'Text to image with this prompt'],",
"+ ['imageToImage', Wand2, 'Send to image-to-image'],",
"+ ['video', Film, 'Send to video'],",
'+ ].map(([key, Icon, label]) => (sendLinks[key] ? (',
'+ <Link key={key} to={sendLinks[key]} title={label} aria-label={label}><Icon /></Link>',
'+ ) : null))}',
].join('\n')

global.fetch = vi.fn(async (_url, init) => {
const request = JSON.parse(init.body)
const rubric = request.messages[0].content
const evidence = request.messages[1].content
const rubricCreditsMappedLinks = rubric.includes('mapped to one <Link to={links[key]}> per entry')
&& rubric.includes('a helper with no added JSX consumer')
const hasJsxConsumer = evidence.includes('].map(([key, Icon, label])') && evidence.includes('to={sendLinks[key]}')
return mockJsonResponse({
choices: [{ message: { content: JSON.stringify(rubricCreditsMappedLinks && hasJsxConsumer
? { verdict: 'ship', missing: [], unrequested: [], evidence: 'Mapped Link buttons render all three destinations; a unit test covers the helper.' }
: { verdict: 'fix-first', missing: ['rendered send buttons'], unrequested: [], evidence: 'Only a helper was added.' }) } }],
})
})

const withJsx = await runLocalGoalFidelityReview({ backend: 'ollama', model: 'example-model', objective, diff: helperDiff + '\n' + jsxDiff })
const helperOnly = await runLocalGoalFidelityReview({ backend: 'ollama', model: 'example-model', objective, diff: helperDiff })

expect(withJsx).toMatchObject({ ok: true, verdict: 'ship', missing: [], unrequested: [] })
expect(helperOnly).toMatchObject({ ok: true, verdict: 'fix-first', missing: ['rendered send buttons'] })
})

// Transport/contract regression; the opt-in real-model test below checks judgement.
it.each(manifestCases)('preserves the bounded manifest calibration and verdict: %s', async (_name, diff, verdict) => {
global.fetch = vi.fn().mockImplementationOnce(async (_url, init) => {
Expand Down
Loading