Skip to content

feat: document resource type - #1127

Open
michael-johnston wants to merge 24 commits into
mainfrom
maj_document
Open

feat: document resource type#1127
michael-johnston wants to merge 24 commits into
mainfrom
maj_document

Conversation

@michael-johnston

@michael-johnston michael-johnston commented Jul 1, 2026

Copy link
Copy Markdown
Member

This PR adds a new resource type, document, for storing markdown or html docs that describe spaces, operations or combinations thereof.

Create with ado create document -f DOCUMENT.YAML. Read with ado get document $RESOURCEID. A document can have multiple related resources. It's not required that it has any - for example a document containing a report about the entire project at a point in time should not have to list every single space and operation, and there is no "project" resource.

Example document yaml

metadata:
  name: Example operation report
  description: Summary report for an operation
contentType: "markdown"
content: |
  # Operation report

  This is an example document resource.
relatedResources:
  - operation-test-12345678

ado describe document when output is to terminal will

  • for markdown, render using rich
  • for html, write to file and open browser

when output is not to a tty the content field is dumped raw.

#925

@michael-johnston michael-johnston linked an issue Jul 1, 2026 that may be closed by this pull request
@michael-johnston michael-johnston changed the title Maj document feat: document resource type Jul 1, 2026
@michael-johnston
michael-johnston marked this pull request as ready for review July 14, 2026 10:39
@AlessandroPomponio

Copy link
Copy Markdown
Collaborator

From just the description, I wonder whether this design will lead to more of the same problems that we already have for Datacontainers. As it stands, Datacontainers store their content in the resources table and can be very large (we have examples of datacontainers from sft-trainer which exceed 18MB), ballooning the whole table up and slowing down retrieval.

In general, as well, storing byte-like objects in a stringified format in a JSON/text column seems like an antipattern. I would think about only storing references to BLOBs and having a dedicated table for those. Later on in the future we could also take advantage of it to slim down datacontainers

@michael-johnston

Copy link
Copy Markdown
Member Author

@AlessandroPomponio We could simplify this and remove the attachments functionality for initial implementation. Add it back later if needed/when we decided how to store the attachments (i.e. link instead of raw data, where the raw data goes).

@AlessandroPomponio

Copy link
Copy Markdown
Collaborator

Sounds good!

@AlessandroPomponio AlessandroPomponio 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.

The PR description says this is also meant for HTML reports. If we keep it to markdown only, though, we could also have a describe command that uses rich's Markdown renderer to render and print the document to the terminal.

This PR is also missing updating the ado cli reference doc

Comment on lines +130 to +132
```bash
uv run ado get document -q 'config.relatedResources=OPERATION_ID'
```

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.

Maybe before adding documents we could implement #376 and remove this field, since we already have a related resources table that has this exact purpose anyway.

This, however, would mean that we wouldn't be able to just retrieve the resource if we want to display this information, we would also have to lookup the related resources table

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The thing if its not there how on create do we know the resources the report is related to? :-)

This field is the same as spaces in operation yaml or samplestore in space yaml

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.

that's true - we should add some kind of mechanism to ensure this field doesn't become stale, though

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

How does it become stale

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.

If a new operation/space/whatever becomes related to the report and it's either:

  • Not added to the relatedResources field
  • Not appropriately added to the resources_relationship table

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is already a possibility, and has to be handled by a convention. There is no way to validate that whatever is in the field (string of text) is actually related to what is written or not.

So for reports that are on an operation/space - we assume who ever writes the report will put the correct resource ids. We can't edit this afterwards as of this PR - but it is tied to single resource ids anyway, so can't become stale in the sense you say.

For report that are on a project, adding every single resource in the project to the related resources seems overkill, although nothing stopping anyone. For project reports the date of the report and the date of the resources is enough to tell you what was present when it was written.

For a document which is about future to-be-created spaces/operations etc. (a study) there is nothing to add on creation (usually) and the related resources can't be edited. With the current capabilities of document we would rely on a convention, that would be to define labels.

Comment thread ado/core/document/config.py
Comment thread ado/core/document/resource.py Outdated
Comment thread ado/core/document/resource.py Outdated
Comment thread tests/core/document/conftest.py Outdated
Comment thread tests/core/document/test_document_config.py
Comment thread tests/core/document/test_document_resource.py
Comment thread tests/core/document/test_document_resource.py Outdated
Comment thread tests/core/document/test_document_resource.py Outdated
Comment thread tests/fixtures/document_with_attachment.yaml Outdated
@michael-johnston

Copy link
Copy Markdown
Member Author

The PR description says this is also meant for HTML reports. If we keep it to markdown only, though, we could also have a describe command that uses rich's Markdown renderer to render and print the document to the terminal.

Yes I did think of this and it was originally markdown. However then Daniele or Vassilis was showing me that bob was creating HTML reports with graphical elements and I thought we should store them.

I mean we could have it also id html and describe open a brokers.

It would be good to have the describe for markdown in any case as reading the reports as human is not easy as is.

For discriminating markdown and html content
For discriminating markdown and html content
outputs markdown as rich objects if output stream is terminal, otherwise writes content field to pipe/file.

if html and directed to terminal the content is written to file and then browser opened.
@michael-johnston

Copy link
Copy Markdown
Member Author

If implemented describe for markdown and html, also discriminating behaviour between tty and no tty

Comment on lines +34 to +36
When stdout is a terminal, markdown is rendered with rich and HTML is opened
in the default browser. When stdout is redirected (pipe or file), the raw
``content`` body is written with no styling.

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 think we should support this behaviour to keep a clear cut definition of describe. Markdown reports should be always rendered, while HTML reports should exit with a clear error when the output is not a terminal.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

How can I save a human readable md file then with the content?

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 would say ado get doc -o content. We could also internally map content to config for other resource types

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think the definition of describe should be to output human readable version of the content. I think it's fine for this output to adapt based on the capabilities of what stdout it connected to.

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.

The describe help says:

Print a human-friendly description of a resource or an experiment.

I think rendered markdown fits perfectly this definition, and I can see how despite not being "printed", saving an html file and opening it in the browser can map to it as well.

If we instead detect the redirect and either:

  • Print raw markdown
  • Print raw html

We're not printing any kind of human-friendly description. We're just allowing you to get the resource content by means of describe.

@michael-johnston michael-johnston Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not really if you send the markdown to file (as currently implemented) you get an md file with proper formatting - humans readable

Screenshot 2026-07-28 at 09 13 11

if you get the yaml you get a compressed string - not really human readable.

Screenshot 2026-07-28 at 09 12 09

if you send the rich formatted output to a file then you get additional formatting symbols - not really human readable.

Comment thread ado/cli/resources/document/describe.py Outdated
Comment thread ado/cli/resources/document/describe.py Outdated
Comment thread tests/core/document/test_document_config.py
Needs a different solution
is_terminal is not accurate enough for identify non-terminal output. In particular if certain rich envvars e.g. FORCE_COLOR are set it will report True even if output is redirected.
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.

feat: document resource type for storing reports and plans

2 participants