Skip to content

Add RSA settings - #15

Open
shashankk42 wants to merge 2 commits into
FLASK-LLNL:mainfrom
shashankk42:feature/rsa-settings
Open

Add RSA settings#15
shashankk42 wants to merge 2 commits into
FLASK-LLNL:mainfrom
shashankk42:feature/rsa-settings

Conversation

@shashankk42

@shashankk42 shashankk42 commented Apr 3, 2026

Copy link
Copy Markdown

Summary

Add RsaSettingsPanel: a reusable UI component for configuring Recursive Self-Aggregation (RSA) parameters. Imported by
FLASK-Copilot's retrosynthesis tab.

Changes

  • lcc_ui_components/src/RsaSettingsPanel.tsx: new component: checkbox to enable RSA, mode selector (standalone / RAG), and
    validated N / K / T inputs (each ≥ 2, with K ≤ N)
  • lcc_ui_components/src/index.ts — export the new component

Related PRs

@tbennun tbennun left a comment

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.

A few unnecessary changes. This PR should be redone in the context of the FLASK-Copilot PR

Comment thread lc_conductor/backend_helper_function.py Outdated
Comment thread lc_conductor/backend_manager.py Outdated
Comment on lines +106 to +107
except (RuntimeError, Exception) as send_error:
logger.debug(

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.

Isn't RuntimeError a subclass of Exception?

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.

Not resolved

Comment thread lc_conductor/backend_manager.py Outdated
)
except (RuntimeError, Exception) as send_error:
logger.debug(
f"Could not send error to WebSocket (likely closed): {send_error}"

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.

also, this should be logger.exception

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.

Not resolved...

Comment thread lc_conductor/backend_manager.py Outdated
Comment on lines +114 to +117
except (RuntimeError, Exception) as send_error:
logger.debug(
f"Could not send complete to WebSocket (likely closed): {send_error}"
)

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.

I don't see a reason to change any of these lines

Comment thread lc_conductor/callback_logger.py Outdated
"message": {"source": source, "message": msg, **kwargs},
}
)
except (RuntimeError, Exception):

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.

again, the two might not be necessary

Comment thread lc_conductor/callback_logger.py Outdated
Comment on lines +99 to +103
except (RuntimeError, Exception) as e:
# WebSocket is closed or disconnected - log to console but don't raise
logger.debug(
f"Could not send message to WebSocket (likely closed): {str(e)}"
)

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.

logger.exception and one Exception please

Comment thread lc_conductor/callback_logger.py Outdated
Comment on lines +104 to +105
# Unbind the websocket so we don't keep trying to send to it
self.websocket = None

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.

This is a bad assumption that should be removed

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.

Looks like this is a duplicate of the FLASK-Copilot settings panel. I think this one is the only one that should exist, so the other one should be removed.

Comment on lines +51 to +55
useRsa?: boolean;
rsaMode?: 'standalone' | 'rag';
rsaN?: number;
rsaK?: number;
rsaT?: number;

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.

This means that the type changes in FLASK-Copilot are unnecessary

@bvanessen
bvanessen force-pushed the feature/rsa-settings branch from f140619 to b8d712f Compare May 12, 2026 21:04
@shashankk42
shashankk42 force-pushed the feature/rsa-settings branch from c3e5b2d to d022c5d Compare May 19, 2026 21:50
@shashankk42 shashankk42 changed the title Add RSA settings and AI-based retrosynthesis flag to RunSettings Add RSA settings May 19, 2026
@shashankk42
shashankk42 force-pushed the feature/rsa-settings branch from 89a7b13 to b1416f6 Compare June 1, 2026 02:48

@tbennun tbennun left a comment

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.

Too many unrelated changes

Comment thread lc_conductor/backend_helper_function.py Outdated
Comment thread lc_conductor/backend_manager.py Outdated
Comment on lines +106 to +107
except (RuntimeError, Exception) as send_error:
logger.debug(

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.

Not resolved

Comment thread lc_conductor/backend_manager.py Outdated
)
except (RuntimeError, Exception) as send_error:
logger.debug(
f"Could not send error to WebSocket (likely closed): {send_error}"

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.

Not resolved...

tb = "".join(traceback.format_exception(exc))
msg = f"Background task failed with exception: {type(exc).__name__}: {tb}"
logger.error(msg)
await self.websocket.send_json(

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.

Unrelated changes to the PR should be reverted and uploaded as a separate PR with sufficient reasoning and or a failing case.

Comment thread .eslintrc.json Outdated
Comment on lines +8 to +32
@@ -19,6 +24,11 @@
"@typescript-eslint/semi": "warn", // Changes "semi" rule violations to warnings
"@typescript-eslint/quotes": ["warn", "double"], // Example of a rule with options as a warning
"@typescript-eslint/no-unused-vars": "warn", // Changes "no-unused-vars" to a warning
"@typescript-eslint/no-explicit-any": "warn" // Changes "no-explicit-any" to a warning - // eslint-disable-line @typescript-eslint/no-explicit-any
"@typescript-eslint/no-explicit-any": "warn", // Changes "no-explicit-any" to a warning - // eslint-disable-line @typescript-eslint/no-explicit-any
"react/react-in-jsx-scope": "off",
"react-hooks/rules-of-hooks": "warn",
"react/no-unescaped-entities": "warn",
"react/display-name": "warn",
"react/prop-types": "warn"

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.

Why were these lines changed?

@shashankk42
shashankk42 force-pushed the feature/rsa-settings branch from b1416f6 to 07a4dd4 Compare June 1, 2026 19:12
@shashankk42

Copy link
Copy Markdown
Author

Too many unrelated changes

Reverted non-RSA diffs.

@shashankk42
shashankk42 requested a review from tbennun June 1, 2026 19:26
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.

3 participants