Skip to content

SCRUM-141 + fixed algo bugs#36

Merged
livia-cutra merged 2 commits intomainfrom
file-discovery-bugs
Mar 25, 2026
Merged

SCRUM-141 + fixed algo bugs#36
livia-cutra merged 2 commits intomainfrom
file-discovery-bugs

Conversation

@livia-cutra
Copy link
Copy Markdown
Collaborator

Fixed file discovery issues and multi-file coverage. I also found an algorithm rendering bug and fixed that.

Comment thread src/JavaFileWatcher.ts
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Now have webview manager handling webviews getting added and messages being sent to them

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves Codescape’s VS Code extension state synchronization between the backend parse store and the webviews, aiming to fix initial load “dropped messages”, support multi-file coverage, and adjust the frontend rendering/layout logic.

Changes:

  • Send a backend “FULL_STATE” snapshot to webviews after a READY handshake (panel + sidebar), and introduce a broadcast() helper on JavaFileWatcher.
  • Update the panel/sidebar providers to receive FileParseStore and trigger full-state sync on webview readiness.
  • Update the webview renderer logic to support incremental updates (PARTIAL_STATE), dependency-based layout, depth-sorted rendering, and hover interactions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File Description
src/extension.ts Adds full-state sync + READY handshake, wires store into webviews, and updates the embedded webview rendering/patching logic.
src/JavaFileWatcher.ts Adds a broadcast() helper for sending messages to all registered webviews.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/extension.ts Outdated
});
}

console.log("CREATE PANEL CALLED");
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

console.log("CREATE PANEL CALLED") is at module top-level, so it will run on extension load even if the command is never invoked. This looks like leftover debug logging; consider removing it or moving it inside createPanel() behind a debug/trace flag to avoid noisy logs for all users.

Suggested change
console.log("CREATE PANEL CALLED");

Copilot uses AI. Check for mistakes.
Comment thread src/extension.ts
Comment on lines +693 to +706
if (msg.type === 'FULL_STATE' && msg.payload) {
console.log("CLASSES:", msg.payload.classes);
console.log('[FULL_STATE] received:', msg.payload);

state.classes = msg.payload.classes;

//build graph input
const nodes = buildNodesFromClasses(state.classes);

// run algorithm
state.layout = computeLayout(nodes);

assignColors();
render();
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

In the webview FULL_STATE handler, state.status is never updated after assigning state.classes. Since the initial status is "loading", render() will keep showing the loading UI and return early. Set state.status to "empty" or "ready" based on state.classes.length (and optionally handle an error case) before calling render().

Copilot uses AI. Check for mistakes.
Comment thread src/extension.ts
Comment on lines +415 to +420
function patchState({ changed = [], related = [], removed = [] }) {
console.log("patchState called");

const nodes = buildNodesFromClasses(state.classes);
state.layout = computeLayout(nodes);

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

patchState() computes state.layout from state.classes before applying removed/changed/related updates, so the layout can become stale (missing new classes, including removed ones). Apply the class list updates first, then rebuild nodes and recompute the layout from the updated state.classes.

Copilot uses AI. Check for mistakes.
Comment thread src/extension.ts
Comment on lines +455 to +465
return classes.map(cls => {
const neighbors = [];

//extract dependencies from fields
if (cls.Fields) {
cls.Fields.forEach(field => {
const type = field.type;

//only include if it's another class in the project
if (classNames.has(type)) {
neighbors.push(type);
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

buildNodesFromClasses() compares field.type directly against known class names. Parser field types can include generics/arrays (e.g. List<Foo>/Foo[]), so these dependencies won’t be detected and neighbors will be incomplete. Normalize field.type (strip generic/array suffixes similar to relations.ts) before checking classNames.has(...).

Suggested change
return classes.map(cls => {
const neighbors = [];
//extract dependencies from fields
if (cls.Fields) {
cls.Fields.forEach(field => {
const type = field.type;
//only include if it's another class in the project
if (classNames.has(type)) {
neighbors.push(type);
// Normalize a raw field type by stripping array and generic suffixes.
const normalizeType = (rawType: any): string | null => {
if (typeof rawType !== "string") {
return null;
}
let t = rawType.trim();
// Remove any number of trailing array brackets, e.g. Foo[][] -> Foo
while (t.endsWith("[]")) {
t = t.slice(0, -2).trim();
}
// Remove a trailing generic part, e.g. List<Foo> -> List
const genericStart = t.indexOf("<");
if (genericStart !== -1 && t.endsWith(">")) {
t = t.substring(0, genericStart).trim();
}
return t;
};
return classes.map(cls => {
const neighbors = [];
//extract dependencies from fields
if (cls.Fields) {
cls.Fields.forEach(field => {
const normalizedType = normalizeType(field.type);
//only include if it's another class in the project
if (normalizedType && classNames.has(normalizedType)) {
neighbors.push(normalizedType);

Copilot uses AI. Check for mistakes.
Comment thread src/extension.ts Outdated
Comment on lines +592 to +615
const isoX = (position.col - position.row) * TILE_L / 2 + offsetX;
const isoY = (position.col + position.row) * TILE_L / 4 + offsetY;

placeIsoBuilding(
ctx,
position.col,
position.row,
floors,
state.colors[cls.Classname] || "#598BAF",
TILE_L,
offsetX,
offsetY
);

const width = TILE_L;
const height = floors * TILE_L / 2;

buildingRegistry.push({
className: cls.Classname,
x: isoX - width / 2,
y: isoY - height,
width: width,
height: height
});
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

The hover hitbox Y-coordinate is computed from isoY, but placeIsoBuilding() renders the building with baseY = isoY + TILE_L / 2 (see renderer.js). This offset means buildingRegistry rectangles won’t line up with the drawn buildings, making hover detection inaccurate. Derive the registry box from the same baseY used for rendering.

Copilot uses AI. Check for mistakes.
Comment thread src/extension.ts Outdated
Comment on lines +617 to +618
console.log("buildingRegistry:", buildingRegistry);

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

console.log("buildingRegistry:", buildingRegistry) runs once per class on every render, which can severely degrade canvas performance and spam logs as projects grow. Remove this or gate it behind an explicit debug flag.

Copilot uses AI. Check for mistakes.
Comment thread src/JavaFileWatcher.ts Outdated
Comment on lines +87 to +97
broadcast(message: any) {
if (this._webviews.length === 0) {
console.log("[broadcast] no webviews yet");
return;
}

for (const v of this._webviews) {
v.postMessage(message).then(delivered => {
console.log("[broadcast] delivered:", delivered);
});
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

broadcast() calls postMessage(...).then(...) without handling rejections. If a webview is disposed or postMessage fails, this can create unhandled promise rejections and leave dead webviews in _webviews. Add a .catch(...) (and optionally remove the failing webview from the list) to keep broadcasts reliable.

Copilot uses AI. Check for mistakes.
Comment thread src/JavaFileWatcher.ts Outdated
Comment on lines +87 to +88
broadcast(message: any) {
if (this._webviews.length === 0) {
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

broadcast(message: any) loses type safety and makes it easy to send malformed messages. Consider typing this parameter to a shared message contract (e.g. a WebviewMessage union) or at least unknown/Record<string, unknown> with runtime validation.

Copilot uses AI. Check for mistakes.
@livia-cutra livia-cutra merged commit e431159 into main Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants