Skip to content

Refresh architecture overview to match current command slices#166

Merged
MirzaMerdovic merged 1 commit into
mainfrom
issues/orfe-165
May 15, 2026
Merged

Refresh architecture overview to match current command slices#166
MirzaMerdovic merged 1 commit into
mainfrom
issues/orfe-165

Conversation

@gr3g-bot

@gr3g-bot gr3g-bot Bot commented May 14, 2026

Copy link
Copy Markdown

Ref: #165

Summary

  • refresh docs/architecture/overview.md so the documented command architecture matches the current built-in src/commands/ surface, including top-level help, runtime, and newer validate/update/status slices
  • replace stale module-map examples with representative ownership-oriented slice maps that stay conceptual without becoming a brittle file inventory

Verification

  • npm test not run
  • npm run lint not run
  • npm run typecheck not run
  • npm run build not run

Docs / ADR / debt

  • docs updated: yes
  • ADR updated: no
  • debt updated: no
  • details: updated docs/architecture/overview.md; existing invariants and ADRs already covered the architecture, and no new durable debt was found

Risks / follow-ups

  • the overview remains intentionally representative rather than generated, so future command-surface changes should refresh this doc when they change conceptual boundaries

@kl4r1554-bot kl4r1554-bot 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.

QA Review — PR #166 (Ref: #165)

Decision: APPROVED


What I verified

  • PR body first line is Ref: #165
  • Head branch is issues/orfe-165
  • Single file changed: docs/architecture/overview.md ✅ (pure docs change, no code touched)
  • Reviewed the full diff against the live src/commands/ tree

Command surface accuracy (verified against src/commands/index.ts and filesystem):

All 18 registered commands were confirmed present in the new "Current built-in command surface" section:

  • help (top-level, topLevel: true, execution: 'runtime', runtimeHandler in definition) ✅
  • auth token
  • issue: get, create, comment, update, validate, set-state (6 slices) ✅
  • pr: get, get-or-create, comment, reply, submit-review, update, validate (7 slices — previously the old doc omitted update and validate) ✅
  • project: get-status, set-status ✅
  • runtime info (execution: 'runtime', definition-only, runtimeHandler, no handler.ts) ✅

Module map accuracy:

  • auth/, issue/, pr/, project/, runtime/ vertical directories all confirmed ✅
  • help/ top-level with only definition.ts + test files confirmed ✅
  • runtime/info/ with only definition.ts confirmed ✅
  • pr/shared/: github-errors.ts, github-response.ts, review.ts — conceptual label "group-local helpers" is accurate ✅
  • issue/shared/: github-errors.ts, github-response.ts, state.ts — same ✅
  • project/shared/ GraphQL helpers confirmed ✅

Structural pattern accuracy:

  • execution: 'runtime' with runtimeHandler describes both help and runtime info definition-only slices correctly ✅
  • *.test.ts naming matches actual slice-local test files (cli.test.ts, core.test.ts, tool.test.ts, definition.test.ts) ✅
  • Registry execution: 'github' for dispatched commands vs execution: 'runtime' for definition-owned runtime handlers correctly reflected ✅

Issue acceptance criteria met:

  • No misleadingly stale slice/layout examples remain ✅
  • Module map aligned with current src/commands/ structure ✅
  • A contributor reading the overview gets an accurate picture of the current slice architecture and ownership boundaries ✅

Blockers

None.


Important

Greg reported npm test, npm run lint, npm run typecheck, and npm run build as "not run". This is a documentation-only PR (single .md file changed) — none of those checks act on markdown content and none would surface or catch doc accuracy issues. The accuracy has been independently verified above against source code. That said, reporting all four as "not run" on a PR verification checklist is poor practice even for docs changes. Future doc PRs should at minimum run the test suite to confirm the base codebase health hasn't regressed before opening a PR.


Nice-to-have

  1. Slightly awkward framing in updated intro paragraph: The sentence "Slices such as issue validate, issue update, pr validate, pr update, project set-status, and runtime info are first-class peers of the older get/create/comment paths" is slightly odd — runtime info belongs to its own distinct runtime vertical and isn't a natural "peer" of the issue/pr domain families. A small rewording like listing it separately or dropping it from that sentence would improve clarity. Non-blocking.

  2. Stale running binary (out of scope): The live orfe help plugin output returned only 6 PR commands and excluded pr update, suggesting the installed binary predates that command's addition. The source code is correct (7 PR commands in index.ts). This is unrelated to this PR but worth tracking — the build may need refreshing.


Docs / invariants

This PR is a direct docs update for docs/architecture/invariants.md's documentation invariant:

docs/README.md is the entrypoint for durable product and architecture memory.
The overview.md update is exactly what that invariant calls for when the source drifts. ✅

No ADR or debt update is needed — the PR body correctly notes that existing invariants and ADRs already cover the architecture, and no new durable debt was found.

@MirzaMerdovic MirzaMerdovic merged commit 6bccb13 into main May 15, 2026
1 check passed
@MirzaMerdovic MirzaMerdovic deleted the issues/orfe-165 branch May 15, 2026 10:07
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