TECH_DEBT: Moving documentation from confluence to github #550
Conversation
…ervice in docs/service_specs.
seanmcilvenna
requested review from
sdmcgeown,
amphillipsLGC and
dvargaslantana
WalkthroughThe pull request introduces significant changes to the documentation structure within the Link Cloud project. It removes detailed descriptions of various services from the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (4)
docs/README.md (1)
1-26: Documentation structure aligns with tech debt objectivesThe new documentation structure successfully achieves the PR's objective of moving documentation from Confluence to GitHub. The organization is clear and maintainable, with:
- A high-level overview
- Dedicated sections for functionality and service specifications
- Consistent use of relative links
This change appropriately falls within the scope of a TECH-DEBT PR as it only involves documentation restructuring without affecting core functionality.
Consider adding a table of contents at the top of the file for easier navigation when the documentation grows larger.
🧰 Tools
🪛 LanguageTool
[grammar] ~4-~4: You should probably use “include”.
Context: ... Overview This page and its references includes documentation for Link's services and t...(AGREEMENT_SENT_START)
[uncategorized] ~25-~25: A punctuation mark might be missing here.
Context: ...](service_specs/report.md) * Submission * Tenant(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/bff.md (1)
5-11: Consider adding deployment-related specifications.The service overview provides basic information but could be enhanced with the following deployment-critical details:
- Health check endpoints
- Resource requirements (CPU/memory limits/requests)
- Service dependencies
- Logging configuration
docs/functionality/census_management.md (2)
9-34: Fix indentation in Bulk FHIR process steps.The content thoroughly documents the acquisition methods. However, there's an indentation inconsistency in the Bulk FHIR process steps.
Apply this formatting fix:
- **Process**: - - Link initiates a `$export` request for patient data by `groupId`. - - The export process is monitored and polled routinely until completion. - - Upon completion, patient resources are retrieved, and the FHIR ID of each patient is extracted and stored. + - Link initiates a `$export` request for patient data by `groupId`. + - The export process is monitored and polled routinely until completion. + - Upon completion, patient resources are retrieved, and the FHIR ID of each patient is extracted and stored.🧰 Tools
🪛 Markdownlint
26-26: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
27-27: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
28-28: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
35-45: Minor wording improvement needed.The sections provide clear information about scheduling and data persistence. Consider simplifying the last sentence.
Apply this language improvement:
-This would provide users with meaningful patient identifiers, facilitating easier navigation and record management within the Link UI. +This would provide users with meaningful patient identifiers, facilitating navigation and record management within the Link UI.🧰 Tools
🪛 LanguageTool
[style] ~45-~45: This phrase is redundant. Consider writing “facilitating”.
Context: ...rs with meaningful patient identifiers, facilitating easier navigation and record management within...(FACILITATE_EASIER)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
README.md(1 hunks)docs/README.md(1 hunks)docs/functionality/census_management.md(1 hunks)docs/functionality/submission_folder.md(1 hunks)docs/service_specs/account.md(1 hunks)docs/service_specs/admin_ui.md(1 hunks)docs/service_specs/audit.md(1 hunks)docs/service_specs/bff.md(1 hunks)docs/service_specs/census.md(1 hunks)docs/service_specs/data_acquisition.md(1 hunks)docs/service_specs/measure_eval.md(1 hunks)docs/service_specs/normalization.md(1 hunks)docs/service_specs/notification.md(1 hunks)docs/service_specs/query_dispatch.md(1 hunks)docs/service_specs/report.md(1 hunks)docs/service_specs/submission.md(1 hunks)docs/service_specs/tenant.md(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- README.md
- docs/service_specs/account.md
- docs/service_specs/admin_ui.md
- docs/service_specs/audit.md
- docs/service_specs/data_acquisition.md
- docs/service_specs/notification.md
- docs/service_specs/tenant.md
🧰 Additional context used
📓 Path-based instructions (10)
docs/README.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/functionality/census_management.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/functionality/submission_folder.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/bff.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/census.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/measure_eval.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/normalization.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/query_dispatch.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/report.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/submission.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
🪛 LanguageTool
docs/README.md
[grammar] ~4-~4: You should probably use “include”.
Context: ... Overview This page and its references includes documentation for Link's services and t...
(AGREEMENT_SENT_START)
[uncategorized] ~25-~25: A punctuation mark might be missing here.
Context: ...](service_specs/report.md) * Submission * Tenant
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/functionality/census_management.md
[style] ~45-~45: This phrase is redundant. Consider writing “facilitating”.
Context: ...rs with meaningful patient identifiers, facilitating easier navigation and record management within...
(FACILITATE_EASIER)
docs/functionality/submission_folder.md
[style] ~24-~24: In American English, abbreviations like “etc.” require a period.
Context: ...HYPO - Patient/X - Condition/X, etc. - shared-resources.json - a Bundle o...
(ETC_PERIOD)
docs/service_specs/census.md
[grammar] ~5-~5: The plural noun “tenants” cannot be used with the article “a”. Did you mean “a tenant” or “tenants”?
Context: ...s primarily responsible for maintaining a tenants admit and discharge patient information...
(A_NNS)
docs/service_specs/measure_eval.md
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ...Overview The Measure Eval service is a Java based application that is primarily responsib...
(BASED_HYPHEN)
[uncategorized] ~42-~42: Possible missing comma found.
Context: ...pplication to use CQFramework libraries directly rather than relying on a separate CQF-R...
(AI_HYDRA_LEO_MISSING_COMMA)
docs/service_specs/normalization.md
[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/query_dispatch.md
[style] ~5-~5: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...y responsible for applying a lag period prior to making FHIR resource query requests aga...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~5-~5: In American English, abbreviations like “etc.” require a period.
Context: ... Labs have had their results finalized, etc), tenants are able to customize how lon...
(ETC_PERIOD)
[style] ~5-~5: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... their results finalized, etc), tenants are able to customize how long they would like the ...
(BE_ABLE_TO)
[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/report.md
[style] ~5-~5: Consider removing “of” to be more concise
Context: ...forms various workflows to determine if all of the patient MeasureReports are accounted fo...
(ALL_OF_THE)
[style] ~5-~5: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...reReports are accounted for that period prior to initiating the submission process. - *...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
docs/service_specs/submission.md
[style] ~10-~10: Consider using “who” when you are referring to people instead of objects.
Context: .../R4/list.html) resource of all patients that were admitted into the facility during ...
(THAT_WHO)
[style] ~13-~13: Consider removing “of” to be more concise
Context: .../R4/bundle.html) resource that contains all of the shared resources (Location, Medication,...
(ALL_OF_THE)
[style] ~13-~13: In American English, abbreviations like “etc.” require a period.
Context: ...shared resources (Location, Medication, etc) that are referenced in the patient Mea...
(ETC_PERIOD)
🪛 Markdownlint
docs/functionality/census_management.md
26-26: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
27-27: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
28-28: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
docs/functionality/submission_folder.md
21-21: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
22-22: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
23-23: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
24-24: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
🔇 Additional comments (6)
docs/README.md (2)
12-26: Well-organized service documentation structure
The service specifications section is well-structured with consistent naming and organization. Each service has its dedicated documentation file, making it easy to maintain and navigate.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~25-~25: A punctuation mark might be missing here.
Context: ...](service_specs/report.md) * Submission * Tenant
(AI_EN_LECTOR_MISSING_PUNCTUATION)
14-26: Verify service specification files exist
Let's ensure all service specification files are present.
✅ Verification successful
Let me try one more verification to check if the files might be in a different location or have a different extension:
All service specification files are present and correctly linked
The service specification files are all present in the docs/service_specs/ directory, matching exactly with the links in the README. The links are correctly structured and point to valid files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all service specification files exist
echo "Checking service specification files..."
for file in "service_specs/account.md" "service_specs/admin_ui.md" "service_specs/audit.md" \
"service_specs/bff.md" "service_specs/census.md" "service_specs/data_acquisition.md" \
"service_specs/measure_eval.md" "service_specs/normalization.md" "service_specs/notification.md" \
"service_specs/query_dispatch.md" "service_specs/report.md" "service_specs/submission.md" \
"service_specs/tenant.md"; do
if [ -f "$file" ]; then
echo "✓ Found: $file"
else
echo "✗ Missing: $file"
fi
done
Length of output: 6812
Script:
#!/bin/bash
# Search for markdown files containing service specs
echo "Searching for service specification files..."
fd -e md -t f
Length of output: 890
🧰 Tools
🪛 LanguageTool
[uncategorized] ~25-~25: A punctuation mark might be missing here.
Context: ...](service_specs/report.md) * Submission * Tenant
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/bff.md (1)
1-4: LGTM! Clear navigation and service rename notice.
The back navigation and service rename notice are well-formatted and provide clear information.
docs/functionality/census_management.md (3)
1-2: LGTM! Clear navigation link.
The back navigation link follows markdown best practices with a clear relative path.
3-8: Well-structured overview section!
The overview effectively introduces the Census Management module, its purpose, and scope. The content successfully transitions from Confluence while maintaining clarity and completeness.
1-45: Documentation successfully migrated from Confluence.
The new documentation file effectively captures the Census Management functionality while meeting the TECH_DEBT requirements of non-functional changes. The content is well-structured, technically accurate, and maintains a good balance between detail and readability.
🧰 Tools
🪛 LanguageTool
[style] ~45-~45: This phrase is redundant. Consider writing “facilitating”.
Context: ...rs with meaningful patient identifiers, facilitating easier navigation and record management within...
(FACILITATE_EASIER)
🪛 Markdownlint
26-26: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
27-27: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
28-28: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
Addressing some typos suggested by coderabbit
…tions for `img` tags missing `alt` attributes
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (25)
docs/README.md (1)
1-26: Well-structured documentation indexThe overall organization effectively supports the migration from Confluence to GitHub, providing a clear hierarchy and easy navigation to all service specifications and functionality documentation.
Consider adding:
- A brief description for each service in the list to help users quickly identify the service they need
- A table of contents at the top for easier navigation in longer documentation
🧰 Tools
🪛 LanguageTool
[uncategorized] ~4-~4: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...es and the functionality those services supports. ## Functionality * [Census Managemen...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
docs/service_specs/audit.md (3)
1-10: LGTM! Consider adding deployment prerequisites.The overview section is well-structured and provides essential service information. Consider adding deployment prerequisites or dependencies to make the documentation more comprehensive.
14-18: Fix table formatting and enhance security documentation.The table structure needs minor formatting improvements. Also, consider adding:
- Required permissions for the Azure App Configuration access
- Instructions for secret management
Apply this formatting fix:
-| Name | Value | Secret? | -|---------------------------------------------|-------------------------------|---------| +| Name | Value | Secret? | +|---------------------------------------------|-------------------------------|---------|🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...(AI_EN_LECTOR_MISSING_PUNCTUATION)
21-28: Consider adding additional Kafka configuration parameters.The current Kafka configuration might be incomplete. Consider documenting:
- Security settings (SSL/SASL)
- Consumer configuration (auto-offset-reset, max-poll-records)
- Error handling and retry policies
docs/service_specs/account.md (3)
14-18: Standardize placeholder formatFor better readability and consistency, consider using a standardized format for configuration placeholders.
Suggested change:
- | ConnectionStrings__AzureAppConfiguration | `<AzureAppConfigEndpoint>` | Yes | + | ConnectionStrings__AzureAppConfiguration | `${AZURE_APP_CONFIG_ENDPOINT}` | Yes |
36-38: Improve table formatting and endpoint documentationThe table formatting could be improved, and the endpoint placeholder should follow a consistent format.
Suggested changes:
| Name | Value | Secret? | |----------------------------------------------|--------------------------------|---------| - | TenantApiSettings__TenantServiceBaseEndpoint | `<TenantServiceUrl>/api` | No | + | TenantApiSettings__TenantServiceBaseEndpoint | `${TENANT_SERVICE_URL}/api` | No |🧰 Tools
🪛 LanguageTool
[uncategorized] ~37-~37: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | TenantApiSettings__TenantServiceBaseEn...(AI_EN_LECTOR_MISSING_PUNCTUATION)
44-46: Enhance event documentationConsider adding more details about the
AuditableEventOccurredevent, such as:
- Event payload structure
- Example message
- Use cases or scenarios when this event is produced
Would you like me to help create a template for the enhanced event documentation?
docs/service_specs/data_acquisition.md (3)
14-18: Standardize placeholder syntaxConsider using a consistent syntax for placeholders. Currently using
<AzureAppConfigEndpoint>, suggest using the same format across all documentation files.-| ConnectionStrings__AzureAppConfiguration | `<AzureAppConfigEndpoint>` | Yes | +| ConnectionStrings__AzureAppConfiguration | `{{AZURE_APP_CONFIG_ENDPOINT}}`| Yes |🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...(AI_EN_LECTOR_MISSING_PUNCTUATION)
23-26: Consider making Kafka GroupId configurableThe
GroupIdis currently hardcoded to "data-acquisition-events". Consider making this configurable to support different deployment scenarios (e.g., multiple instances, staging environments).-| KafkaConnection__GroupId | data-acquisition-events | No | +| KafkaConnection__GroupId | {{KAFKA_GROUP_ID}} | No |
28-37: Enhance event documentation with schemas and examplesThe current event documentation only lists event names. Consider adding:
- Event schema/payload structure
- Example payloads
- Event triggering conditions
- Event handling expectations
This would help developers better understand the event-driven architecture.
Would you like me to provide a template for documenting events with these details?
docs/service_specs/normalization.md (2)
15-19: Minor table formatting improvements neededThe table structure is good, but there's a minor formatting issue with the separator line.
-|---------------------------------------------|-------------------------------|---------| +| --------------------------------------------- | ----------------------------- | -------- |🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...(AI_EN_LECTOR_MISSING_PUNCTUATION)
37-44: Consider adding brief descriptions for each eventWhile the events are clearly listed, adding a brief description of each event's purpose and payload structure would improve clarity.
Example format:
## Consumed Events - **PatientDataAcquired**: Triggered when patient data is successfully acquired from EHR. Contains patient demographics and resource references. ## Produced Events - **PatientNormalized**: Emitted after successful normalization of patient data. Contains normalized FHIR resources. - **NotificationRequested**: Triggered when the normalization process requires user attention. Contains notification details and severity level.docs/service_specs/measure_eval.md (4)
1-11: Minor grammatical improvement needed in the overviewThe overview section is well-structured and informative. However, "Java based" should be hyphenated as "Java-based" for correct grammar.
-The Measure Eval service is a Java based application that is primarily responsible +The Measure Eval service is a Java-based application that is primarily responsible🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ...Overview The Measure Eval service is a Java based application that is primarily responsib...(BASED_HYPHEN)
28-34: Consider using placeholder URLs in documentationThe configuration section contains environment-specific URLs. Consider replacing these with placeholder values to:
- Avoid exposing internal domains
- Make the documentation applicable across different environments
-| MeasureEvalConfig__TerminologyServiceUrl | `https://cqf-ruler.nhsnlink.org/fhir` | No | -| MeasureEvalConfig__EvaluationServiceUrl | `https://cqf-ruler.nhsnlink.org/fhir` | No | +| MeasureEvalConfig__TerminologyServiceUrl | `<CQF_RULER_URL>/fhir` | No | +| MeasureEvalConfig__EvaluationServiceUrl | `<CQF_RULER_URL>/fhir` | No |
35-43: Consider adding event schema detailsThe events section would be more helpful with additional details about:
- Event payload schemas
- Required fields
- Example messages
44-44: Add comma for better readabilityAdd a comma after "directly" to improve readability.
-> **Note**: This service is being re-designed as a Java application to use CQFramework libraries directly rather than relying on a separate CQF-Ruler installation. +> **Note**: This service is being re-designed as a Java application to use CQFramework libraries directly, rather than relying on a separate CQF-Ruler installation.🧰 Tools
🪛 LanguageTool
[uncategorized] ~44-~44: Possible missing comma found.
Context: ...pplication to use CQFramework libraries directly rather than relying on a separate CQF-R...(AI_HYDRA_LEO_MISSING_COMMA)
docs/service_specs/census.md (2)
5-5: Fix grammar in service descriptionThe sentence has a grammatical error with the plural noun "tenants".
-The Census service is primarily responsible for maintaining a tenants admit and discharge patient information needed to determine when a patient is ready for reporting. +The Census service is primarily responsible for maintaining tenant admit and discharge patient information needed to determine when a patient is ready for reporting.🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: The plural noun “tenants” cannot be used with the article “a”. Did you mean “a tenant” or “tenants”?
Context: ...s primarily responsible for maintaining a tenants admit and discharge patient information...(A_NNS)
44-50: Consider enhancing event documentationWhile the events are listed, adding brief descriptions of their purpose and payload structure would make this documentation more useful for developers.
Example enhancement:
## Consumed Events -**Event**: `PatientIDsAcquired` +**Event**: `PatientIDsAcquired` +- **Purpose**: Triggered when new patient IDs are available for census processing +- **Payload**: Array of patient identifiers and their associated metadata +- **Source Service**: [Service name that produces this event] ## Produced Events -**Event**: `PatientCensusScheduled` +**Event**: `PatientCensusScheduled` +- **Purpose**: Indicates that a patient's census data has been scheduled for processing +- **Payload**: Patient identifier and scheduled processing timestamp +- **Consuming Services**: [List of services that consume this event]docs/service_specs/query_dispatch.md (2)
5-5: Enhance readability with minor text improvementsConsider these style improvements:
- Replace "prior to" with "before"
- Add a period after "etc."
- Replace "are able to" with "can"
-The Query Dispatch service is primarily responsible for applying a lag period prior to making FHIR resource query requests against a facility endpoint. The current implementation of the Query Dispatch service handles how long Link Cloud should wait before querying for a patient's FHIR resources after being discharged. To ensure that the encounter related data for the patient has been settled (Medications have been closed, Labs have had their results finalized, etc), tenants are able to customize how long they would like the lag from discharge to querying to be. +The Query Dispatch service is primarily responsible for applying a lag period before making FHIR resource query requests against a facility endpoint. The current implementation of the Query Dispatch service handles how long Link Cloud should wait before querying for a patient's FHIR resources after being discharged. To ensure that the encounter related data for the patient has been settled (Medications have been closed, Labs have had their results finalized, etc.), tenants can customize how long they would like the lag from discharge to querying to be.🧰 Tools
🪛 LanguageTool
[style] ~5-~5: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...y responsible for applying a lag period prior to making FHIR resource query requests aga...(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~5-~5: In American English, abbreviations like “etc.” require a period.
Context: ... Labs have had their results finalized, etc), tenants are able to customize how lon...(ETC_PERIOD)
[style] ~5-~5: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... their results finalized, etc), tenants are able to customize how long they would like the ...(BE_ABLE_TO)
44-51: Consider enhancing event documentationWhile the events are listed, it would be helpful to include:
- Event schemas/payloads
- Example messages
- Event handling behavior
- Any retry or error handling policies
This additional information would make the documentation more valuable for developers implementing event handlers.
Would you like me to provide a template for the enhanced event documentation?
docs/service_specs/report.md (1)
43-54: Consider enhancing event documentationWhile the events are listed clearly, adding brief descriptions for each event would help developers understand:
- The purpose and trigger conditions for each event
- The payload structure
- Any specific handling requirements
Example format:
## Consumed Events - **ReportScheduled**: Triggered when {condition}. Contains {key fields}. ```json { "example": "payload structure" }</blockquote></details> <details> <summary>docs/service_specs/submission.md (4)</summary><blockquote> `16-16`: **Use forward slashes for path consistency** Replace Windows-style backslashes with forward slashes for cross-platform compatibility. ```diff -An example of the submission package can be found at `\link-cloud\Submission Example`. +An example of the submission package can be found at `/link-cloud/Submission Example`.
10-14: Minor grammar and style improvementsA few style improvements would enhance readability:
-| Patient List | A [List](https://hl7.org/fhir/R4/list.html) resource of all patients that were admitted into the facility during the reporting period | No | +| Patient List | A [List](https://hl7.org/fhir/R4/list.html) resource of all patients who were admitted into the facility during the reporting period | No | -| Other Resources | A [Bundle](https://hl7.org/fhir/R4/bundle.html) resource that contains all of the shared resources (Location, Medication, etc) that are referenced in the patient Measure Reports | No | +| Other Resources | A [Bundle](https://hl7.org/fhir/R4/bundle.html) resource that contains all shared resources (Location, Medication, etc.) that are referenced in the patient Measure Reports | No |🧰 Tools
🪛 LanguageTool
[style] ~10-~10: Consider using “who” when you are referring to people instead of objects.
Context: .../R4/list.html) resource of all patients that were admitted into the facility during ...(THAT_WHO)
[style] ~13-~13: Consider removing “of” to be more concise
Context: .../R4/bundle.html) resource that contains all of the shared resources (Location, Medication,...(ALL_OF_THE)
[style] ~13-~13: In American English, abbreviations like “etc.” require a period.
Context: ...shared resources (Location, Medication, etc) that are referenced in the patient Mea...(ETC_PERIOD)
41-46: Add blank lines around tables for better readabilityAccording to Markdown best practices, tables should be surrounded by blank lines.
### Database + | Name | Value | Secret? | |----------------------------------------------|------------------------------------------------------|----------| | MongoDB__ConnectionString | `<ConnectionString>` | Yes | | MongoDb__DatabaseName | `<DatabaseName>` | No | +🧰 Tools
🪛 Markdownlint
42-42: null
Tables should be surrounded by blank lines(MD058, blanks-around-tables)
60-66: Enhance event documentationThe events section would benefit from additional details such as:
- Event payload structure
- Event handling behavior
- Success/failure scenarios
Would you like me to help create a template for more detailed event documentation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
docs/README.md(1 hunks)docs/service_specs/account.md(1 hunks)docs/service_specs/admin_ui.md(1 hunks)docs/service_specs/audit.md(1 hunks)docs/service_specs/bff.md(1 hunks)docs/service_specs/census.md(1 hunks)docs/service_specs/data_acquisition.md(1 hunks)docs/service_specs/measure_eval.md(1 hunks)docs/service_specs/normalization.md(1 hunks)docs/service_specs/notification.md(1 hunks)docs/service_specs/query_dispatch.md(1 hunks)docs/service_specs/report.md(1 hunks)docs/service_specs/submission.md(1 hunks)docs/service_specs/tenant.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/service_specs/admin_ui.md
- docs/service_specs/notification.md
- docs/service_specs/tenant.md
🧰 Additional context used
📓 Path-based instructions (11)
docs/README.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/account.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/audit.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/bff.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/census.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/data_acquisition.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/measure_eval.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/normalization.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/query_dispatch.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/report.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
docs/service_specs/submission.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
📓 Learnings (1)
docs/README.md (1)
Learnt from: seanmcilvenna
PR: lantanagroup/link-cloud#550
File: docs/README.md:8-10
Timestamp: 2024-11-14T23:07:29.074Z
Learning: When reviewing markdown files in the `docs/` directory, recognize that relative links to files (e.g., `functionality/census_management.md`) are valid if the referenced files exist relative to the location of the current file.
🪛 LanguageTool
docs/README.md
[uncategorized] ~4-~4: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...es and the functionality those services supports. ## Functionality * [Census Managemen...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
docs/service_specs/account.md
[uncategorized] ~37-~37: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | TenantApiSettings__TenantServiceBaseEn...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/audit.md
[uncategorized] ~15-~15: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/bff.md
[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/census.md
[grammar] ~5-~5: The plural noun “tenants” cannot be used with the article “a”. Did you mean “a tenant” or “tenants”?
Context: ...s primarily responsible for maintaining a tenants admit and discharge patient information...
(A_NNS)
docs/service_specs/data_acquisition.md
[uncategorized] ~15-~15: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/measure_eval.md
[uncategorized] ~5-~5: This expression is usually spelled with a hyphen.
Context: ...Overview The Measure Eval service is a Java based application that is primarily responsib...
(BASED_HYPHEN)
[uncategorized] ~44-~44: Possible missing comma found.
Context: ...pplication to use CQFramework libraries directly rather than relying on a separate CQF-R...
(AI_HYDRA_LEO_MISSING_COMMA)
docs/service_specs/normalization.md
[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/query_dispatch.md
[style] ~5-~5: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...y responsible for applying a lag period prior to making FHIR resource query requests aga...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[style] ~5-~5: In American English, abbreviations like “etc.” require a period.
Context: ... Labs have had their results finalized, etc), tenants are able to customize how lon...
(ETC_PERIOD)
[style] ~5-~5: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... their results finalized, etc), tenants are able to customize how long they would like the ...
(BE_ABLE_TO)
docs/service_specs/report.md
[style] ~5-~5: Consider removing “of” to be more concise
Context: ...forms various workflows to determine if all of the patient MeasureReports are accounted fo...
(ALL_OF_THE)
[style] ~5-~5: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...reReports are accounted for that period prior to initiating the submission process. - *...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[uncategorized] ~15-~15: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/submission.md
[style] ~10-~10: Consider using “who” when you are referring to people instead of objects.
Context: .../R4/list.html) resource of all patients that were admitted into the facility during ...
(THAT_WHO)
[style] ~13-~13: Consider removing “of” to be more concise
Context: .../R4/bundle.html) resource that contains all of the shared resources (Location, Medication,...
(ALL_OF_THE)
[style] ~13-~13: In American English, abbreviations like “etc.” require a period.
Context: ...shared resources (Location, Medication, etc) that are referenced in the patient Mea...
(ETC_PERIOD)
🪛 Markdownlint
docs/service_specs/submission.md
42-42: null
Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (19)
docs/README.md (2)
6-10: LGTM! Clear structure and valid links.
The Functionality section is well-organized with proper relative links to detailed documentation.
12-26: Verify all service specification files exist
The structure looks good, but let's ensure all referenced service specification files are present.
✅ Verification successful
All service specification files are present and correctly linked
All 13 service specification markdown files referenced in the README.md exist in the expected location under docs/service_specs/. The documentation structure is complete and valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify existence of all service specification files
echo "Checking service specification files..."
cd docs
for file in service_specs/*.md; do
if [ -f "$file" ]; then
echo "✓ Found: $file"
else
echo "✗ Missing: $file"
fi
done
Length of output: 2103
docs/service_specs/data_acquisition.md (1)
9-9: Consider removing specific port number from documentation
Exposing specific port numbers in public documentation could potentially aid malicious actors. Consider making this configurable and documenting it in internal deployment guides instead.
docs/service_specs/normalization.md (2)
1-12: LGTM! Clear and well-structured overview section.
The overview section effectively communicates the service's purpose and technical specifications.
10-10: Verify database migration status
The documentation mentions "previously Mongo", suggesting a database migration. Let's verify if this migration is complete across all environments.
docs/service_specs/census.md (1)
15-19: LGTM! Well-structured environment variables section
The table clearly indicates which values are secrets and uses appropriate placeholders for sensitive information.
docs/service_specs/bff.md (5)
1-4: LGTM! Clear navigation and service rename notice.
The back navigation and service rename warning are well-structured and visible.
5-11: LGTM! Complete service metadata.
The overview section includes all essential metadata fields (Technology, Image Name, Port, Database, Scale) and follows the consistent structure seen across other service specifications.
13-19: LGTM! Environment variables are well-documented.
The environment variables table is clear and consistent with the documentation structure.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
20-42: LGTM! Well-structured app settings documentation.
The app settings section is logically organized into Service Endpoints and Identity Provider subsections, with consistent formatting for URL endpoints.
1-42: Verify documentation migration completeness.
The documentation has been successfully migrated from Confluence while maintaining a clear structure. This aligns with the PR's TECH_DEBT objectives of moving documentation without affecting core functionality.
✅ Verification successful
Documentation migration is complete and consistent
The verification confirms that all service specifications in docs/service_specs/ maintain:
- Consistent structure with Overview, Environment Variables, and App Settings sections
- Proper back navigation links to the main documentation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify documentation completeness and consistency
# Check if all service specs follow similar structure
echo "Verifying documentation structure consistency..."
rg -l "^## Overview|^## Environment Variables|^## App Settings" docs/service_specs/
# Verify back navigation links
echo "\nVerifying navigation links..."
rg -l "^\[← Back Home\]" docs/service_specs/
Length of output: 1115
🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/service_specs/query_dispatch.md (2)
15-19: LGTM! Good security practices
The environment variables section clearly identifies sensitive information and follows good security practices.
22-29: LGTM! Clear configuration documentation
The Kafka connection and additional settings sections are well-documented and provide clear configuration guidance.
Also applies to: 38-43
docs/service_specs/report.md (3)
14-18: LGTM! Environment variables are properly documented
The environment variables section correctly identifies secrets and uses placeholder values for sensitive information.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
1-54: Documentation structure aligns well with PR objectives
The documentation successfully moves service specifications from Confluence to GitHub while maintaining a clear, organized structure. The content is comprehensive and follows good documentation practices.
🧰 Tools
🪛 LanguageTool
[style] ~5-~5: Consider removing “of” to be more concise
Context: ...forms various workflows to determine if all of the patient MeasureReports are accounted fo...
(ALL_OF_THE)
[style] ~5-~5: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...reReports are accounted for that period prior to initiating the submission process. - *...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[uncategorized] ~15-~15: A punctuation mark might be missing here.
Context: ...-----------------------------|---------| | Link__Audit__ExternalConfigurationSour...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
1-11: Consider adding migration details for the database change
The documentation mentions a database change from MongoDB to MSSQL. Consider adding a note about the migration timeline or any actions required by service operators.
🧰 Tools
🪛 LanguageTool
[style] ~5-~5: Consider removing “of” to be more concise
Context: ...forms various workflows to determine if all of the patient MeasureReports are accounted fo...
(ALL_OF_THE)
[style] ~5-~5: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...reReports are accounted for that period prior to initiating the submission process. - *...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
docs/service_specs/submission.md (3)
1-66: Documentation migration successfully implemented
The documentation has been successfully migrated from Confluence and follows a clear, well-structured format. The changes align with the TECH_DEBT PR objectives as they only involve documentation updates without affecting core functionality.
🧰 Tools
🪛 LanguageTool
[style] ~10-~10: Consider using “who” when you are referring to people instead of objects.
Context: .../R4/list.html) resource of all patients that were admitted into the facility during ...
(THAT_WHO)
[style] ~13-~13: Consider removing “of” to be more concise
Context: .../R4/bundle.html) resource that contains all of the shared resources (Location, Medication,...
(ALL_OF_THE)
[style] ~13-~13: In American English, abbreviations like “etc.” require a period.
Context: ...shared resources (Location, Medication, etc) that are referenced in the patient Mea...
(ETC_PERIOD)
🪛 Markdownlint
42-42: null
Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
44-45: Consider removing MongoDB configuration
Since the service has migrated to MSSQL, these MongoDB configurations might be obsolete.
#!/bin/bash
# Check if MSSQL configuration exists
rg -i "mssql|sqlserver" --type-add config:*.{json,yaml,yml,xml} --type config
# Verify if MongoDB is still referenced in code
ast-grep --pattern 'MongoDB'21-21: Verify database migration status
The documentation mentions a database migration from MongoDB to MSSQL. Let's verify if any MongoDB configurations can be removed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
README.md (4)
Line range hint
1-277: Reconsider PR type or split changesAccording to the coding guidelines, PRs labeled as "TECH-DEBT" should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. However, this PR involves significant documentation restructuring and content migration from Confluence to GitHub.
Consider either:
- Removing the "TECH-DEBT" label from the PR title, or
- Splitting these changes into separate PRs - one for TECH-DEBT improvements and another for documentation restructuring
🧰 Tools
🪛 LanguageTool
[style] ~224-~224: Consider using “who” when you are referring to people instead of objects.
Context: ...nt that contains a list of all patients that are currently admitted in the facility....(THAT_WHO)
16-18: LGTM! Consider adding a brief overviewThe new Documentation section effectively directs users to detailed documentation. Consider adding a one-line overview of what kind of documentation users can expect to find in the linked file.
## Documentation -Documentation on Link's implementation and the functionality it supports can be found [here](docs/README.md). +Documentation on Link's implementation, service specifications, and supported functionality can be found [here](docs/README.md).
77-77: LGTM! Images enhance documentation clarityThe added images effectively illustrate the UI and workflows. All images include descriptive alt text and use appropriate relative paths.
Consider adding image dimensions as variables in a central configuration file for consistent sizing across documentation.
Also applies to: 206-206, 216-216, 222-222, 237-237
Line range hint
2-10: Update Table of Contents to match document structureThe Table of Contents includes a "Link Cloud Services" section that has been removed from the document. Please update the Table of Contents to reflect the current document structure.
## Table of Contents * [Introduction](#introduction) -* [Link Cloud Services](#link-cloud-services) +* [Documentation](#documentation) * [Helpful Tools](#helpful-tools) * [Docker Installation](#docker-installation) * [Quick Tenant Configuration](#quick-tenant-configuration) * [Manual Reporting](#manual-reporting) * [Reporting Event Workflow](#reporting-event-workflow)🧰 Tools
🪛 LanguageTool
[misspelling] ~14-~14: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...rveillance requirements. It is based on a event driven micro service architecture...(EN_A_VS_AN)
[uncategorized] ~14-~14: The adjective “event-driven” is spelled with a hyphen.
Context: ...eillance requirements. It is based on a event driven micro service architecture using C#, Ja...(DRIVEN_HYPHEN)
[misspelling] ~14-~14: This word is normally spelled as one.
Context: ...irements. It is based on a event driven micro service architecture using C#, Java, Kafka and ...(EN_COMPOUNDS_MICRO_SERVICE)
🪛 Markdownlint
21-21: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (5)
docs/images/docker_submission.pngis excluded by!**/*.pngdocs/images/readme_event_acquisition_evaluation.pngis excluded by!**/*.pngdocs/images/readme_event_census_discharge.pngis excluded by!**/*.pngdocs/images/readme_event_report_scheduling.pngis excluded by!**/*.pngdocs/images/readme_kafkaui.pngis excluded by!**/*.png
📒 Files selected for processing (1)
README.md(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
README.md (1)
Pattern **: PRs that have "TECH-DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates, and logging improvements. These TECH-DEBT PRs should not affect core functionality.
🔇 Additional comments (1)
README.md (1)
18-18: Verify documentation file paths
Let's verify that all referenced documentation files exist in their expected locations.
✅ Verification successful
Documentation links are valid and accessible
All referenced documentation files exist in their expected locations:
- docs/README.md is present
- All referenced images in docs/images/ directory are available
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that referenced documentation files exist
# Check if docs/README.md exists
if [ ! -f "docs/README.md" ]; then
echo "Error: docs/README.md is missing"
exit 1
fi
# Check if all referenced images exist
for img in docs/images/readme_*.png; do
if [ ! -f "$img" ]; then
echo "Error: $img is missing"
exit 1
fi
done
echo "All referenced files exist"
Length of output: 490
🛠 Description of Changes
Please see the root README on my branch for docs.
Added a section for "Documentation" that links to the docs/README.md
Exported from confluence each of the service specs and moved them to the docs/service_specs folder.
Moved the service-specific docs from that root README.md into their equivalent service spec.
Created new documentation for Census Management under "functionality" and moved the "submission folder structure" in functionality from confluence.
🧪 Testing Performed
N/A
But, after approval and merging, I plan to archive each of the equivalent pages in confluence.
Summary by CodeRabbit
Release Notes
New Features
Documentation