Skip to content

[ESSREDUCE] refactor: WIP - #689

Closed
jokasimr wants to merge 1 commit into
mainfrom
refactor-widget-models2
Closed

[ESSREDUCE] refactor: WIP#689
jokasimr wants to merge 1 commit into
mainfrom
refactor-widget-models2

Conversation

@jokasimr

@jokasimr jokasimr commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Copied from #654

@github-actions github-actions Bot added the essreduce Issues for essreduce. label Aug 5, 2026
@github-actions github-actions Bot changed the title refactor: WIP [ESSREDUCE] refactor: WIP Aug 5, 2026

@SimonHeybrock SimonHeybrock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With a new look, I think a remember our discussion from before summer (on the original PR), as well as some new comments:

  1. Concerns about having model in ParameterSpec. My idea was that WorkflowSpec should be independent of workflow implementation. Having model kind of prescribes a sciline implementation.
  2. use_workflow_default is similar (linked to what we have in our sciline-based workflows). I think our conclusion was that we should never use workflow defaults.
  3. I think title and description should be mandatory.
  4. Is ParameterSpec necessary, or would it be cleaner to just use pydantic? We may alternatively consider using a Protocol that could be fulfilled by Pydantic models.
  5. WorkflowSpec: typical_outputs and other rather bespoke fields should likely be kept out of a generic spec, at least in v0. This feature was what we used in the original UI generation code, but should maybe be kept in a subclass for now (not part of this PR).
  6. Is WorkflowRegistry necessary?
  7. Why does WorkflowSpec hold a workflow factory? The intent of WorkflowSpec is to be generic metadata for a workflow, this seems to much tied to what is implemented concretely (based on old implementation)?

I think it is easier to have a fresh suggestion instead of editing all this into this one. I will open a PR for discussion.

@SimonHeybrock

Copy link
Copy Markdown
Member

As announced above, I have opened #690 as a fresh take — pure-interface spec (no factory, no sciline keys), one pydantic params model per workflow, plus an ADR with the rationale. Thanks for the groundwork here; much of the analysis carried over. I propose we close this PR and continue the discussion on #690.

@jokasimr jokasimr closed this Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

essreduce Issues for essreduce.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants