-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(sub): polish substantial docs #967
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe documentation for the Substantial runtime has been updated to correct typographical errors and enhance the structure of the content. A new section titled "Backend" has been added, focusing on atomic operations for persisting and recovering workflow states. The previous "Backend" section has been renamed to "Persistence and Lifecycle." New subsections under "Key Concepts" have been introduced, detailing functionalities such as context, interrupts, saving, and event handling. Additionally, a new section on "Advanced Filters" has been added, providing guidance on filtering executed workflows. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx (3)
41-43
: Enhance backend documentation with usage recommendations.Consider adding:
- Which backend is recommended for production use
- Trade-offs between different backends (e.g., persistence guarantees, performance characteristics)
- Configuration examples for each backend type
95-102
: Fix GraphQL query formatting.The query has a typo in the language identifier and incorrect indentation.
-```gralhql +```graphql query { - awesomeSend( - run_id: "<workflow_run_id>", - event: { name: "myEvent", payload: 1234 } - ) + awesomeSend( + run_id: "<workflow_run_id>", + event: { name: "myEvent", payload: 1234 } + ) }
53-53
: Fix grammar and style issues.Several grammar issues need attention:
- Line 53: "all of the" can be simplified to "all the"
- Line 57: "An special" should be "A special"
- Line 71: Remove the duplicate "the"
-the workflow input (namely `kwargs` as seen in the example above), but it can also be thought as a namespace that contains all of the core functions +the workflow input (namely `kwargs` as seen in the example above), but it can also be thought as a namespace that contains all the core functions -An special state of the program +A special state of the program -if the the result was saved +if the result was savedAlso applies to: 57-57, 71-71
🧰 Tools
🪛 LanguageTool
[style] ~53-~53: Consider removing “of” to be more concise
Context: ...be thought as a namespace that contains all of the core functions used for durableness. *...(ALL_OF_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
[style] ~53-~53: Consider removing “of” to be more concise
Context: ...be thought as a namespace that contains all of the core functions used for durableness. *...
(ALL_OF_THE)
[misspelling] ~56-~56: Use “A” instead of ‘An’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...sed for durableness. * Interrupts An special state of the program that is pr...
(EN_A_VS_AN)
[duplication] ~71-~71: Possible typo: you repeated a word.
Context: ...ple) or replayed (after interrupts), if the the result was saved the saved function res...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pre-commit
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
Outdated
Show resolved
Hide resolved
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
Outdated
Show resolved
Hide resolved
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
Outdated
Show resolved
Hide resolved
@coderabbitai can you check for more typos or bad phrasings? You've missed some. |
Tip For best results, initiate chat on the files or code changes. @michael-0acf4 Yes, I found several issues that need attention:
Would you like me to suggest specific rewording for any of these sections? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 9534 |
Tip For best results, initiate chat on the files or code changes. @michael-0acf4 Yes, I found several issues in the documentation:
Would you like me to continue looking for more issues or would you prefer to address these first? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 9534 |
Tip For best results, initiate chat on the files or code changes. @michael-0acf4 I found several additional issues that weren't caught in the previous reviews:
Would you like me to suggest specific rewording for any of these sections? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 9534 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx (1)
87-89
:⚠️ Potential issueFix incorrect usage of ctx.save.
The function is being executed immediately instead of being passed as a callback.
-const result = await ctx.save(() => sendEmail()); +const result = await ctx.save(sendEmail);
🧹 Nitpick comments (4)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx (4)
14-14
: Fix grammatical issue in the microservices description.Change "microservices interactions" to "microservice interactions" for better grammatical correctness.
-- **Task orchestration**: Coordinate complex sequences of workflows (analogous to microservices interactions). ++ **Task orchestration**: Coordinate complex sequences of workflows (analogous to microservice interactions).🧰 Tools
🪛 LanguageTool
[grammar] ~14-~14: Nouns are not usually modified by plural nouns. Is it possible that you meant to use the singular or possessive form here?
Context: ...ex sequences of workflows (analogous to microservices interactions). For example, the workfl...(PLURAL_MODIFIER)
73-74
: Fix typo in the save description.Change "relies" to "relies" in the explanation.
-A save is one of the main building blocks of a workflow, many functions available on the context object relies on it. +A save is one of the main building blocks of a workflow, many functions available on the context object relies on it.
366-367
: Improve phrasing in the Advanced Filters introduction.The current phrasing could be more professional.
-In practice, you will have a lot of workflows that has been executed in different states. +In practice, you will have many workflows that have been executed in different states.🧰 Tools
🪛 LanguageTool
[style] ~366-~366: The phrase ‘a lot of’ might be wordy and overused. Consider using an alternative.
Context: ...ced Filters In practice, you will have a lot of workflows that has been executed in dif...(A_LOT_OF)
450-456
: Fix formatting in the example explanation.Remove the double dots and improve the sentence structure.
-For exapmle, the operand -```graphql -{ status: { contains: "\"COMPLETED\"" } } -``` - -..should cover `"COMPLETED"` and `"COMMPLETED_WITH_ERROR"`. +For example, the operand +```graphql +{ status: { contains: "\"COMPLETED\"" } } +``` + +This should cover both `"COMPLETED"` and `"COMPLETED_WITH_ERROR"`.🧰 Tools
🪛 LanguageTool
[typographical] ~454-~454: Two consecutive dots
Context: ...s: { contains: ""COMPLETED"" } } ``` ..should cover"COMPLETED"
and `"COMMPLE...(DOUBLE_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
(4 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
[grammar] ~14-~14: Nouns are not usually modified by plural nouns. Is it possible that you meant to use the singular or possessive form here?
Context: ...ex sequences of workflows (analogous to microservices interactions). For example, the workfl...
(PLURAL_MODIFIER)
[style] ~53-~53: Consider removing “of” to be more concise
Context: ...be thought as a namespace that contains all of the core functions used for durableness. I...
(ALL_OF_THE)
[uncategorized] ~61-~61: Possible missing comma found.
Context: ...ll pause the program at the line it was emitted then queue it back to execute later. O...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~76-~76: Possible missing comma found.
Context: ...a workflow is resumed (after a Typegate reboot for example) or replayed (after interru...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~366-~366: The phrase ‘a lot of’ might be wordy and overused. Consider using an alternative.
Context: ...ced Filters In practice, you will have a lot of workflows that has been executed in dif...
(A_LOT_OF)
[style] ~448-~448: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ...ing of the given value. * status
- Can be one of "COMPLETED"
, `"COMMPLETED_W...
(MISSING_IT_THERE)
[typographical] ~454-~454: Two consecutive dots
Context: ...s: { contains: ""COMPLETED"" } } ``` ..should cover "COMPLETED"
and `"COMMPLE...
(DOUBLE_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pre-commit
🔇 Additional comments (1)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx (1)
41-43
: LGTM! Clear explanation of backend options.The section effectively explains the backend abstraction and available options.
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx (7)
14-14
: Fix grammatical issue in the overview.The phrase "microservices interactions" should be "microservice interactions" for better grammatical correctness.
-Task orchestration**: Coordinate complex sequences of workflows (analogous to microservices interactions). +Task orchestration**: Coordinate complex sequences of workflows (analogous to microservice interactions).🧰 Tools
🪛 LanguageTool
[grammar] ~14-~14: Nouns are not usually modified by plural nouns. Is it possible that you meant to use the singular or possessive form here?
Context: ...ex sequences of workflows (analogous to microservices interactions). For example, the workfl...(PLURAL_MODIFIER)
Line range hint
16-39
: Add more context about the confirmation event.Consider adding a brief explanation of:
- How the confirmation event is triggered
- What happens if no confirmation is received
- Typical timeout scenarios
🧰 Tools
🪛 LanguageTool
[grammar] ~14-~14: Nouns are not usually modified by plural nouns. Is it possible that you meant to use the singular or possessive form here?
Context: ...ex sequences of workflows (analogous to microservices interactions). For example, the workfl...(PLURAL_MODIFIER)
73-74
: Fix verb agreement in the sentence.The verb should agree with the plural subject "functions".
-A save is one of the main building blocks of a workflow, many functions available on the context object relies on it. +A save is one of the main building blocks of a workflow, many functions available on the context object rely on it.🧰 Tools
🪛 LanguageTool
[uncategorized] ~73-~73: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...nctions available on the context object relies on it. This is mainly because a save c...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
75-77
: Enhance the save functionality explanation.The explanation should include what happens to the saved value during replays. Consider adding:
- How the value persistence works
- When the function is re-executed vs when the saved value is used
🧰 Tools
🪛 LanguageTool
[uncategorized] ~76-~76: Possible missing comma found.
Context: ...a workflow is resumed (after a Typegate reboot for example) or replayed (after interru...(AI_HYDRA_LEO_MISSING_COMMA)
120-121
: Document event timeout behavior.Consider adding information about:
- Whether events can timeout
- How to handle timeout scenarios
- Best practices for event handling
366-366
: Improve phrasing and fix verb agreement.The current sentence has style and grammar issues.
-In practice, you will have a lot of workflows that has been executed in different states. +In practice, you will have many workflows that have been executed in different states.🧰 Tools
🪛 LanguageTool
[style] ~366-~366: The phrase ‘a lot of’ might be wordy and overused. Consider using an alternative.
Context: ...ced Filters In practice, you will have a lot of workflows that has been executed in dif...(A_LOT_OF)
[uncategorized] ~366-~366: This verb does not appear to agree with the subject. Consider using a different form.
Context: ..., you will have a lot of workflows that has been executed in different states. Subs...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
454-455
: Fix typographical error.Remove the double dots for proper punctuation.
-..should cover `"COMPLETED"` and `"COMPLETED_WITH_ERROR"`. +...should cover `"COMPLETED"` and `"COMPLETED_WITH_ERROR"`.🧰 Tools
🪛 LanguageTool
[typographical] ~454-~454: Two consecutive dots
Context: ...s: { contains: ""COMPLETED"" } } ``` ..should cover"COMPLETED"
and `"COMPLET...(DOUBLE_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
(4 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
[grammar] ~14-~14: Nouns are not usually modified by plural nouns. Is it possible that you meant to use the singular or possessive form here?
Context: ...ex sequences of workflows (analogous to microservices interactions). For example, the workfl...
(PLURAL_MODIFIER)
[style] ~53-~53: Consider removing “of” to be more concise
Context: ...be thought as a namespace that contains all of the core functions used for durableness. I...
(ALL_OF_THE)
[uncategorized] ~61-~61: Possible missing comma found.
Context: ...ll pause the program at the line it was emitted then queue it back to execute later. O...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~63-~63: You might be missing the article “a” here.
Context: ...cute later. One simple example of such function is when you want to wait for a given am...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~73-~73: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...nctions available on the context object relies on it. This is mainly because a save c...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~76-~76: Possible missing comma found.
Context: ...a workflow is resumed (after a Typegate reboot for example) or replayed (after interru...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~366-~366: The phrase ‘a lot of’ might be wordy and overused. Consider using an alternative.
Context: ...ced Filters In practice, you will have a lot of workflows that has been executed in dif...
(A_LOT_OF)
[uncategorized] ~366-~366: This verb does not appear to agree with the subject. Consider using a different form.
Context: ..., you will have a lot of workflows that has been executed in different states. Subs...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[style] ~448-~448: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ...ing of the given value. * status
- Can be one of "COMPLETED"
, `"COMPLETED_WI...
(MISSING_IT_THERE)
[typographical] ~454-~454: Two consecutive dots
Context: ...s: { contains: ""COMPLETED"" } } ``` ..should cover "COMPLETED"
and `"COMPLET...
(DOUBLE_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pre-commit
dbb8443
to
a91fe40
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx (1)
87-88
:⚠️ Potential issueFix the save example to use an arrow function.
The current example incorrectly passes the result of
sendEmail()
toctx.save
instead of passing a function.-const result = await ctx.save(() => sendEmail()); +const result = await ctx.save(async () => sendEmail());
🧹 Nitpick comments (5)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx (5)
Line range hint
16-39
: Consider adding type annotations for better clarity.While the example is clear, adding explicit type annotations would improve readability and type safety.
-export async function sendEmail(ctx: Context) { +export async function sendEmail(ctx: Context): Promise<string> { const { to } = ctx.kwargs; - const info = await ctx.save(() => sendSubscriptionEmail(to)); + const info: string = await ctx.save(() => sendSubscriptionEmail(to)); - const timeSent = await ctx.save(() => new Date().toJSON()); + const timeSent: string = await ctx.save(() => new Date().toJSON());
40-43
: Add a note about production usage recommendations.Consider adding a note about which backends are recommended for production use versus development/testing.
This abstraction implements a set of atomic operations that allows Typegate to persist and recover the workflow state. Currently, we have the **Redis** backend available, along with others like **fs** and **memory**, which are primarily intended for development or testing purposes. + +:::note +For production environments, we recommend using the **Redis** backend as it provides the necessary durability and performance characteristics. +:::
52-53
: Improve sentence structure.The current phrasing is a bit wordy.
-The context object contains the workflow input (namely `kwargs` as seen in the example above), but it can also be thought as a namespace that contains all of the core functions used for durableness. +The context object contains the workflow input (namely `kwargs` as seen in the example above), but it can also be thought as a namespace containing the core functions used for durableness.🧰 Tools
🪛 LanguageTool
[style] ~52-~52: Consider removing “of” to be more concise
Context: ...be thought as a namespace that contains all of the core functions used for durableness. I...(ALL_OF_THE)
251-253
: Fix punctuation in the filter operations description.Add a comma for better readability.
-- **contains**: Check if the workflow output is a list that contains the given value or if the given value is a substring of it. +- **contains**: Check if the workflow output is a list that contains the given value, or if the given value is a substring of it.🧰 Tools
🪛 LanguageTool
[uncategorized] ~251-~251: Possible missing comma found.
Context: ...utput is a list that contains the given value or if the given value is a substring of...(AI_HYDRA_LEO_MISSING_COMMA)
261-262
: Fix formatting in the example explanation.Remove the double dots.
-..should cover `"COMPLETED"` and `"COMPLETED_WITH_ERROR"`. +This should cover `"COMPLETED"` and `"COMPLETED_WITH_ERROR"`.🧰 Tools
🪛 LanguageTool
[typographical] ~261-~261: Two consecutive dots
Context: ...s: { contains: ""COMPLETED"" } } ``` ..should cover"COMPLETED"
and `"COMPLET...(DOUBLE_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
(3 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx
[style] ~52-~52: Consider removing “of” to be more concise
Context: ...be thought as a namespace that contains all of the core functions used for durableness. I...
(ALL_OF_THE)
[uncategorized] ~251-~251: Possible missing comma found.
Context: ...utput is a list that contains the given value or if the given value is a substring of...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~261-~261: Two consecutive dots
Context: ...s: { contains: ""COMPLETED"" } } ``` ..should cover "COMPLETED"
and `"COMPLET...
(DOUBLE_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: pre-commit
🔇 Additional comments (2)
docs/metatype.dev/docs/reference/runtimes/substantial/index.mdx (2)
8-14
: LGTM! Clear and concise introduction.The introduction effectively communicates the key features and benefits of the Substantial runtime.
391-393
: LGTM! Well-structured API addition.The new
send_confirmation
endpoint follows the established pattern and is properly documented.
Migration notes
None
Summary by CodeRabbit