Hardening/airgapped tests 07132026#64
Merged
Merged
Conversation
1. Optional ASCII banner
Current state:
- Both shells already accept bannerPath.
- Both unconditionally print the banner when interactive and the file exists.
- Incidence: strategus_incidence_shell.R
- CohortMethod: strategus_cohort_methods_shell.R
Plan:
- Add an explicit boolean parameter such as showBanner = TRUE to both exported shell functions.
- Gate the existing banner-read / cat() block on interactive && isTRUE(showBanner).
- Keep bannerPath unchanged so existing users can still point to custom ASCII art.
- Update roxygen for both functions to document that showBanner = FALSE suppresses the logo without affecting the rest of startup messaging.
Reasoning:
- This is low-risk and backward compatible.
- A boolean is clearer than overloading bannerPath = NULL, though supporting both is harmless if you want extra flexibility.
2. Build-time h help at every design step
Current state:
- Execution-menu help already exists.
- Build-time prompts mostly go through readline_with_dialogue() / readline_with_navigation(), but they do not currently intercept h.
- The dialogue wrapper already centralizes /ohdsi and /back, which makes it the best place to add another command hook.
- Dialogue wrapper: workflow_dialogue.R
Plan:
- Extend the interactive dialogue/session layer so prompt reads can optionally recognize h / help as a local shell command, similar to /ohdsi.
- Add a build-help dispatcher keyed off current workflow step and role, using the existing step metadata already tracked in dialogue state.
- Use the normalized step names already defined in:
- workflow_dialogue_mapping.R
- Start with concise help text for these stages:
- study_intent
- intent_split
- role statement entry (target, comparator, outcome)
- source selection (ai/file/dir/db)
- DB import prompts
- phenotype selection / review
- design setup / analytic settings
- Standardize a small command vocabulary:
- h or help: show commands for current step
- /ohdsi <question>: contextual guidance
- /back: go to previous supported boundary
- Enter: accept default / continue
- Keep the help text step-specific, not global. The user asked for “appropriate for the step they are on”; that argues against a single monolithic help page.
Implementation shape:
- Best approach is to add an optional help_handler into new_workflow_dialogue_session() and let readline_with_dialogue() loop when h is entered.
- That avoids sprinkling if (tolower(inp) == "h") throughout both shells.
3. Construct and confirm study_intent after direct cohort-statement entry
Current state:
- Both shells allow blank study intent and direct acquisition.
- Incidence direct-acquisition branch: strategus_incidence_shell.R
- CohortMethod direct-acquisition branch: strategus_cohort_methods_shell.R
- Right now studyIntent can remain "", which weakens /ohdsi and downstream context.
Plan:
- After the user finishes entering cohort role statements in direct-acquisition mode, detect !nzchar(trimws(studyIntent)).
- Build a default study_intent template from the entered statements:
- Incidence:
- Summarize the incidence of the outcome <outcome cohort statement> in patients from the target cohort <target cohort statement>.
- CohortMethod:
- Compare the rate of occurrence of outcome <outcome cohort statement> between <target cohort statement> and <comparator cohort statement>.
- Present that constructed text to the user for confirmation/edit before proceeding.
- Persist the final confirmed value exactly like an entered study intent, so:
- dialogue context uses it
- saved state/project metadata uses it
- restarts/resume keep it
Important design point:
- This should happen only after the role statements are known, not earlier.
- It should also run for non-interactive mode if studyIntent is blank and role statements are supplied, but in that case it should auto-accept the template rather than prompting.
Open issue:
- In your CohortMethod template, you wrote “<comparator outcome statement>”. I assume that is a typo and should be comparator cohort statement. I’d implement it that way unless you want outcome wording there intentionally.
4. Seed DB config templates when project folder is first created
Current state:
- Already implemented in both shells during project initialization.
- The cohort-source DB guardrail also already exists and cleanly blocks DB import until the template is populated:
- cohort_acquisition.R
Plan:
- Do not invent new behavior unless you want different template contents or different timing.
- Keep the existing seeding behavior, but clean it up by extracting the duplicated seed_strategus_runtime_templates() logic into a shared helper.
- Then align tests and docs with reality:
- confirm templates are created on first run
- confirm non-DB source modes are not blocked by a blank cohort-source template
- confirm DB mode is blocked until the cohort-source file is populated
This is mostly refactor + validation, not a feature addition.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.