(closed — opened in error) - #10
Closed
DougManuel wants to merge 1 commit into
Closed
Conversation
Sequences the work identified by a review of the pipeline/analysis code, engineering practice, and the metadata/schema/config layer. Four phases: - Phase 0 makes verification possible (restorable environment, CI running tests, draft-profile smoke test). Nothing downstream can be confirmed until this lands: renv::restore() currently fails and no CI runs the 33 existing tests. - Phase 1 addresses scientific correctness, each fix landing with its regression test and an explicit protocol call (fix code to match the protocol, or amend the protocol with recorded rationale). Four of the seven items are undocumented methodological choices rather than bugs, and two divergences run the other way, where the code does not do what the protocol specifies. - Phase 2 converts the metadata layer from documentation into enforcement. The LinkML schemas are tooling-clean; the gap is a serialization adapter and a CI step. - Phase 3 prepares for merging modelling machinery from DemPoRT. Also records five design decisions that block Phase 3, and a cross-cutting workstream for the Master/RDC environment, which currently cannot run. The plan sits at the repository root because docs/development/ is gitignored.
There was a problem hiding this comment.
Pull request overview
Adds a repository-root remediation plan that sequences work from a three-pass audit so the pipeline can be verified, scientific correctness issues can be fixed with regression tests and protocol calls, and the metadata/config layer can be enforced as a reusable “spine” ahead of integrating DemPoRT modelling machinery.
Changes:
- Adds a phased remediation roadmap (verification infrastructure → scientific correctness → metadata enforcement → splice preparation).
- Documents key correctness defects to address, each with proposed tests and explicit protocol decision points.
- Captures cross-cutting design decisions and the remote-environment (statscan/RDC) execution workstream.
Suppressed comments (1)
remediation-plan.md:160
- Pronoun reference is unclear here ("It" / "its"), which makes the guidance harder to read. Consider naming the subject explicitly (external review/reviewers).
It will have nothing useful to say about CI configuration or lockfiles; don't spend its attention there.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ## What goes to external review | ||
|
|
||
| An external model won't have the repo, so give it the review report, this plan, and the specific code excerpts plus the SAS reference — and scope it to **methodology only**: |
DougManuel
changed the title
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
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.
Opened in error and closed. No changes were merged.