Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
6 changes: 4 additions & 2 deletions packages/kilo-ui/src/components/diff.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,8 @@ export function Diff<T>(props: DiffProps<T>) {
containerWrapper: container,
})
} else {
const oldFile = local.before!
const newFile = local.after!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Non-null assertions removed the last runtime guard for malformed Diff props

This replaces the throwing invariant with bare ! assertions and drops the runtime check entirely. The type-level reasoning is sound (local.fileDiff falsy + patchDiff() falsy implies the DiffPair variant, where before/after are required), but it only holds if every caller actually satisfies the discriminated union at runtime. Diff is a shared kilo-ui component consumed across the webview, so a caller bug that supplies neither fileDiff nor before/after is exactly the scenario the original SUGGESTION comment on this line called out.

With the guard gone, that misuse is no longer visible: { ...oldFile, contents, cacheKey } with oldFile undefined silently spreads to nothing (object spread of undefined doesn't throw), producing a oldFile/newFile missing required fields like path, which gets handed to instance.render(...) after container.innerHTML has already been cleared and instance reassigned. That reintroduces a stuck/broken diff view with no error surfaced anywhere — the same failure mode flagged before the throw was added, just without even a console error this time.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushing back on this one: the malformed-props behavior is unchanged from main, not introduced by this PR. The base code already executes { ...local.before, ... } / { ...local.after, ... } with no runtime guard. The final patch only assigns those same values to locals with non-null assertions before spreading them, because splitProps loses the DiffProps discriminated-union narrowing. Non-null assertions emit no JavaScript, so the runtime behavior is identical. Adding runtime validation or an error-reporting API would be a separate behavioral change outside this typecheck cleanup. The reviewed commit also passed the complete CI suite, including typecheck and both visual-regression jobs.

const beforeContents = before()
const afterContents = after()

Expand All @@ -749,8 +751,8 @@ export function Diff<T>(props: DiffProps<T>) {
}

instance.render({
oldFile: { ...local.before, contents: beforeContents, cacheKey: cacheKey(beforeContents) },
newFile: { ...local.after, contents: afterContents, cacheKey: cacheKey(afterContents) },
oldFile: { ...oldFile, contents: beforeContents, cacheKey: cacheKey(beforeContents) },
newFile: { ...newFile, contents: afterContents, cacheKey: cacheKey(afterContents) },
lineAnnotations: annotations,
containerWrapper: container,
})
Expand Down
1 change: 1 addition & 0 deletions packages/kilo-vscode/knip.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"webview-ui/kiloclaw/index.tsx",
"webview-ui/marketplace/index.tsx",
"webview-ui/pierre-worker.ts",
"webview-ui/src/assets.d.ts",
"webview-ui/src/index.tsx",
"src/**/__tests__/**/*.{ts,spec.ts}",
"src/**/*.test.ts",
Expand Down
2 changes: 1 addition & 1 deletion packages/kilo-vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1167,7 +1167,7 @@
"watch-tests": "tsc -p . -w --outDir out",
"pretest": "bun run compile-tests && bun run compile && bun run lint",
"check-types": "tsc --noEmit",
"check-types:webview": "bun script/typecheck.ts --project webview-ui/tsconfig.json",
"check-types:webview": "tsc --noEmit --project webview-ui/tsconfig.json",
"typecheck": "bun run check-types && bun run check-types:webview",
"format": "prettier --write .",
"format:check": "prettier --check .",
Expand Down
38 changes: 0 additions & 38 deletions packages/kilo-vscode/script/typecheck.ts

This file was deleted.

9 changes: 9 additions & 0 deletions packages/kilo-vscode/webview-ui/src/assets.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
declare module "*.svg" {
const src: string
export default src
}

declare module "*?worker&url" {
const src: string
export default src
}
Loading