Converge Copilot Instructions and Remaining Intent Files with Hub - #88
Converge Copilot Instructions and Remaining Intent Files with Hub#88ptr727 wants to merge 4 commits into
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
PR Summary by QodoResync Intent Documentation and Line-Ending Policies with Hub
AI Description
Diagram
High-Level Assessment
Files changed (8) |
There was a problem hiding this comment.
🟡 Changes recommended
The new ARCHITECTURE.md introduces a product-name typo and inconsistent hard-coded test counts that should be corrected for documentation accuracy.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR continues the hub resync by converging Copilot/bootstrap guidance and repo documentation with the current hub canon, while extracting repo-specific architecture/process details into a dedicated ARCHITECTURE.md and updating supporting intent files (workflow contract docs, spelling dictionary, and EOL normalization rules).
Changes:
- Rewrite
.github/copilot-instructions.mdto the current hub “bootstrap + review runbook” shape and move repo architecture/process documentation into a newARCHITECTURE.md. - Re-vendor/merge governance docs (
WORKFLOW.md,CODESTYLE.md,GOVERNANCE.md) to match updated hub wording and link targets (notably routing local verification references toOPERATIONS.md). - Simplify line-ending policy enforcement via
.gitattributes/.editorconfigand refresh the fleet cspell word list.
File summaries
| File | Description |
|---|---|
| WORKFLOW.md | Updates the CI/CD contract text and examples to the current hub canonical. |
| GOVERNANCE.md | Adjusts repository layout docs to point architecture content to ARCHITECTURE.md. |
| cspell.json | Updates the allowed word list to match current fleet vocabulary. |
| CODESTYLE.md | Repoints “local verification” references to OPERATIONS.md and refreshes wording around pre-commit gates and shell checks. |
| ARCHITECTURE.md | New document capturing repo-specific architecture, pipeline, and test layout content formerly in Copilot instructions. |
| .github/copilot-instructions.md | Converges Copilot bootstrap + review runbook sections with hub, preserving the Disproved Claims ledger structure. |
| .gitattributes | Simplifies git-level EOL normalization to text=auto with LF default and CRLF for Windows scripts. |
| .editorconfig | Simplifies line-ending commentary and keeps LF default with a bat/cmd CRLF exception. |
Review details
Suppressed comments (1)
ARCHITECTURE.md:168
- This section also hard-codes a different test count for
CommandLineTests.csthan the earlier project-structure list. To keep the doc consistent (and reduce future maintenance), consider removing the per-file count here as well.
- `DateInferenceTests.cs`: Core date inference functionality (33 tests)
- `DateInferenceEdgeCasesTests.cs`: Date inference edge cases and integration (19 tests)
- `CommandLineTests.cs`: Command line parsing and validation (24 tests)
- `ProcessTaskTests.cs`: Process task tests (61 tests)
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Code Review by Qodo
1. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ptr727
commented
Aug 29, 2026
Re: suppressed finding on ARCHITECTURE.md:168 (duplicate/drifted test-count in the Test Architecture section's now-removed 'Test Categories' sub-list) - fixed in 5a625fc, same commit that fixed the parallel finding above it. That whole redundant sub-list (a partial, already-drifted duplicate of the fuller Project Structure list) was removed rather than re-synced, since keeping two lists of the same fact in sync is exactly what keeps re-drifting. |
There was a problem hiding this comment.
🔵 Needs a closer look
WORKFLOW.md contains a repo-specific statement about publish-release.yml lacking a plan job/single-job gating that does not match the current workflow structure.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
WORKFLOW.md:171
- D4.1’s repo-specific note is out of date with the current
publish-release.yml: this repo does run aplanjob (and a separatevalidatejob) and gates thepublishreusable workflow on bothneeds.plan.outputs.publishandneeds.validate.result. As written, the doc says there is no plan job and a single-job gate ongithub.ref_name, which no longer matches the workflow and can mislead future edits to the release chain.
- **D4.1 Gated single-branch publish.** Output: PRs smoke-test and publish nothing. A **human merge never auto-publishes**. The fleet shape puts a first `plan` job (`publish-plan-task.yml`) in front and gates every job on it, covering a **code-affecting bot push to `main`**, a **dispatch** of `main`/`develop`, and a **main-only weekly schedule**. **This repo carries no plan job and no push trigger at all**: `publish-release.yml` runs on `workflow_dispatch` or the weekly `main`-only schedule, and its single job gates on `github.ref_name` being `main` or `develop`, so a dispatch from a feature branch is a no-op. Nothing publishes on a push, bot or human, and accumulated changes ship in the next scheduled or dispatched run. Each run builds one branch.
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
CI's repo_gate.py --check eol/eol-coverage (reached via the pin bumped in a72c48c) now requires a '* text=auto eol=<ending>' default plus explicit *.bat/*.cmd CRLF pins, the hub's new .gitattributes shape. The old '* -text' plus per-path LF pins model this PR left untouched fails both checks. Confirmed byte-identical via --renormalize (no file content changes, policy-text only), and confirmed clean locally against the same repo_gate.py checks CI runs. This duplicates one commit from the intent-files PR (#88, stacked on this branch), which will become empty and drop out once that branch is rebased onto this commit.
5a625fc to
53c35f5CompareThe prior commit adopted the hub's simplified .gitattributes but left .editorconfig's comment block describing the superseded per-type-pin model (shell/Dockerfiles/uv.lock/.py-by-path/husky hook pins), which a review caught as now internally inconsistent within this PR. Sync it to the same hub canonical wording used for .gitattributes. This duplicates the matching hunk in the stacked intent-files PR (#88), which will become empty and drop out once that branch is rebased onto this commit.
…es with hub Rewrite .github/copilot-instructions.md's four declared sections to the current hub canonical (the review runbook shrank substantially, its mechanics now delegated to scripts/pr_review.py). Move this file's nine undeclared, repo-specific sections (architecture, patterns, processing pipeline, per-command implementation detail, test layout) into a new ARCHITECTURE.md, the destination spec/section-model.md declares for this content; drop the 'Command Line Usage' section outright as a duplicate of README.md's own Usage section. Preserve the (currently empty) Disproved Claims ledger and each of CODESTYLE.md/WORKFLOW.md's own repo-specific adaptations verbatim across the re-vendor. Re-vendor the remaining intent files against the current hub canonical: CODESTYLE.md (repoint its two dead GOVERNANCE.md 'Running the Linters Locally' links at OPERATIONS.md 'Local Verification', which now holds that content; adopt the hub's stronger pre-commit-gate wording; keep the repo's own 'The Full Post-Change Set' subsection), WORKFLOW.md (3-way merge against the hub revision this repo last converged with, picking up the .NET-publish artifact-naming rename and several D-clause wording fixes while preserving every repo-specific adaptation), .gitattributes and .editorconfig (adopt the hub's much-simplified '* text=auto eol=lf' blanket normalization, dropping the now-redundant per-path LF pins; confirmed byte-identical via --renormalize, so this is a policy-text simplification only), and cspell.json's fleet word list (opencode, Qodo added; nugetlibrary, pypilibrary dropped, matching the WORKFLOW.md artifact rename). .editorconfig-checker.json and version.json show no drift since this repo's last hub sync; their audit 'intent' findings are the timestamp advisory's known false-positive shape (AUDIT.md section 7). Depends on #86 (this branch is stacked on chore/hub-resync-2026-08), since the deleted repo-config/spec/secrets.json and the rewritten AUDIT.md/OPERATIONS.md that PR introduces are load-bearing for the CODESTYLE.md link fix and the GOVERNANCE.md Repository Layout entry here. Audit run 2026-08-29T01:09:26Z | hub b823bc4
This content was moved verbatim out of copilot-instructions.md in the prior commit, and review against the actual current source surfaced several pre-existing staleness bugs in it, not introduced by the move but newly caught because moving it to a canonical architecture doc drew a full review pass: - PhotoPrims -> PhotoPrism (spelling). - ../CODESTYLE.md -> ./CODESTYLE.md (ARCHITECTURE.md lives at the repo root now, not under .github/; the old relative path escaped the repo). - Removed HttpClientFactory.cs and AssemblyInfo.cs entries: neither file exists, and nothing in the tree uses Polly or sets a User-Agent header. Added the real, previously-undocumented LoggerFactory.cs. - Removed the DateInferenceEdgeCasesTests.cs entry (does not exist) and every hard-coded per-file test count, confirmed already drifted in two places (CommandLineTests.cs: 15 vs 24; the whole PhotoCleanerTests Project: 300 vs the actual 260) and structurally guaranteed to drift again the next time a test is added. - Supported-extensions list: added .asf, .m2t, .psd, confirmed present in MediaUtilities.SupportedExtensions but missing from the list. - Video conversion tiers: corrected against ProcessTask's actual s_remuxExtensions/s_reencodeExtensions/s_reencodeAudioExtensions sets (remux is .m2t/.mkv, not .mts/.m2ts/.mkv; re-encode includes .asf). - JSON source generation: SourceGenerationContext does not exist; ExifToolJson uses ExifToolJsonContext, and the Immich verify protocol's ImmichVerifyJsonContext was undocumented entirely. - Trim pattern: the real code trims a ReadOnlySpan<char> with a 5-char array (no duplicate space), not a 6-arg string Trim.
53c35f5 to
c684190CompareThere was a problem hiding this comment.
🟡 Changes recommended
WORKFLOW.md repo-specific notes are inconsistent with the current .github/workflows/* behavior (paths-filter/trigger/plan-job shape), and CODESTYLE.md references a non-existent local catalog/snippets/ path without clarifying it is hub-hosted.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Review caught that this repo's actual test-pull-request.yml and publish-release.yml no longer match the repo-specific deviations WORKFLOW.md's D1.1/D1.4/D1.5/D4.1 claimed, predating this resync (the git log's 'Adopt .NET Publish Workflow Schema' migration). Verified against the current workflow YAML directly: - test-pull-request.yml DOES carry a changes job with a dorny/paths-filter (one combined 'release' filter excluding .github/workflows/**), contra the 'this repo runs no paths filter' claim in both D1.1 and D1.4. - check-workflow-status needs [changes, validate, smoke-build] and treats each as success-or-skipped, contra D1.5's 'demands success from both... since neither is ever skipped' claim. - publish-release.yml DOES carry a plan job gating validate/publish on its outputs.publish, contra D4.1's 'this repo carries no plan job... single job' claim. It does still have no push trigger at all, which is the part of the old claim that was correct. Rewrote D1.1/D1.4/D1.5/D4.1 and the S1/S2/S3/S5/S6 scenario table rows to match the actual current workflows. Also reworded a CODESTYLE.md sentence naming catalog/snippets/ as a plain path to make explicit it's hub-only and not carried here (intent fidelity, judged by meaning, not byte-locked - direct fix rather than a decline).
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces/solidifies a routing mismatch where AGENTS.md still points architecture content at .github/copilot-instructions.md instead of ARCHITECTURE.md.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
.github/copilot-instructions.md:14
- This file now says architecture/pipeline details live in ARCHITECTURE.md, but AGENTS.md still routes those details to .github/copilot-instructions.md. That mismatch will send agents/readers to the wrong document; update AGENTS.md (and any other routers) to point architecture content at ARCHITECTURE.md to keep the instruction map consistent.
Do not duplicate rules from `AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, or `WORKFLOW.md` here.
This file contains only Copilot-specific bootstrap and output requirements. This project's
architecture, processing pipeline, and per-command implementation details live in
[`ARCHITECTURE.md`](../ARCHITECTURE.md).
ARCHITECTURE.md:90
- The exiftool CliWrap example uses
_fileInfo.FullName, but the codebase pattern uses afilePath(or similar) variable; as written, the snippet won't compile and may mislead contributors copying it. Adjust the example to match the actual call style used in MediaUtilities.
BufferedCommandResult result = await Cli.Wrap("exiftool")
.WithArguments(["-groupNames", "-json", "-validate", "-all", _fileInfo.FullName])
.ExecuteBufferedAsync();
- **Files reviewed:** 6/6 changed files
- **Comments generated:** 0 new
- **Review effort level:** Lite
</details>
AGENTS.md's own preamble (not the verbatim section below it) still routed architecture/pipeline/test-layout content at .github/copilot-instructions.md, stale since this PR moved that content to ARCHITECTURE.md. Repointed both routing sentences. ARCHITECTURE.md's External Tool Execution Pattern example used _fileInfo.FullName, which does not compile against the actual call in MediaUtilities.cs (uses filePath). Fixed the example to match.
ptr727
commented
Aug 29, 2026
Re: two suppressed findings from the last round (both fixed in 30c5819):
|
There was a problem hiding this comment.
🟢 Approval recommended
The documentation and instruction-set convergence matches the referenced workflow/YAML behavior and introduces no inconsistencies in the updated cross-references.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
…ets (#86) * Converge instruction set with hub, retire hub-hosted repo-config/secrets Re-vendor AGENTS.md and GOVERNANCE.md's declared-verbatim sections from the current hub canonical (hub main@b823bc4). Delete repo-config/*.json, repo-config/README.md, and spec/secrets.json: per spec/divergences.json these are 'retire' dispositions, hub-hosted and checked from a hub checkout (repo-config/configure.sh, spec/audit.py) rather than carried here. Rewrite AUDIT.md's General Settings/Rulesets/Secrets sections to point at those hub tools instead of the now-deleted local payloads, add the now-required OPERATIONS.md 'Local Verification' leading section, and sweep every other inbound reference to the deleted paths across GOVERNANCE.md, WORKFLOW.md, and host-tools.json. Also fix publish-release.yml's 'publish' job: its explicit if: bypassed the implicit needs-succeeded gate, so a failed validate job would not have blocked a publish. Add needs.validate.result == 'success'. Audit run 2026-08-29T01:09:26Z | hub b823bc4 * Bump the hub-hosted workflow pin past the retire dead-path exemption CI's prose gate (validate-task.yml) flagged 6 dead-path findings against spec/secrets.json and repo-config/README.md, both deleted by the prior commit. Per RESYNC.md section 4: a repo's pinned validate-task.yml SHA can predate the retire disposition's own dead-path exemption in prose_lint.py's HUB_HOSTED set. Confirmed: the previous pin (74ef727a7aa96493bcd5bc2ce49b2977721678a3) predates both exemption commits (7787662 for spec/secrets.json, e31d3fc for repo-config/), per git merge-base --is-ancestor. Bump every ptr727/ProjectTemplate reusable-workflow uses: in this repo's own workflows to the current hub main (b823bc493fcdc47e9d9d9d2fc183e3ca10114146), which contains both. Verified locally: running the current hub prose_lint.py against this branch's diff now finds 0 violations. * Fix .NET Format task's unordered CSharpier/build race .vscode/tasks.json's .NET Format task declared dependsOn [CSharpier Format, .NET Build] with no dependsOrder, so VS Code's default (parallel) let the build read source while CSharpier was still rewriting it, producing a nondeterministic local run. Add dependsOrder: sequence so CSharpier finishes before the build starts, matching OPERATIONS.md 'Local Verification''s documented ordering. * Silence SC1091 on the Husky-generated pre-commit hook's sourced line The pin bump in a72c48c reaches the hub's newer validate-task.yml, whose shellcheck/shfmt discovery now correctly picks up .husky/pre-commit as a shebang-executed extensionless script (per the shell-codestyle skill's discovery rule) and lints it for the first time. It found SC1091 on the '. $(dirname "$0")/_/husky.sh' line, which shellcheck can't follow since that file is generated at husky-install time and absent at lint time. Not a hub defect: the hub's own canonical catalog/snippets/husky/pre-commit template already carries this exact directive comment. PhotoCleaner's actual generated hook was just missing it. Verified dotnet husky install does not overwrite an existing .husky/pre-commit, so the fix is stable across reinstalls. Confirmed clean locally: shellcheck and shfmt both exit 0 on the file now. * Adopt the hub's simplified .gitattributes, required by the bumped pin CI's repo_gate.py --check eol/eol-coverage (reached via the pin bumped in a72c48c) now requires a '* text=auto eol=<ending>' default plus explicit *.bat/*.cmd CRLF pins, the hub's new .gitattributes shape. The old '* -text' plus per-path LF pins model this PR left untouched fails both checks. Confirmed byte-identical via --renormalize (no file content changes, policy-text only), and confirmed clean locally against the same repo_gate.py checks CI runs. This duplicates one commit from the intent-files PR (#88, stacked on this branch), which will become empty and drop out once that branch is rebased onto this commit. * Sync .editorconfig's line-ending comment with the new .gitattributes The prior commit adopted the hub's simplified .gitattributes but left .editorconfig's comment block describing the superseded per-type-pin model (shell/Dockerfiles/uv.lock/.py-by-path/husky hook pins), which a review caught as now internally inconsistent within this PR. Sync it to the same hub canonical wording used for .gitattributes. This duplicates the matching hunk in the stacked intent-files PR (#88), which will become empty and drop out once that branch is rebased onto this commit.
Part of the periodic hub resync (see also #86 and the skills-tree PR from the same run). Audit
run
2026-08-29T01:09:26Z | hub b823bc4.Stacked on #86 (targets
chore/hub-resync-2026-08, notdevelop): the deletedrepo-config//spec/secrets.jsonand the rewrittenAUDIT.md/OPERATIONS.mdthat PRintroduces are load-bearing for the
CODESTYLE.mdlink fix and theGOVERNANCE.mdRepositoryLayout entry here. Retarget to
developonce #86 merges..github/copilot-instructions.md's four declared sections to the current hubcanonical (the review runbook shrank substantially, its mechanics now delegated to
scripts/pr_review.py). Preserve the (currently empty) Disproved Claims ledger.pipeline, per-command implementation detail, test layout) into a new
ARCHITECTURE.md, thedestination
spec/section-model.mddeclares for this content. Drop "Command Line Usage"outright as a duplicate of
README.md's own Usage section.CODESTYLE.md(repoint its two deadGOVERNANCE.md"Running the Linters Locally"links at
OPERATIONS.md"Local Verification", which now holds that content since thatsection moved hub-only; adopt the hub's stronger pre-commit-gate wording; keep the repo's own
"The Full Post-Change Set" subsection) and
WORKFLOW.md(3-way merge against the hub revisionthis repo last converged with — picks up the .NET-publish artifact-naming rename and several
D-clause wording fixes while preserving every repo-specific adaptation).
.gitattributes/.editorconfig(* text=auto eol=lfblanketnormalization, dropping the now-redundant per-path LF pins). Confirmed byte-identical via
--renormalize, so this is a policy-text simplification only, no file content changes.cspell.json's fleet word list (opencode,Qodoadded;nugetlibrary,pypilibrarydropped, matching the
WORKFLOW.mdartifact rename)..editorconfig-checker.jsonandversion.jsonshow no drift since this repo's last hub sync;their audit
intentfindings are the timestamp-advisory false-positive shape (AUDIT.mdsection 7).
🤖 Generated with Claude Code