-
Notifications
You must be signed in to change notification settings - Fork 13
docs: Update AGENTS.md #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- add repository overview and structure guidance - document instrumentation/test parity rules and workflows - include common scripts plus reference examples
Completed Working on "Auto PR Description"✅ Workflow completed successfully. |
Completed Working on "Code Review"✅ Workflow completed successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary:
- Findings: 0 BLOCKER, 0 CRITICAL, 3 MAJOR, 0 MINOR, 0 SUGGESTION, 0 PRAISE (3 comments across 1 file).
Key themes:
- Documentation overstates test coverage and instrumentation parity—statements about every instrumentation having matching specs and extending
BaseTraceInstrumentationdo not matchconsole-loggerimplementations. - Release guidance misrepresents the beta pipeline by claiming
releaseBeta.ymlrunssemantic-releasewhen it currently performs only a version bump plusnpm publish --tag beta.
Next steps:
- Either bring
console-loggerinstrumentation/test structure in line with the documented rules or explicitly carve out the exceptions so future contributors are not misled. - Update the instrumentation guidance to reflect the actual inheritance model (call out outliers) to avoid incorrect implementation patterns.
- Align the Quality Standards section with the real beta workflow, or adjust the workflow to match the described automation.
| ├── .eslintrc.js / .prettierrc # Linting & formatting rules | ||
| └── jest.config.js # Coverage thresholds (branches ≥80%, lines ≥90%) | ||
| ``` | ||
| > **Tip:** Every instrumentation file inside `src/trace/instrumentation/` has a matching `.spec.ts`, making it easy to track parity between runtime and tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: Both the repository tree tip and Critical Rule #1 claim that every file under src/trace/instrumentation has a sibling .spec.ts, yet console-logger.instrumentation.ts has no spec. The doc overpromises coverage and will misguide contributors enforcing those rules. Suggested fix: Either add console-logger.instrumentation.spec.ts or revise both sections to list console logger as an exception so the rule matches reality.
| Use `@Traceable()` similarly at the class level (see `src/trace/decorators/traceable.ts`) to auto-wrap every method. | ||
|
|
||
| ### Instrumentation classes | ||
| All instrumentations extend `BaseTraceInstrumentation` and implement the shared `Instrumentation` interface. For example, `ControllerInstrumentation` (`src/trace/instrumentation/controller.instrumentation.ts`) scans controller methods and wraps undecorated handlers: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: The Instrumentation classes guidance states that all instrumentation classes extend BaseTraceInstrumentation, but ConsoleLoggerInstrumentation only implements Instrumentation and directly patches the console logger. This inaccurate blanket statement will cause contributors to model new instrumentations after the wrong base class. Suggested fix: Clarify that console logger is the outlier (it does not inherit from the base class) or narrow the claim to the instrumentations that actually extend BaseTraceInstrumentation.
| ## Quality Standards | ||
| - **Coverage:** `jest.config.js` enforces `branches: 80` and `lines: 90`. Failing to meet these thresholds will break CI. | ||
| - **CI workflow:** `.github/workflows/ci.yml` runs `npm ci`, `npm run lint`, `npm run test:cov`, and `npm run build` on every PR against `main`. Keep scripts stable and deterministic. | ||
| - **Releases:** `release.yml` / `releaseBeta.yml` trigger `semantic-release`, so commits must follow Conventional Commit semantics for versioning to work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major]: The Quality Standards section says release.yml and releaseBeta.yml both trigger semantic-release, but .github/workflows/releaseBeta.yml only bumps the version and runs npm publish --tag beta. Documenting semantic-release here misleads contributors about the beta release gate and required commit semantics. Suggested fix: Either update releaseBeta.yml to call semantic-release or rewrite the guidance to describe the current manual npm publish flow for beta builds.
Summary
Adds the AGENTS.md guide so automation agents have an up-to-date reference for working in this repository.
Changes
Why
This keeps the autogenerated AGENTS.md file aligned with the current repository structure and practices, helping downstream automation remain accurate.
Summary
This PR introduces a comprehensive AGENTS.md guide that explains how automation agents should interact with the
@amplication/opentelemetry-nestjsrepository. It captures the project’s purpose, structure, workflows, and quality expectations so future automation stays aligned with current practices.Changes
Commits
Testing