Skip to content

docs: ARCHITECTURE map + contributor guide (GitHub Flow) - #13

Merged
prashar32 merged 1 commit into
mainfrom
docs/contributing
May 30, 2026
Merged

docs: ARCHITECTURE map + contributor guide (GitHub Flow)#13
prashar32 merged 1 commit into
mainfrom
docs/contributing

Conversation

@prashar32

Copy link
Copy Markdown
Owner

Makes the repo contributor-friendly and obvious where to code, and documents the GitHub Flow model (single main; fork → branch → PR to main; build & test + CodeQL + maintainer review gate every merge).

  • ARCHITECTURE.md (new) — one-diagram overview (deterministic core + three surfaces), the public/private boundary, an internal/ package tour, the governed-call request flow, and an "I want to… — where do I code?" table.
  • CONTRIBUTING.md (expanded) — dev setup (make targets), the PR flow, conventional commits, the trust rules (internal boundary, deterministic-only enforcement, no-telemetry egress incl. the approval webhook, honesty), and a pre-PR checklist.
  • README.md — a Contributing section linking both + good-first-issue + Code of Conduct.

No code or branch/ruleset changes — main is already set up for GitHub Flow.

Make the repo easy to contribute to and obvious where to code.

- ARCHITECTURE.md: one-diagram overview of the deterministic core + three
  surfaces, the public/private boundary, an internal/ package tour, the
  governed-call request flow, and a "where do I code?" table.
- CONTRIBUTING.md: dev setup (make targets), the GitHub Flow PR process
  (fork -> branch off main -> PR; build & test + CodeQL required + review),
  conventional commits, the trust rules (internal/ boundary, deterministic
  enforcement, no-telemetry egress incl. the approval webhook, honesty), and a
  pre-PR checklist.
- README: a Contributing section linking both + good-first-issue + CoC.
@prashar32

Copy link
Copy Markdown
Owner Author

@claude

@prashar32

Copy link
Copy Markdown
Owner Author

@claude

@claude

claude Bot commented May 30, 2026

Copy link
Copy Markdown

Claude finished @prashar32's task in 1m 57s —— View job


PR Review

  • Gather context and read changed files
  • Review ARCHITECTURE.md
  • Review CONTRIBUTING.md
  • Review README.md
  • Post final review feedback

Overall this is a high-quality documentation PR. The content is accurate, the structure is clear, and all cross-references are valid. A few observations:

ARCHITECTURE.md

Strengths:

  • The ASCII diagram accurately models the three ingress surfaces and internal request flow.
  • The package tour is complete — every package under internal/ is accounted for (governor, pricing, provider, runs, storage, approval, gateway, mcp, memory, otel, httpapi, config, httpx, id, version, app).
  • All linked files exist (docs/VISION.md, COMPATIBILITY.md).
  • The pkg/ — public Go packages (none yet) note is honest and sets the right expectation.

Minor nit — ASCII diagram right border:
In the diagram, the line:

budgets, kill switch │──▶ provider ──▶ LLM (your key)

The here is the box's right border, with ──▶ going outward. This is visually ambiguous — it reads as if is part of the flow arrow rather than a wall. Consider:

budgets, kill switch ├──▶ provider ──▶ LLM (your key)

...or placing the outbound arrow clearly outside the closing └─…─┘ line. Minor, but worth fixing for clarity. Fix this →

Minor nit — "I want to…" is a bullet list, not a table:
The PR description calls this a "table" but it's rendered as a bullet list. Not a bug — the bullet list is readable — but worth aligning the description with reality if this doc evolves.


CONTRIBUTING.md

Strengths:

  • All make targets are accurate against the Makefile (build, check, test, sdk-test, help are all real targets).
  • make check is correctly described as "gofmt check + go vet + race tests" (Makefile: check: fmt vet test).
  • The four trust rules (internal boundary, deterministic/LLM split, no-telemetry, honesty constraint) are well-articulated and internally consistent with ARCHITECTURE.md.
  • Pre-PR checklist is comprehensive and actionable.
  • All linked files exist (CHANGELOG.md, SECURITY.md, CODE_OF_CONDUCT.md).

Suggestion — fork vs. direct branch:
Step 1 says "Fork the repo and create a branch off main." Maintainers with write access would branch directly. A small parenthetical ("external contributors: fork first; maintainers can branch directly") avoids confusion for repo members. Fix this →

Suggestion — ./riskkernel serve after make build:
The getting-started block ends with ./riskkernel serve but doesn't mention that make build places the binary at the repo root. This is implied but could be made explicit with a one-line comment:

make build        # produces ./riskkernel at the repo root
./riskkernel serve

README.md

Strengths:

  • The new Contributing section is appropriately brief and links to both ARCHITECTURE.md and CONTRIBUTING.md.
  • Correctly describes the gate: CI (build & test + CodeQL) + maintainer review.
  • The good first issue link and Code of Conduct line set the right welcoming tone.

No issues.


Summary

This PR is ready to merge with the ASCII diagram nit and the fork-vs-branch clarification being the only actionable items. Everything else is accurate, complete, and consistent with the actual codebase structure and Makefile.

@prashar32
prashar32 merged commit a80c0e3 into main May 30, 2026
4 checks passed
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