feat: enhance supercode-cli with new features and updates#224
Conversation
- Added support for the brag skill in skills-lock.json and updated .gitignore to exclude related files. - Bumped supercode-cli version to 0.1.83 in package.json. - Improved AI chat functionality by adding reasoning content handling in index.ts. - Introduced tool call budget guidelines in context.ts to optimize tool usage. - Added foundational concepts on transformers and fine-tuning in techy documentation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
WalkthroughThe proxy AI pipeline now serializes and validates tool calls, limits multi-turn execution, tracks repeated or denied calls, and handles reasoning output and final summaries. The ChangesAI runtime and tool execution
Transformer and fine-tuning documentation
Project metadata
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ServerProxyService
participant ToolExecutor
participant ModelProvider
Client->>ServerProxyService: Send chat request with tools
ServerProxyService->>ModelProvider: Request serialized tools and messages
ModelProvider-->>ServerProxyService: Tool call or reasoning/text delta
ServerProxyService->>ToolExecutor: Validate and execute tool call
ToolExecutor-->>ServerProxyService: Tool result or validation error
ServerProxyService->>ModelProvider: Continue within step budget
ServerProxyService-->>Client: Stream reasoning, text, or final summary
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)apps/supercode-cli/server/package.jsonTraceback (most recent call last): skills-lock.jsonTraceback (most recent call last): 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/supercode-cli/server/src/cli/ai/tool-executor.ts (1)
217-233: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winOff-by-one:
iter = maxIterations - 1does not actually grant one more iteration.After this assignment, the
forloop's owniter++bringsiterto exactlymaxIterations, and the loop conditioniter < maxIterationsimmediately fails — the loop terminates without any furtherstreamTextcall. This has the same net effect as the olditer = maxIterationsbehavior the comment says it replaces: the model never actually sees the "all tool calls returned empty" sentinel before the function returns, defeating the purpose of the change.🐛 Proposed fix
- // Allow one more iteration so the model sees the sentinel and responds. - iter = maxIterations - 1 + // Allow exactly one more iteration so the model sees the sentinel and responds. + iter = maxIterations - 2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/supercode-cli/server/src/cli/ai/tool-executor.ts` around lines 217 - 233, Fix the iteration handling in the all-empty-results branch of the tool-execution loop so the sentinel message is followed by one additional streamText iteration. Update the assignment involving iter near the allToolResults.every and isEmptyToolResult checks to account for the loop’s automatic iter++ and avoid terminating when the loop condition is next evaluated.
🧹 Nitpick comments (3)
apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts (1)
239-276: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRepetition/denial sentinels warn but never stop the loop.
Unlike the analogous guards in
tool-executor.ts(stopForRepetition/stopForDenialLoop, which set a flag andbreak), here the "you're repeating yourself" / "stop calling the denied tool" notices are pushed intocurrentMessagesbut no stop flag is set. If the model ignores the warning, the exact same condition re-triggers every subsequent iteration, re-appending the same notice and burning through the remainingMAX_STEPSbudget with real API calls instead of terminating early.♻️ Suggested direction
+ if (repeated || sawDenial) { + // push the relevant notice(s) as today, then: + currentMessages.push({ role: "system" as const, content: "..." }) + break // let the caller's final-summary fallback (below) take over + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts` around lines 239 - 276, Update the loop containing the repetition and denial sentinel checks to terminate early when either condition is detected, matching the stop behavior of tool-executor.ts methods stopForRepetition and stopForDenialLoop. Set the existing loop-stop mechanism and break or return after handling the condition, while preserving the notices in currentMessages.apps/supercode-cli/server/src/index.ts (1)
381-384: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReasoning-handling logic is copy-pasted across 6 provider loops.
The
const reasoningChunk = delta?.reasoning_content || delta?.reasoning; if (reasoningChunk) { ... }block is repeated verbatim in the openrouter, nvidia, mergedev, orcarouter, concentrateai, and supercode streaming loops, and the fallback-content selection (fbMsg.content ?? fbMsg.reasoning_content ?? "") is duplicated identically between the concentrateai and supercode branches. Any future fix (e.g. a new provider's reasoning field name) now needs to be applied in 6+ places.♻️ Suggested extraction
function extractReasoningChunk(delta: any): string | undefined { return delta?.reasoning_content || delta?.reasoning } function pickFallbackContent(msg: any, reasoningContent: string): string { const content = msg?.content ?? msg?.reasoning_content ?? "" return content || reasoningContent }Then each loop becomes
const reasoningChunk = extractReasoningChunk(delta)and the two fallback blocks becomeconst fbContent = pickFallbackContent(fbMsg, reasoningContent).Also applies to: 528-531, 638-641, 749-752, 869-873, 936-941, 1033-1037, 1096-1101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/supercode-cli/server/src/index.ts` around lines 381 - 384, Extract the repeated reasoning selection into a shared extractReasoningChunk helper and replace each provider loop’s inline reasoning_content/reasoning logic with it, including all listed streaming branches. Extract the duplicated fallback-content selection into pickFallbackContent and use it in the concentrateai and supercode fallback paths, preserving the current precedence and behavior.apps/supercode-cli/server/src/cli/ai/provider.ts (1)
77-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared step budget for
onStepBudget.
ServerProxyService.sendMessagedoes not acceptonStepBudget; it ownsMAX_STEPS = 8internally. Avoid keeping8duplicated inprovider.ts; either reuse the service’s budget value when notifying the callback or drop this wiring untilServerProxyServicesupports passing the budget through.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/supercode-cli/server/src/cli/ai/provider.ts` around lines 77 - 80, Update the sendMessage adapter’s onStepBudget handling to avoid duplicating the literal 8 in provider.ts. Reuse ServerProxyService’s shared MAX_STEPS value when invoking onStepBudget, or remove this callback wiring until ServerProxyService.sendMessage supports propagating the budget; keep the existing svc.sendMessage invocation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts`:
- Around line 404-438: Update usage handling in the surrounding tool-call loop
and final-summary fallback so each request’s usage is added to the running total
rather than assigned over it. Replace the per-iteration and finalResult usage
assignments with field-wise accumulation matching tool-executor.ts, while
preserving existing values when a request omits usage.
- Around line 293-334: Update the validation-failure envelope in the tool
execution flow around toolFn.inputSchema.safeParse to include success: false,
matching the structured error contract used by the existing tool-executor catch
path. Extract the duplicated assistant tool-call and tool-result message appends
into a pushToolCallMessages helper, then reuse it in both the validation-error
branch and normal execution path while preserving their existing result
handling.
- Around line 374-393: Guard the tool-call argument handling in the tool call
repetition loop by defaulting null or undefined call.args to an empty object
before passing it to Object.keys and JSON.stringify. Update the logic around
toolCallHistory and preserve the existing toolName/argsKey tracking behavior for
valid arguments.
In `@apps/supercode-cli/server/src/cli/workspace/context.ts`:
- Around line 108-132: The tool-call budget text in the context prompt conflicts
with the limit enforced by executeToolLoop. Align the “Max 5 rounds” guidance
and related wording with the actual configured limit, or update the runtime
default/configuration to enforce five rounds; ensure the prompt and
executeToolLoop use the same maximum.
In `@techy/06-transformers-llm-finetuning.md`:
- Around line 139-144: Escape the literal pipe characters in the ChatML, LLaMA
3, and Phi examples within the chat-format table so they are rendered as content
rather than column separators. Preserve the existing examples and table
structure, ensuring the table passes MD056.
- Around line 94-99: Add language identifiers to every fenced code block in this
document, including the chat/template examples near the shown section and the
additional blocks at the referenced locations. Use text for chat or template
examples, and text or math for the loss expression, while preserving their
contents unchanged.
- Around line 283-304: Add citations or explicit measurement conditions for the
PEFT comparison table, including model precision, sequence length, batch size,
optimizer, and whether memory is peak GPU memory; clarify how the full
fine-tuning estimate accounts for parameters, gradients, optimizer states, and
activations. In the LLaMA 3 8B example, either cite the source and benchmark
setup for the hardware, duration, model sizes, and accuracies or label all
figures as illustrative estimates and state their assumptions.
- Around line 24-25: Update the “Example size” table entry for the generative
models column to remove the unofficial “1.7T (GPT-4 estimated)” GPT-4 parameter
count, replacing it with a parameter size from a model with a known published
count or ending the range at 125M (GPT-2). Keep the surrounding model examples
and table structure unchanged.
- Around line 420-421: Update the catastrophic-forgetting evaluation guidance in
the fine-tuning assessment section to remove the universal 3–5% performance-drop
rule. In its place, instruct readers to define project-specific release
thresholds based on task and safety requirements, and compare benchmark results
using variance, confidence intervals, and appropriate statistical analysis;
retain the benchmark examples and mitigation guidance.
- Around line 47-57: The LLaMA 3 example presents unsupported SFT and DPO counts
as factual. In the “Training LLaMA 3 70B” example, link the primary Meta source
for the 15-trillion-token and 6.4-million-H100-hour figures, and remove or
explicitly label the 25,000 SFT examples and 100,000 DPO pairs as illustrative,
unverified values.
---
Outside diff comments:
In `@apps/supercode-cli/server/src/cli/ai/tool-executor.ts`:
- Around line 217-233: Fix the iteration handling in the all-empty-results
branch of the tool-execution loop so the sentinel message is followed by one
additional streamText iteration. Update the assignment involving iter near the
allToolResults.every and isEmptyToolResult checks to account for the loop’s
automatic iter++ and avoid terminating when the loop condition is next
evaluated.
---
Nitpick comments:
In `@apps/supercode-cli/server/src/cli/ai/provider.ts`:
- Around line 77-80: Update the sendMessage adapter’s onStepBudget handling to
avoid duplicating the literal 8 in provider.ts. Reuse ServerProxyService’s
shared MAX_STEPS value when invoking onStepBudget, or remove this callback
wiring until ServerProxyService.sendMessage supports propagating the budget;
keep the existing svc.sendMessage invocation unchanged.
In `@apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts`:
- Around line 239-276: Update the loop containing the repetition and denial
sentinel checks to terminate early when either condition is detected, matching
the stop behavior of tool-executor.ts methods stopForRepetition and
stopForDenialLoop. Set the existing loop-stop mechanism and break or return
after handling the condition, while preserving the notices in currentMessages.
In `@apps/supercode-cli/server/src/index.ts`:
- Around line 381-384: Extract the repeated reasoning selection into a shared
extractReasoningChunk helper and replace each provider loop’s inline
reasoning_content/reasoning logic with it, including all listed streaming
branches. Extract the duplicated fallback-content selection into
pickFallbackContent and use it in the concentrateai and supercode fallback
paths, preserving the current precedence and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffd5d9c5-e8fd-49ad-91df-3e60756a6654
📒 Files selected for processing (12)
.gitignoreapps/supercode-cli/server/package.jsonapps/supercode-cli/server/src/cli/ai/provider.tsapps/supercode-cli/server/src/cli/ai/server-proxy-service.tsapps/supercode-cli/server/src/cli/ai/tool-executor.tsapps/supercode-cli/server/src/cli/commands/slashCommands/model.tsapps/supercode-cli/server/src/cli/workspace/context.tsapps/supercode-cli/server/src/index.tsskills-lock.jsontechy/06-transformers-llm-finetuning.mdtechy/concept-reference.mdtechy/index.md
| // Validate tool args against the Zod inputSchema before executing. | ||
| // The remote AI may return tool calls with missing or malformed args | ||
| // (e.g. run_command with no "command" field), which would produce | ||
| // confusing output like "$ undefined" and near-instant failures. | ||
| let validated = call.args | ||
| if (toolFn.inputSchema) { | ||
| const parsed = (toolFn.inputSchema as any).safeParse(call.args) | ||
| if (parsed?.success) { | ||
| validated = parsed.data | ||
| } else { | ||
| toolResult = JSON.stringify({ | ||
| error: `Invalid arguments for ${call.toolName}`, | ||
| issues: parsed?.error?.issues || [], | ||
| received: call.args, | ||
| }) | ||
| stepResults.push({ toolName: call.toolName, args: call.args, result: toolResult }) | ||
| if (onToolResult) { | ||
| onToolResult({ toolName: call.toolName, args: call.args, result: toolResult }) | ||
| } | ||
| // Skip execution — send the validation error back to the AI | ||
| currentMessages.push({ | ||
| role: "assistant", | ||
| content: null, | ||
| tool_calls: [ | ||
| { | ||
| id: call.toolCallId, | ||
| type: "function", | ||
| function: { name: call.toolName, arguments: JSON.stringify(call.args) }, | ||
| }, | ||
| ], | ||
| } as any) | ||
| currentMessages.push({ | ||
| role: "tool", | ||
| tool_call_id: call.toolCallId, | ||
| content: toolResult, | ||
| } as any) | ||
| this.collectedToolCalls.push(call) | ||
| continue | ||
| } | ||
| } | ||
| try { | ||
| toolResult = await toolFn.execute(call.args) | ||
| toolResult = await toolFn.execute(validated) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validation-error tool result doesn't match the codebase's structured envelope contract, and the push logic is duplicated.
The validation-failure result is JSON.stringify({ error, issues, received }) — it omits success: false. isEmptyToolResult (tool-result.ts) explicitly checks parsed.success === false to treat failure envelopes as non-empty; without that field this object falls through to the trimmed.length < 20 fallback heuristic, relying on incidental string length rather than an intentional contract match. tool-executor.ts's equivalent catch block already includes success: false — this new code should match that convention.
Separately, the message-push block in this branch (assistant tool_call + tool result push, lines 313-329) duplicates the normal-path push block below (lines 349-369) almost line-for-line.
🛡️ Proposed fix for the envelope shape
toolResult = JSON.stringify({
+ success: false,
error: `Invalid arguments for ${call.toolName}`,
issues: parsed?.error?.issues || [],
received: call.args,
})Consider extracting a small pushToolCallMessages(currentMessages, call, toolResult) helper to remove the duplicated push logic between the two branches.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Validate tool args against the Zod inputSchema before executing. | |
| // The remote AI may return tool calls with missing or malformed args | |
| // (e.g. run_command with no "command" field), which would produce | |
| // confusing output like "$ undefined" and near-instant failures. | |
| let validated = call.args | |
| if (toolFn.inputSchema) { | |
| const parsed = (toolFn.inputSchema as any).safeParse(call.args) | |
| if (parsed?.success) { | |
| validated = parsed.data | |
| } else { | |
| toolResult = JSON.stringify({ | |
| error: `Invalid arguments for ${call.toolName}`, | |
| issues: parsed?.error?.issues || [], | |
| received: call.args, | |
| }) | |
| stepResults.push({ toolName: call.toolName, args: call.args, result: toolResult }) | |
| if (onToolResult) { | |
| onToolResult({ toolName: call.toolName, args: call.args, result: toolResult }) | |
| } | |
| // Skip execution — send the validation error back to the AI | |
| currentMessages.push({ | |
| role: "assistant", | |
| content: null, | |
| tool_calls: [ | |
| { | |
| id: call.toolCallId, | |
| type: "function", | |
| function: { name: call.toolName, arguments: JSON.stringify(call.args) }, | |
| }, | |
| ], | |
| } as any) | |
| currentMessages.push({ | |
| role: "tool", | |
| tool_call_id: call.toolCallId, | |
| content: toolResult, | |
| } as any) | |
| this.collectedToolCalls.push(call) | |
| continue | |
| } | |
| } | |
| try { | |
| toolResult = await toolFn.execute(call.args) | |
| toolResult = await toolFn.execute(validated) | |
| toolResult = JSON.stringify({ | |
| success: false, | |
| error: `Invalid arguments for ${call.toolName}`, | |
| issues: parsed?.error?.issues || [], | |
| received: call.args, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts` around lines
293 - 334, Update the validation-failure envelope in the tool execution flow
around toolFn.inputSchema.safeParse to include success: false, matching the
structured error contract used by the existing tool-executor catch path. Extract
the duplicated assistant tool-call and tool-result message appends into a
pushToolCallMessages helper, then reuse it in both the validation-error branch
and normal execution path while preserving their existing result handling.
| // Track per-step results for empty-result sentinel (next iteration). | ||
| seenStepResults.length = 0 | ||
| for (const sr of stepResults) { | ||
| seenStepResults.push({ toolName: sr.toolName, result: sr.result }) | ||
| if (isDeniedToolResult(sr.result)) { | ||
| const prev = deniedCounts.get(sr.toolName) ?? 0 | ||
| deniedCounts.set(sr.toolName, prev + 1) | ||
| } else { | ||
| deniedCounts.set(sr.toolName, 0) | ||
| } | ||
| } | ||
| // Track tool call repetition. | ||
| for (const call of result.toolCalls) { | ||
| const argsKey = JSON.stringify(call.args, Object.keys(call.args).sort()) | ||
| toolCallHistory.push({ toolName: call.toolName, argsKey }) | ||
| } | ||
| if (toolCallHistory.length > 12) { | ||
| toolCallHistory.splice(0, toolCallHistory.length - 12) | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Unguarded Object.keys(call.args) can throw if a tool call arrives with null/undefined args.
call.args comes from event.args in request() with no default. Object.keys(null) / Object.keys(undefined) throws a TypeError, which would propagate out of this async loop uncaught. tool-executor.ts guards the equivalent spot by defaulting args to {} (tc.args || (tc as any).input || {}, line 114) — this file lacks that same defensive default before calling Object.keys.
🐛 Proposed fix
for (const call of result.toolCalls) {
- const argsKey = JSON.stringify(call.args, Object.keys(call.args).sort())
+ const safeArgs = call.args ?? {}
+ const argsKey = JSON.stringify(safeArgs, Object.keys(safeArgs).sort())
toolCallHistory.push({ toolName: call.toolName, argsKey })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Track per-step results for empty-result sentinel (next iteration). | |
| seenStepResults.length = 0 | |
| for (const sr of stepResults) { | |
| seenStepResults.push({ toolName: sr.toolName, result: sr.result }) | |
| if (isDeniedToolResult(sr.result)) { | |
| const prev = deniedCounts.get(sr.toolName) ?? 0 | |
| deniedCounts.set(sr.toolName, prev + 1) | |
| } else { | |
| deniedCounts.set(sr.toolName, 0) | |
| } | |
| } | |
| // Track tool call repetition. | |
| for (const call of result.toolCalls) { | |
| const argsKey = JSON.stringify(call.args, Object.keys(call.args).sort()) | |
| toolCallHistory.push({ toolName: call.toolName, argsKey }) | |
| } | |
| if (toolCallHistory.length > 12) { | |
| toolCallHistory.splice(0, toolCallHistory.length - 12) | |
| } | |
| // Track per-step results for empty-result sentinel (next iteration). | |
| seenStepResults.length = 0 | |
| for (const sr of stepResults) { | |
| seenStepResults.push({ toolName: sr.toolName, result: sr.result }) | |
| if (isDeniedToolResult(sr.result)) { | |
| const prev = deniedCounts.get(sr.toolName) ?? 0 | |
| deniedCounts.set(sr.toolName, prev + 1) | |
| } else { | |
| deniedCounts.set(sr.toolName, 0) | |
| } | |
| } | |
| // Track tool call repetition. | |
| for (const call of result.toolCalls) { | |
| const safeArgs = call.args ?? {} | |
| const argsKey = JSON.stringify(safeArgs, Object.keys(safeArgs).sort()) | |
| toolCallHistory.push({ toolName: call.toolName, argsKey }) | |
| } | |
| if (toolCallHistory.length > 12) { | |
| toolCallHistory.splice(0, toolCallHistory.length - 12) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts` around lines
374 - 393, Guard the tool-call argument handling in the tool call repetition
loop by defaulting null or undefined call.args to an empty object before passing
it to Object.keys and JSON.stringify. Update the logic around toolCallHistory
and preserve the existing toolName/argsKey tracking behavior for valid
arguments.
| // If the loop exhausted its budget but tools ran and no text was produced, | ||
| // make one final non-streaming request asking the model to summarize what | ||
| // it found so far. Without this, the user sees "no analysis" despite the | ||
| // model having gathered useful information. | ||
| if (!accumulatedContent.trim() && this.collectedToolCalls.length > 0) { | ||
| const finalMessages = [ | ||
| ...currentMessages, | ||
| { | ||
| role: "system" as const, | ||
| content: | ||
| "SYSTEM NOTICE: You have used all available tool-call rounds without producing a text response. " + | ||
| "You MUST now produce a text summary based on the tool results you already have. " + | ||
| "Do not call any more tools. Analyze what you found and respond to the user.", | ||
| }, | ||
| ] | ||
| try { | ||
| const finalResult = await this.request(finalMessages, undefined, onChunk, undefined, signal, onReasoning) | ||
| if (finalResult.content.trim()) { | ||
| accumulatedContent = finalResult.content | ||
| finishReason = finalResult.finishReason | ||
| usage = finalResult.usage | ||
| } | ||
| } catch (err: any) { | ||
| // Don't mask a user abort as a fallback message. | ||
| if (err?.name === "AbortError" || signal?.aborted) throw err | ||
| // Surface the failure instead of silently returning empty content | ||
| // (which would otherwise show as "no analysis text returned"). | ||
| const count = this.collectedToolCalls.length | ||
| const detail = err?.message ? `: ${err.message}` : "" | ||
| const fallback = `(Ran ${count} tool call${count === 1 ? "" : "s"} but couldn't generate a final summary${detail}.)` | ||
| onChunk?.(fallback) | ||
| accumulatedContent = fallback | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Final-summary fallback overwrites usage instead of accumulating it.
usage = finalResult.usage (and the pre-existing usage = result.usage per-loop-iteration) replaces the running total with just the last request's usage rather than summing across steps — this new fallback compounds it by overwriting once more with a tiny summarization request's usage. For any multi-round tool conversation, the reported token usage understates the real cost. tool-executor.ts's loop explicitly accumulates (accumulatedUsage.inputTokens = accumulatedUsage.inputTokens + ..., lines 236-245); this loop should do the same instead of assigning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/supercode-cli/server/src/cli/ai/server-proxy-service.ts` around lines
404 - 438, Update usage handling in the surrounding tool-call loop and
final-summary fallback so each request’s usage is added to the running total
rather than assigned over it. Replace the per-iteration and finalResult usage
assignments with field-wise accumulation matching tool-executor.ts, while
preserving existing values when a request omits usage.
| lines.push("## Tool Call Budget") | ||
| lines.push("") | ||
| lines.push("You have a limited number of tool-call rounds per response. Follow these rules:") | ||
| lines.push("") | ||
| lines.push("1. **Stop when you have enough.** Once you have the information needed to answer,") | ||
| lines.push(" produce a text response and stop calling tools. Do not keep gathering more") | ||
| lines.push(' context "just in case" \u2014 every extra round wastes the user\u2019s time.') | ||
| lines.push("") | ||
| lines.push("2. **Prefer multi-tool steps.** Use concurrent tool calls within a single round") | ||
| lines.push(" instead of sequential single-tool rounds. For example, read three files at") | ||
| lines.push(" once, then analyze them \u2014 don\u2019t read one file per round.") | ||
| lines.push("") | ||
| lines.push("3. **Max 5 rounds of tool calls.** After 5 rounds of calling tools, you MUST") | ||
| lines.push(" produce a text response with what you have, even if you wanted more context.") | ||
| lines.push(" Do not exceed 5 rounds. If you hit the limit, summarize what you found and") | ||
| lines.push(" what remaining gaps could be explored further.") | ||
| lines.push("") | ||
| lines.push("4. **If every tool returned empty/error, say so and stop.** Do not retry the") | ||
| lines.push(" same tool with slightly different args hoping for a different result. Report") | ||
| lines.push(" what failed and offer next steps.") | ||
| lines.push("") | ||
| lines.push("5. **No empty rounds.** Every tool call round should make progress. If you find") | ||
| lines.push(" yourself calling a tool just to confirm something you already know, skip it") | ||
| lines.push(" and respond instead.") | ||
| lines.push("") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no caller overrides maxIterations to 5, and check for any other "5 rounds"-related constant.
rg -n 'executeToolLoop\(' apps/supercode-cli/server/src -A3 -B3
rg -n 'maxIterations' apps/supercode-cli/server/src
rg -n 'MAX_STEPS' apps/supercode-cli/server/srcRepository: yashdev9274/supercli
Length of output: 2679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== context.ts relevant section =="
sed -n '90,138p' apps/supercode-cli/server/src/cli/workspace/context.ts
echo
echo "== executeToolLoop signature/implement loop guard =="
sed -n '29,60p' apps/supercode-cli/server/src/cli/ai/tool-executor.ts
sed -n '200,238p' apps/supercode-cli/server/src/cli/ai/tool-executor.ts
echo
echo "== minimax-service call sites =="
sed -n '36,54p' apps/supercode-cli/server/src/cli/ai/minimax-service.ts
echo
echo "== provider references =="
rg -n 'onStepBudget|executeToolLoop|MAX_STEPS|DEFAULT_MAX_STEPS' apps/supercode-cli/server/src/cli/ai -C 2
echo
echo "== repository-wide exact maxIterations count =="
rg -n '\bmaxIterations\b' .Repository: yashdev9274/supercli
Length of output: 16601
Align the tool-call budget guidance with the enforced step limit.
The prompt tells the model it has a maximum of 5 tool-call rounds, but the actual loop limit in executeToolLoop is 8. Update the guidance to match the enforced limit or lower the runtime default/config so the prompt is accurate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/supercode-cli/server/src/cli/workspace/context.ts` around lines 108 -
132, The tool-call budget text in the context prompt conflicts with the limit
enforced by executeToolLoop. Align the “Max 5 rounds” guidance and related
wording with the actual configured limit, or update the runtime
default/configuration to enforce five rounds; ensure the prompt and
executeToolLoop use the same maximum.
| | Example size | 110M (BERT-base) to 340M (BERT-large) | 125M (GPT-2) to 1.7T (GPT-4 estimated) | 220M (T5-base) to 11B (T5-XXL) | | ||
| | Representative models | BERT, RoBERTa, ELECTRA, DeBERTa | GPT-4, LLaMA 3, Mistral, Claude | T5, BART, mT5, MarianMT | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)06-transformers-llm-finetuning\.md$' || true
echo "== relevant lines =="
if [ -f techy/06-transformers-llm-finetuning.md ]; then
sed -n '1,60p' techy/06-transformers-llm-finetuning.md | cat -n
fi
echo "== occurrences of 1.7T / GPT-4 / parameter in techy md =="
rg -n "1\.7T|GPT-4|1\.7\s*T|parameter|parameters" techy || trueRepository: yashdev9274/supercli
Length of output: 50377
🌐 Web query:
OpenAI GPT-4 parameters 1.7T estimated published model size
💡 Result:
The estimate that OpenAI's GPT-4 contains approximately 1.7 to 1.8 trillion parameters is based on unofficial reports and industry speculation, rather than confirmed technical data from OpenAI [1][2][3][4]. In its official technical report, OpenAI explicitly declined to disclose details regarding GPT-4's model architecture, including the total number of parameters, citing competitive landscape and safety implications [5][6]. The 1.7T to 1.8T figure originated from third-party analysis and reports shortly after the model's release [1][3][7]. These reports often suggested that GPT-4 utilizes a Mixture of Experts (MoE) architecture, potentially consisting of multiple smaller models (e.g., eight models of roughly 220 billion parameters each) to reach that total [1][3]. While this structure would explain how such a large model could remain computationally feasible for inference, it remains unverified by OpenAI [2][4][7]. Therefore, the 1.7 trillion parameter figure should be treated as an industry estimate rather than an official specification [2][4].
Citations:
- 1: https://the-decoder.com/gpt-4-has-a-trillion-parameters/
- 2: https://news.ycombinator.com/item?id=37804839
- 3: https://the-decoder.com/gpt-4-architecture-datasets-costs-and-more-leaked/
- 4: https://levelup.gitconnected.com/gpt-4-parameters-explained-everything-you-need-to-know-e210c20576ca
- 5: https://cdn.openai.com/papers/gpt-4.pdf?itid=lk_inline_enhanced-template
- 6: https://arxiv.org/html/2303.08774v5
- 7: https://newsletter.semianalysis.com/p/gpt-4-architecture-infrastructure
Avoid treating unofficial GPT-4 parameter counts as model-size examples.
Since OpenAI has not published GPT-4’s parameter count, 1.7T (GPT-4 estimated) reads like an official specification in a concise example table. Remove it, or replace it with a known model size.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@techy/06-transformers-llm-finetuning.md` around lines 24 - 25, Update the
“Example size” table entry for the generative models column to remove the
unofficial “1.7T (GPT-4 estimated)” GPT-4 parameter count, replacing it with a
parameter size from a model with a known published count or ending the range at
125M (GPT-2). Keep the surrounding model examples and table structure unchanged.
| **Stage 1: Pre-training.** The model is trained on trillions of tokens from the internet — web pages, books, academic papers, code repositories. The objective is next-token prediction: given all previous tokens, predict the next one. This single objective, applied at enormous scale, forces the model to learn grammar, factual knowledge, reasoning patterns, translation, code syntax, and world modeling. Pre-training is computationally massive — estimated $50-200M for frontier models. No labeled data is needed; the text itself provides the labels. The output is a base model that can complete text but does not follow instructions or answer questions helpfully. Key considerations: data quality filtering, deduplication (MinHash), PII removal, and legal filtering. The Chinchilla scaling law governs optimal allocation of compute between model size and training tokens. | ||
|
|
||
| **Stage 2: Supervised Fine-Tuning (SFT).** The base model can generate text but does not follow instructions — it might complete a prompt rather than answer it. SFT trains the model on curated (instruction, response) pairs collected from humans or high-quality sources. For example: Instruction: "Explain what a transformer is in one sentence." Response: "A transformer is a neural network architecture that processes all tokens in parallel using self-attention, enabling efficient learning of long-range dependencies." The model continues next-token prediction training, but now on instruction-response data instead of raw internet text. After SFT, the model learns the pattern of following instructions, answering questions helpfully, and formatting responses appropriately. SFT typically uses 10K-100K examples and runs for 1-3 epochs. Quality matters far more than quantity — a small set of carefully written examples outperforms large noisy datasets. | ||
|
|
||
| **Stage 3: Preference Optimization.** SFT teaches the model to follow instructions, but does not teach it to prefer good responses over bad ones. Preference optimization aligns the model with human values: helpfulness, honesty, safety. The two main approaches are RLHF (Reinforcement Learning from Human Feedback) and DPO (Direct Preference Optimization). RLHF trains a separate reward model on human preference judgments (response A is better than response B), then uses PPO to optimize the LLM against this reward model. DPO directly optimizes the LLM on preference pairs without a separate reward model, which is simpler and more stable. Either way, the model learns to rank responses and prefer the ones humans find most helpful and least harmful. | ||
|
|
||
| Many models also include an additional **alignment fine-tuning** stage using techniques like constitutional AI (self-critique and revision based on written principles) or red teaming (adversarial testing to find and fix failure modes). The final model after all stages is what users interact with through chat interfaces and APIs. | ||
|
|
||
| ### Example | ||
|
|
||
| Training LLaMA 3 70B: Pre-training on 15 trillion tokens from the internet — 90% web pages (CommonCrawl, filtered), 5% books, 3% academic papers, 2% code. This took approximately 6.4M GPU hours on H100s. SFT used 25,000 high-quality instruction-response pairs written by contractors and in-house experts, covering diverse tasks: writing, analysis, coding, math, creative. DPO used 100,000 preference pairs (response A vs response B) from human raters. The final model demonstrates instruction following, refuses harmful requests, and produces helpful responses. Without any single stage: pre-training only gives a text completer, SFT only cannot teach new knowledge, preference optimization only cannot teach general capability. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="techy/06-transformers-llm-finetuning.md"
if [ -f "$file" ]; then
echo "== file excerpt =="
nl -ba "$file" | sed -n '1,140p'
else
echo "file not found: $file"
echo "== matching files =="
fd -i 'transformers-llm-finetuning|llm.*finetuning|06-transformers' techy || true
fi
echo
echo "== search for related sources/citations in repo =="
rg -n "CommonCrawl|RLHF|DPO|Chinchilla|LLaMA 3|15 trillion|GPU hours|25,000|100,000|50-200M|200M" . \
--glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: yashdev9274/supercli
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="techy/06-transformers-llm-finetuning.md"
if [ -f "$file" ]; then
echo "== file excerpt =="
awk '{printf "%6d\t%s\n", NR, $0}' "$file" | sed -n '1,140p'
else
echo "file not found: $file"
echo "== matching files =="
fd -i 'transformers-llm-finetuning|llm.*finetuning|06-transformers' techy || true
fi
echo
echo "== search for related sources/citations in repo =="
rg -n "CommonCrawl|RLHF|DPO|Chinchilla|LLaMA 3|15 trillion|GPU hours|25,000|100,000|50-200M|200M" . \
--glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: yashdev9274/supercli
Length of output: 50378
🌐 Web query:
LLaMA 3 70B 15 trillion tokens 6.4M GPU hours H100 SFT 25,000 DPO 100,000 preference pairs official
💡 Result:
The technical specifications for the Llama 3 70B model align with the following official data: Pretraining: The Llama 3 70B model was pretrained on over 15 trillion tokens [1][2]. GPU Compute: The pretraining process utilized 6.4 million GPU hours on H100-80GB hardware [1][2][3]. Alignment Process: The instruction-tuned versions (Instruct) were developed using a combination of supervised fine-tuning (SFT) and Direct Preference Optimization (DPO) [4][5]. Regarding the specific numbers for fine-tuning (25,000 SFT examples and 100,000 preference pairs), these figures are not explicitly cited as official Llama 3 70B training parameters in Meta's official model cards, research papers, or blog posts [1][4][2]. Official documentation generally states that fine-tuning involved over 10 million human-annotated examples for the initial Llama 3 release [1][2], and later documentation for Llama 3.3 mentions over 25 million synthetically generated examples [6][7]. While Meta's "The Llama 3 Herd of Models" paper discusses the use of DPO with preference data [4], it does not publish the specific counts of 25,000 or 100,000 as standard Llama 3 70B training set sizes.
Citations:
- 1: https://github.com/meta-llama/llama-models/blob/main/models/llama3/MODEL_CARD.md
- 2: https://huggingface.co/meta-llama/Meta-Llama-3-70B-Instruct
- 3: https://internal.replicate.com/meta/meta-llama-3-70b-instruct/readme
- 4: https://nbogoychev.com/files/publications/2407.21783v2.pdf
- 5: https://huggingface.co/blog/llama3
- 6: https://github.com/meta-llama/llama-models/blob/main/models/llama3_3/MODEL_CARD.md
- 7: https://github.com/meta-llama/llama-models/blob/main/models/llama3%5F3/MODEL%5FCARD.md
Source the LLaMA 3 fine-tuning counts.
The 15T-token pretraining and 6.4M H100-hour numbers match Meta’s LLaMA 3 materials, but the SFT “25,000 examples” and DPO “100,000 preference pairs” LLaMA 3 70B figures are not in the official model card and are broader “typical SFT” ranges elsewhere in the chapter. Link the primary LLaMA 3 source for the pre-training hours, and either remove/adjust the fine-tuning counts or label them as unvetted illustrative values.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~47-~47: Do not mix variants of the same word (‘pre-train’ and ‘pretrain’) within a single text.
Context: ...a, compute, and techniques. Stage 1: Pre-training. The model is trained on trillions of...
(EN_WORD_COHERENCY)
[uncategorized] ~47-~47: Do not mix variants of the same word (‘pre-train’ and ‘pretrain’) within a single text.
Context: ...ation, code syntax, and world modeling. Pre-training is computationally massive — estimated ...
(EN_WORD_COHERENCY)
[uncategorized] ~57-~57: Do not mix variants of the same word (‘pre-train’ and ‘pretrain’) within a single text.
Context: ...Is. ### Example Training LLaMA 3 70B: Pre-training on 15 trillion tokens from the internet...
(EN_WORD_COHERENCY)
[uncategorized] ~57-~57: Do not mix variants of the same word (‘pre-train’ and ‘pretrain’) within a single text.
Context: ...ul responses. Without any single stage: pre-training only gives a text completer, SFT only c...
(EN_WORD_COHERENCY)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@techy/06-transformers-llm-finetuning.md` around lines 47 - 57, The LLaMA 3
example presents unsupported SFT and DPO counts as factual. In the “Training
LLaMA 3 70B” example, link the primary Meta source for the 15-trillion-token and
6.4-million-H100-hour figures, and remove or explicitly label the 25,000 SFT
examples and 100,000 DPO pairs as illustrative, unverified values.
| ``` | ||
| <|begin_of_text|><|start_header_id|>system<|end_header_id|> | ||
| You are a helpful assistant. | ||
| <|eot_id|><|start_header_id|>user<|end_header_id|> | ||
| What is the capital of France?<|eot_id|><|start_header_id|>assistant<|end_header_id|> | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to fenced code blocks.
These four fences trigger Markdownlint MD040. Use text for chat/template examples and text or math for the loss expression.
Also applies to: 154-165, 228-232, 326-328
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 94-94: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@techy/06-transformers-llm-finetuning.md` around lines 94 - 99, Add language
identifiers to every fenced code block in this document, including the
chat/template examples near the shown section and the additional blocks at the
referenced locations. Use text for chat or template examples, and text or math
for the loss expression, while preserving their contents unchanged.
Source: Linters/SAST tools
| | **ChatML** | GPT-4, many open models | <|im_start|>system\n...<|im_end|>\n<|im_start|>user\n...<|im_end|>\n<|im_start|>assistant\n...<|im_end|> | | ||
| | **LLaMA 3** | LLaMA 3, 3.1 | <|start_header_id|>system<|end_header_id|>\n...<|eot_id|><|start_header_id|>user<|end_header_id|>\n...<|eot_id|><|start_header_id|>assistant<|end_header_id|> | | ||
| | **Mistral** | Mistral, Mixtral | [INST] ... [/INST] for user, no special tags for assistant | | ||
| | **Alpaca** | Alpaca, Vicuna | "Below is an instruction...\n\n### Instruction:\n...\n\n### Response:\n" | | ||
| | **Phi** | Phi-3, Phi-4 | <|user|>\n...<|end|>\n<|assistant|>\n...<|end|> | | ||
| | **Gemma** | Gemma 2 | <bos><start_of_turn>user\n...<end_of_turn>\n<start_of_turn>model\n...<end_of_turn> | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Escape literal pipes in the chat-format table.
The | characters inside ChatML, LLaMA, and Phi examples are parsed as table separators, causing MD056 and broken rendering. Escape them as \| or move the examples outside the table.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 139-139: Table column count
Expected: 3; Actual: 15; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 140-140: Table column count
Expected: 3; Actual: 19; Too many cells, extra data will be missing
(MD056, table-column-count)
[warning] 143-143: Table column count
Expected: 3; Actual: 11; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@techy/06-transformers-llm-finetuning.md` around lines 139 - 144, Escape the
literal pipe characters in the ChatML, LLaMA 3, and Phi examples within the
chat-format table so they are rendered as content rather than column separators.
Preserve the existing examples and table structure, ensuring the table passes
MD056.
Source: Linters/SAST tools
| **QLoRA** combines LoRA with 4-bit quantization of the base model. The pre-trained weights are quantized to 4-bit NormalFloat format, reducing memory by 4x (e.g., 70B model from 140GB to 35GB). LoRA adapters are trained in full precision (BF16/FP16) on top of the quantized base. Gradients are computed by dequantizing to BF16 only for the specific weights involved in the current forward/backward pass. QLoRA enables fine-tuning 70B models on a single 48GB GPU — previously impossible even for inference. The quantized model retains approximately 99-99.9% of the original quality. | ||
|
|
||
| **Other PEFT methods:** | ||
| - **Adapters** insert small bottleneck layers between Transformer sub-layers. | ||
| - **Prefix Tuning** prepends learnable virtual tokens to the input (or to each layer's key/value). | ||
| - **Prompt Tuning** adds learnable soft prompt tokens only to the input embedding layer. | ||
| - **P-Tuning v2** adds learnable prompt embeddings to each Transformer layer. | ||
| - **IA³** learns scaling vectors for keys, values, and feedforward activations. | ||
|
|
||
| **Compare PEFT methods:** | ||
|
|
||
| | Method | Trainable params | Memory (7B model) | Performance vs Full FT | When to use | | ||
| |--------|-----------------|-------------------|----------------------|-------------| | ||
| | Full FT | 7B | ~60 GB (BF16) | Baseline | Maximum performance, unlimited compute | | ||
| | LoRA (r=16) | ~33M | ~18 GB | 95-99% | Most tasks, good balance | | ||
| | QLoRA (r=16) | ~33M | ~10 GB | 93-98% | Limited GPU memory | | ||
| | Adapters | ~40M | ~20 GB | 94-98% | When layer-specific adapters needed | | ||
| | Prefix Tuning | ~2M | ~16 GB | 85-95% | Very low memory, generative tasks | | ||
|
|
||
| ### Example | ||
|
|
||
| Fine-tuning LLaMA 3 8B for medical Q&A. Full fine-tuning: requires 4× A100 (80GB) GPUs, 3 days, outputs a 16GB model file. LoRA (r=16): requires 1× A100 (80GB) or 2× RTX 4090 (24GB), 6 hours, outputs a 16MB adapter file. QLoRA: requires 1× RTX 4090 (24GB), 8 hours, outputs a 16MB adapter file. Performance: full FT achieves 87% accuracy on the medical benchmark, LoRA achieves 86%, QLoRA achieves 85.5%. The 1-2% accuracy gap is often acceptable given the 10x+ reduction in compute cost. Multiple QLoRA adapters can be swapped without reloading the base model, enabling a single serving instance to handle multiple fine-tuned variants. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a '06-transformers-llm-finetuning\.md$' . || true
echo "== surrounding lines =="
if [ -f techy/06-transformers-llm-finetuning.md ]; then
sed -n '250,325p' techy/06-transformers-llm-finetuning.md | cat -n
fi
echo "== citations / sources nearby =="
rg -n "citations?|reference|source|benchmark|LLaMA|medical|GPU|A100|RTX 4090|QLoRA|PEFT" techy/06-transformers-llm-finetuning.mdRepository: yashdev9274/supercli
Length of output: 29624
Define and source the memory and performance estimates.
The PEFT memory table lists single numbers without measurement conditions, and full fine-tuning memory depends on parameters, gradients/optimizer states, activations, sequence length, and batch size. The LLaMA 3 8B example also gives exact hardware, duration, output sizes, and benchmark accuracies without indicating whether they are cited results or a single illustrative setup. Add the source/measurement context, or mark them as illustrative.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@techy/06-transformers-llm-finetuning.md` around lines 283 - 304, Add
citations or explicit measurement conditions for the PEFT comparison table,
including model precision, sequence length, batch size, optimizer, and whether
memory is peak GPU memory; clarify how the full fine-tuning estimate accounts
for parameters, gradients, optimizer states, and activations. In the LLaMA 3 8B
example, either cite the source and benchmark setup for the hardware, duration,
model sizes, and accuracies or label all figures as illustrative estimates and
state their assumptions.
| **Q: How do you evaluate whether fine-tuning caused catastrophic forgetting?** | ||
| A: Run the fine-tuned model on standard benchmarks that test general capabilities. Compare scores before and after fine-tuning. Key benchmarks: MMLU (world knowledge across 57 subjects), ARC (science reasoning), GSM8K (math), HumanEval (coding), TruthfulQA (factual accuracy). If performance drops by more than 3-5% on any benchmark, the fine-tuning may be causing significant forgetting. Mitigations: (1) mix general data with task data during fine-tuning (5-10% general data), (2) use a lower learning rate, (3) use LoRA instead of full fine-tuning (LoRA preserves base model capabilities better), (4) use multi-task fine-tuning that includes general tasks alongside the target task. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked matching file:"
git ls-files | grep -F 'techy/06-transformers-llm-finetuning.md' || true
echo
echo "Relevant lines:"
if [ -f techy/06-transformers-llm-finetuning.md ]; then
sed -n '380,445p' techy/06-transformers-llm-finetuning.md | cat -n -v
fi
echo
echo "Search forgetting thresholds/references in file:"
rg -n "forget|benchmark|evaluat|confidence|threshold|standard benchmark" techy/06-transformers-llm-finetuning.md || trueRepository: yashdev9274/supercli
Length of output: 13725
🌐 Web query:
transformers benchmark MMLU HumanEval GSM8K benchmark variance confidence intervals catastrophic forgetting 3% 5%
💡 Result:
In contemporary large language model (LLM) research, reporting single-point accuracy scores without statistical measures of uncertainty is increasingly recognized as insufficient, as benchmarks like MMLU, GSM8K, and HumanEval exhibit significant variance [1][2]. Variance and Confidence Intervals Benchmark variance arises from both finite-sample noise and systematic differences (e.g., prompt variations, implementation details, or inference settings) [1][3]. For many standard benchmarks, 95% confidence intervals (CIs) are necessary to determine if performance differences between models are statistically significant [4][5][2]. Research indicates that naive reporting often overstates confidence; for instance, synthesized estimates of model performance across multiple papers show that median 95% CI half-widths are often substantial (e.g., ~1.7 points or higher), and gaps between leaderboard entries frequently fall within these margins [1][2]. Central Limit Theorem-based methods are common but can be unreliable for smaller, specialized benchmarks; alternative frequentist or Bayesian approaches are recommended for robust quantification [4][5][6]. Catastrophic Forgetting and Thresholds Catastrophic forgetting—the significant loss of performance on previously learned tasks after training on new ones—is highly sensitive to the metrics and thresholds used [7][8]. While 3% and 5% are sometimes cited as informal thresholds for performance degradation or acceptable "forgetting" in specific LoRA or fine-tuning contexts, there is no universal industry-standard percentage [9]. These thresholds are highly domain-dependent and task-specific [9]. Practitioners are advised to: 1. Establish a baseline on representative samples (ideally 500–1000 examples) of the original distribution before new training [9]. 2. Measure degradation (the "forgetting metric") as the delta between pre-training and post-fine-tuning performance [9]. 3. Treat thresholds like 3% or 5% as guidance rather than absolute rules, adjusting them based on the specific requirements of the downstream task [9]. Evaluation Recommendations To ensure rigorous evaluation, it is recommended to: - Use paired-difference analysis, which can reduce variance by leveraging the fact that models often succeed or fail on the same questions [4][5]. - Report confidence intervals (e.g., 95% CIs) or standard errors (SEM) for all results [4][5][2]. - Account for multiple comparisons when ranking multiple models to avoid false-positive claims of superiority [2]. - Use established tools (e.g., evalci) for automated, statistically rigorous comparisons [2].
Citations:
- 1: https://clawrxiv.io/abs/2604.01984
- 2: https://arxiv.org/html/2607.04429
- 3: https://arxiv.org/html/2607.16259
- 4: https://arxiv.org/html/2411.00640v1
- 5: https://www.anthropic.com/research/statistical-approach-to-model-evals
- 6: https://arxiv.org/pdf/2503.01747
- 7: https://arxiv.org/abs/2102.07686v3
- 8: https://ar5iv.labs.arxiv.org/html/2205.13452
- 9: https://theneuralbase.com/lora-fundamentals/learn/intermediate/catastrophic-forgetting-evaluation/
Avoid a universal 3–5% forgetting threshold.
Benchmark scores have variance and confidence intervals, and what counts as unacceptable forgetting depends on the released task/safety requirements. Replace this fixed rule with project-specific release thresholds and statistical comparison guidance instead of judging catastrophic forgetting by a single percentage drop.
🧰 Tools
🪛 LanguageTool
[style] ~421-~421: Consider a different verb to strengthen your wording.
Context: ...ulQA (factual accuracy). If performance drops by more than 3-5% on any benchmark, the...
(DROP_DECLINE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@techy/06-transformers-llm-finetuning.md` around lines 420 - 421, Update the
catastrophic-forgetting evaluation guidance in the fine-tuning assessment
section to remove the universal 3–5% performance-drop rule. In its place,
instruct readers to define project-specific release thresholds based on task and
safety requirements, and compare benchmark results using variance, confidence
intervals, and appropriate statistical analysis; retain the benchmark examples
and mitigation guidance.
Description
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
bun testpassesbun run typecheckpassesbun run lintpasses (if applicable)Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation