Skip to content

chore: prune superseded root docs (orbit-ui-mobile#380) - #282

Merged
thomasluizon merged 4 commits into
mainfrom
issue-380
Jul 4, 2026
Merged

chore: prune superseded root docs (orbit-ui-mobile#380)#282
thomasluizon merged 4 commits into
mainfrom
issue-380

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

Summary

Prune-only companion to the agentic-dev-workflow pack extraction. Removes two dead root-level docs:

  • description.md — superseded loose doc at repo root (README.md is canonical).
  • C:UsersthomaDocumentsProgrammingProjectsOrbit.planning06-02-start-time.txt — a path-collapsed junk filename (34-byte orphan).

Docs-only; no code references either file (git grep clean). dotnet build succeeds (0 errors; 13 pre-existing warnings unrelated to this change).

Refs thomasluizon/orbit-ui-mobile#380
Paired FE PR: thomasluizon/orbit-ui-mobile#388

🤖 Generated with Claude Code

… orphan

Removes two dead root-level docs superseded by README.md; the second is a
path-collapsed junk filename. Docs-only; no code references them.

Refs thomasluizon/orbit-ui-mobile#380

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Complete

Scope: PR #282 in thomasluizon/orbit-api — "chore: prune superseded root docs (orbit-ui-mobile#380)"
Recommendation: APPROVE

Summary

This is a docs-only, pure-deletion PR removing two dead root-level files: description.md (243 lines, a superseded planning doc — README.md is canonical) and a path-collapsed junk file (C:\Users\thoma\...06-02-start-time.txt, a 2-line orphan). No src/ code, DTOs, controllers, or contract surfaces are touched. git grep across the full repo confirms neither filename nor path is referenced anywhere else (build scripts, docs, code). There are no prior reviews or PR comments to reconcile against — this is the first pass. Risk is effectively zero.

Findings

Critical: None
High: None
Medium: None
Low / Info: None

Rubric dimension pass

All 13 dimensions gate out as N/A for this diff — no logic, no UI, no DTOs, no contract surface, no C# hard-rule surface touched. Dead-code dimension is satisfied by definition (this PR is the dead-code removal, and removal was verified safe via git grep).

Subagents

Agent Verdict
security-reviewer N/A — no src/ code changed
contract-aligner N/A — no DTO/route/packages/shared surface changed

Parity/i18n dimensions: N/A — frontend-only, and this diff has no frontend surface.

Validation

Check Result
Build (dotnet) N/A — skipped per instructions (CI-handled)
Tests (dotnet) N/A — skipped per instructions (CI-handled)
Reference check (git grep for both deleted filenames) PASS — zero references found

What's good

Clean, well-scoped prune with an honest PR description that states the verification already performed (git grep clean, dotnet build clean). Filenames and rationale are clearly documented.

Recommendation

Approve as-is. No action needed.

…w rubric

Mirror of the orbit-ui-mobile rubric addition (thomasluizon/orbit-ui-mobile#386):
API diffs that change the user-facing feature surface (gating, Astra/MCP tools)
must flag the required FEATURES.md update in the paired frontend PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Complete

Scope: PR #282 in thomasluizon/orbit-apichore: prune superseded root docs (orbit-ui-mobile#380)
Recommendation: APPROVE

Note: this PR already had an earlier claude[bot] APPROVE (2026-07-03T22:41:07Z) covering commit 279694a. This review covers the delta since then: commit da55cda, committed 22:48:38Z (after that approval).

Severity Count
Critical (incl. ⚠️ old-client breaks) 0
High 0
Medium 0
Low / Info 1 (carried over, still applies)

What's in the PR

Two commits on branch issue-380:

  1. 279694a — deletes description.md (root, superseded by README.md) and a stray path-collapsed junk file. Confirmed zero references to either via git grep both directions. Previously reviewed and approved.
  2. da55cda — adds a "FEATURES.md parity" dimension (#14) to this repo's own .claude/skills/pr-review/SKILL.md and rubric.md, mirroring the equivalent addition on the orbit-ui-mobile side (#386). New since the last approval — reviewed here.

No src/ C# code, DTOs, controllers, or contract surface changed by either commit, so Correctness (data-flow), dead code (product code), SOLID, comment policy, type safety, console.log, DESIGN.md, Parity, i18n, contract drift, security, and backend hard rules are all N/A — the diff never touches their surface.

Review of commit da55cda (the new delta)

  • SKILL.md's gating list and dimension-order list are updated in the same commit and stay in sync with rubric.md — no drift between the two files.
  • New rubric section 14 follows the existing dimension template (reference line, gate condition, trigger list, severity calls) and doesn't renumber or invalidate any existing cross-reference (dimension 1's "see dimension 13" callout at rubric.md line 86 is unaffected since 14 was appended after 13, not inserted before it).
  • Correctly scopes itself for this repo: since FEATURES.md lives in orbit-ui-mobile, the dimension states the orbit-api-side finding is always "verify in the paired frontend PR" rather than attempting to check a file that isn't in this checkout — consistent with this workflow's own CI-adaptation instructions.
  • This is purely meta/tooling content (review skill, not product code) — no runtime behavior changes.

Findings

INFO: PR body doesn't mention the second commit

  • location: PR #282 body vs. commit da55cda
  • issue: The PR description says "Prune-only companion... Docs-only" and only describes the two file deletions; the second commit editing the review skill's SKILL.md/rubric.md is unmentioned.
  • risk: None functionally — the added rubric section is well-formed and internally consistent (verified above). Purely a reviewability/description-accuracy gap, non-blocking.
  • fix: Update the PR description to mention both commits, or split unrelated chores into separate PRs going forward.

Validation

Check Result
Build (dotnet) N/A — diff has zero .cs/project-file changes; Build/Unit Tests/SonarCloud run as separate required CI checks
Cross-repo (contract-aligner, orbit-ui-mobile#386 side) Not verifiable in this job — sibling repo not checked out

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Complete

Scope: PR #282 in thomasluizon/orbit-api — "chore: prune superseded root docs (orbit-ui-mobile#380)"
Recommendation: APPROVE

Summary

The PR branch (issue-380) contains two commits, one disclosed and one not:

  1. 279694a — deletes description.md (243-line superseded root doc) and a path-collapsed junk .txt file. git grep across the checkout confirms zero remaining references to either filename. Matches the PR body exactly.
  2. da55cda — adds a new "FEATURES.md parity" dimension (#14) to this repo's own .claude/skills/pr-review/SKILL.md and rubric.md. Verified this is genuine branch history (git merge-base --is-ancestor da55cda origin/mainNO, i.e. not already landed on main), but it is not mentioned in the PR body, which only describes the doc deletions.

Findings

Critical: None
High: None

Medium

[MEDIUM] New rubric dimension 14 always emits a High finding when gated, regardless of whether the paired FE PR already did the work
· location: .claude/skills/pr-review/rubric.md:314-317 (last bullet of "14. FEATURES.md parity")
· issue: "In the orbit-api repo the file is not in the checkout: do not verify — emit the finding as 'FEATURES.md update required in thomasluizon/orbit-ui-mobile' (High) so it lands in the paired frontend PR." is unconditional — it fires whenever the gate trips, regardless of whether the paired mobile PR already updated FEATURES.md correctly.
· risk: Per the rubric's own deterministic rule (NEEDS WORK iff >=1 surviving Critical/High), this forces NEEDS WORK on every future orbit-api-only review that touches feature surface, even when correctly paired with an already-compliant FE PR - contradicting the Signal gate stated earlier in the same file ("never manufacture a Critical/High finding to avoid approving").
· fix: Downgrade to Info/Medium when reviewed from the orbit-api side alone (unverifiable here); reserve High for when cross-repo evidence actually shows the FE PR is missing the update.

Low / Info

[INFO] PR body doesn't mention the second commit (da55cda)
· PR body says "Prune-only... Docs-only" and only covers the two deletions; the rubric/skill edit is undisclosed.
· risk: none functionally; description-accuracy gap only.
· fix: update the PR description to mention both commits, or split into separate PRs going forward.

Independent verification performed

  • git merge-base --is-ancestor da55cda origin/mainNO (confirms da55cda is part of this PR's own branch history, not already on main).
  • git grep for both deleted filenames across the tree → no references found.
  • Diffed SKILL.md's gate/order-list edits against rubric.md's new section — internally consistent.

Not verifiable in this job

  • Parity (#9) / i18n (#10) / contract-aligner: sibling repo orbit-ui-mobile not checked out in this CI job.
  • Build/tests: skipped — covered by separate required CI checks (Build, Unit Tests, SonarCloud).

Recommendation

APPROVE. No Critical/High findings on the shipped diff. The Medium finding is a latent design gap in newly-added rubric dimension 14 (will over-trigger NEEDS WORK on unrelated future PRs) — worth fixing before that dimension sees real use, but non-blocking for this PR since it doesn't itself touch feature surface. The Info item (undisclosed second commit in the PR body) is non-blocking.

@sonarqubecloud

sonarqubecloud Bot commented Jul 4, 2026

Copy link
Copy Markdown

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: PR #282

Scope: PR #282 in thomasluizon/orbit-api (issue-380main)
Recommendation: APPROVE

Summary

The stated intent — prune two dead root-level docs (description.md, a path-collapsed
.txt orphan) — is correct and safe: both files are confirmed unreferenced anywhere in
the repo (git grep clean for description.md and for start-time), and README.md is
the canonical doc. However, the PR as it stands on GitHub also carries a second, unrelated
commit (da55cda, adding a "FEATURES.md parity" dimension to .claude/skills/pr-review/)
that the PR body never mentions. That commit is legitimate and correctly mirrors an
already-open frontend PR, but it belongs with a different issue/pairing than what's disclosed here.

Findings

Critical

None.

High

None.

Medium

[MEDIUM] PR bundles an undisclosed, unrelated commit from a different issue
· dimension: 1 (Correctness — diff vs. stated intent) / PR hygiene
· location: commit da55cda (.claude/skills/pr-review/SKILL.md, .claude/skills/pr-review/rubric.md)
· issue: The PR body only describes pruning description.md and the mangled .txt file
  (issue #380, paired with orbit-ui-mobile#388). The diff also includes a second
  commit, da55cda ("add FEATURES.md parity dimension (#14) to the pr-review rubric"),
  unrelated to #380 — it mirrors orbit-ui-mobile#386, a separate, still-open PR for
  issue #378. Neither the title nor the body discloses this second change.
· risk: A reviewer approving on the stated "prune-only, docs-only" description has no
  visibility that they are also merging a change to the review tooling itself. If
  orbit-ui-mobile#386 changes shape before merging, the two "mirrored" rubric copies
  can silently diverge with no PR-level trace back to why orbit-api's copy already has it.
· fix: Split da55cda into its own PR referencing issue #378 / orbit-ui-mobile#386, or
  at minimum update the PR body to disclose and justify bundling it here.
· reference: rubric.md dimension 1 (Correctness — does the diff match what the PR says)

Verified da55cda's content itself is sound: IAiTool, [McpServerTool],
PayGateService, and AppConstants all exist in src/, so the new dimension's
triggers reference real symbols, and the text matches what's already open in
orbit-ui-mobile#386. This is a process/disclosure finding, not a defect in the rubric text.

Low / Info

  • Info: description.md and the mangled .txt file are both confirmed dead (zero
    references anywhere in the tree) — clean, safe deletion.
  • Info: dimension 14's bullet order (general check before the orbit-api-specific
    "file not in checkout" override) reads slightly out of sequence on first pass but
    resolves correctly by the last bullet — not worth a fix.

Subagents

Agent Verdict
security-reviewer N/A — no src/ code changed
contract-aligner N/A — no DTO / route / shared-type change

Validation

Check Result
Build (dotnet) N/A — no .cs files touched; dotnet not invocable in this sandboxed session
Tests (dotnet) N/A — same as above

What's good

  • Both deleted files are genuinely dead weight: description.md is a stale
    point-in-time project snapshot from very early scaffolding (references a
    WeatherForecast stub, setup branch, etc.) fully superseded by README.md; the
    .txt file is an obvious path-collapse artifact.
  • The FEATURES.md rubric addition (once split out) is well-formed: gated correctly,
    references real backend symbols, and matches the sibling repo's already-open mirror
    PR (orbit-ui-mobile#386) verbatim.

Recommendation

Approve as-is — nothing here is functionally broken or risky. Before merging, consider
asking the author to split commit da55cda into its own PR (or amend the body) so the
issue-380 prune and the issue-378 rubric mirror each have their own disclosed,
traceable pairing with their respective frontend PRs.

@thomasluizon
thomasluizon merged commit 3830567 into main Jul 4, 2026
8 checks passed
@thomasluizon
thomasluizon deleted the issue-380 branch July 4, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant