Add new agent skill cellix-tdd#186
Merged
Merged
Conversation
…arsing helpers - Introduced a new package `@cellix/query-params` to provide query-string parsing utilities. - Implemented `parseBooleanFlag` for boolean flag parsing with explicit error handling. - Implemented `parseStringList` for splitting and trimming comma-separated strings. - Added tests for both parsing functions to ensure expected behavior. - Created a manifest and README to document the package purpose, scope, and usage. refactor: internal refactor of @cellix/retry-policy package - Refactored the backoff calculation logic into a separate internal module. - Maintained the public API and ensured all existing tests passed. - Updated documentation to reflect internal changes while keeping the public contract stable. fix: resolve leaky API in @cellix/http-headers package - Removed unnecessary internal helper export from the package to maintain a clean public API. - Updated README and manifest to ensure alignment with the intended public contract. feat: create new @cellix/slugify package for URL-safe slug generation - Developed a new package `@cellix/slugify` to generate predictable slugs from display text. - Implemented the `slugify` function with options for separator control. - Added tests and documentation to support usage and examples. fix: address internal helper usage in tests for @cellix/command-router package - Removed direct imports of internal helpers in tests to comply with public contract testing rules. - Ensured all tests only interact with the public API of the package.
Contributor
Reviewer's GuideIntroduces the new cellix-tdd agent skill (workflow docs, rubric, evaluator, fixtures, and CLI wrappers) and tightens the @cellix/ui-core package’s public contract, documentation, and tests while enhancing shared Vitest configuration for unit and Storybook projects. Updated class diagram for ui-core public componentsclassDiagram
class ComponentQueryLoaderProps {
+Error error
+React.JSX.Element errorComponent
+boolean loading
+object hasData
+React.JSX.Element hasDataComponent
+React.JSX.Element noDataComponent
+number loadingRows
+React.JSX.Element loadingComponent
}
class ComponentQueryLoader {
+render(props: ComponentQueryLoaderProps) React.JSX.Element
}
class RequireAuthProps {
+React.JSX.Element children
+boolean forceLogin
}
class RequireAuth {
+render(props: RequireAuthProps) React.JSX.Element
}
ComponentQueryLoader ..> ComponentQueryLoaderProps : uses
RequireAuth ..> RequireAuthProps : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
- extract shared utilities (fileExists, directoryExists, getDefaultSummaryPath)
into evaluator/utils.ts to eliminate duplication across all three scripts
- fix findExportDeclarations to handle barrel/re-export patterns:
named re-exports (export { Name [as Alias] } from './source'),
star re-exports (export * from './source'), and export default.
follows one level deep with cycle detection via visited set.
TSDoc accepted at either re-export site or declaration site.
getPublicDeclarations now deduplicates by name across entry files.
- fix readmeConsumerFacing heading regex to accept common consumer-facing
headings: Getting Started, Installation, API, Overview, How to Use, Guide
- expand mentionsSurface regex to accept 'exporting' and 'exported'
in addition to the existing surface/export keyword set
- fix init-cellix-tdd-summary.ts readTemplate() to resolve the template
path via import.meta.url instead of process.cwd() so it works
regardless of working directory
- update leaky-overbroad-api fixture agent-output to have clearly
incomplete release hardening notes so all three keyword checks fail
cleanly after the mentionsSurface fix
- add Copilot Agent Notes section to SKILL.md covering: ask_user for
collaboration step, task/explore agent delegation for discovery
(including grep command for finding monorepo consumers), and the
iterative evaluator loop
- add .agents/skills/** and .github/skills/** to knip ignore so
evaluator scripts and fixture package source files are not treated
as unused workspace code, and the .github/skills symlink is not
followed into the skill directory
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- validate that the resolved package root is inside process.cwd() before constructing the default summary path; reject out-of-repo paths with a clear error so ../.. segments cannot cause the summary to be written outside the intended runs/ directory - change getPublicDeclarations dedup key from export name alone to filePath:name composite so that the same symbol name exported from different source files across multiple package entrypoints is checked independently; true duplicates (same source file re-exported via multiple entries) still collapse correctly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
content has been captured in .agents/skills/cellix-tdd/references/ Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…scope, and downstream impact checks - Add conditional Contract Gate step (step 3) between Consumer Usage Exploration and Public Contract Definition - Gate triggers on new packages, removed/renamed exports, material behavioral changes, dependent breakage, or uncertainty; skips for clearly additive low-risk changes - Add two-question export justification requirement to Public Contract Definition - Clarify semver policy: do not justify breaking changes because pre-1.0; additive changes may still be non-breaking but evaluate conservatively - Add evaluator scope disclaimer to step 9, Validation Expectations, and Copilot Agent Notes: passing score ≠ contract correctness or publication readiness - Add downstream dependent discovery (rg) to Discovery First section and verification check to Release Hardening - Add three new anti-patterns: skipping contract gate when warranted, test-driven export widening, treating evaluator pass as publish approval - Update evaluator requiredOutputSections, summary template, and all 6 fixture agent-outputs to include contract gate summary - Fix paragraph formatting in Copilot Agent Notes iterative evaluation section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… comprehensive documentation, improved component APIs, and public contract testing
…ns in Vitest configs
…nization, and validation processes after first trial usage - Updated SKILL.md to clarify documentation and testing requirements for public exports. - Enhanced evaluate-cellix-tdd.ts to ensure tests are grouped by exported member and to improve TSDoc quality checks. - Added new fixtures for ambiguous flat tests, including README and manifest files for example @cellix/network-endpoint. - Improved validation summaries across various agent outputs to include package build and existing test confirmations. - Revised rubric.md to reflect stricter requirements for documentation alignment and public contract testing. - Updated summary-template.md to guide on documenting test plans and validation steps more comprehensively.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
evaluate-cellix-tdd.tsfile is quite large and mixes CLI concerns, filesystem traversal, markdown parsing, and TypeScript export analysis; consider splitting this into smaller modules (e.g., CLI/arg parsing, package/introspection utilities, markdown/docs analysis, scoring) to improve readability and maintainability. - Both
evaluate-cellix-tdd.tsandcheck-cellix-tdd.tsimplement their own argument parsing and usage printing; factoring this into a shared helper (or at least a common argument/usage module) would reduce duplication and the risk of the two CLIs drifting in behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `evaluate-cellix-tdd.ts` file is quite large and mixes CLI concerns, filesystem traversal, markdown parsing, and TypeScript export analysis; consider splitting this into smaller modules (e.g., CLI/arg parsing, package/introspection utilities, markdown/docs analysis, scoring) to improve readability and maintainability.
- Both `evaluate-cellix-tdd.ts` and `check-cellix-tdd.ts` implement their own argument parsing and usage printing; factoring this into a shared helper (or at least a common argument/usage module) would reduce duplication and the risk of the two CLIs drifting in behavior.
## Individual Comments
### Comment 1
<location path=".agents/skills/cellix-tdd/fixtures/new-package-greenfield/package/src/index.ts" line_range="20" />
<code_context>
+ export function slugify(input: string, options: SlugifyOptions = {}): string {
</code_context>
<issue_to_address>
**issue (bug_risk):** The `slugify` implementation is missing method chaining dots before `replace`, which will cause a syntax error.
Those `replace` calls must be chained as methods on the string; otherwise this won’t parse:
```ts
return input
.trim()
.toLowerCase()
.replace(/[^a-z0-9]+/g, separator)
.replace(new RegExp(`${separator}+`, "g"), separator)
.replace(new RegExp(`^${separator}|${separator}$`, "g"), "");
```
As written, the file will fail to parse and break the fixture package.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… & unit tests, add integration runner Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
evaluate-cellix-tdd.tsfile is very large and mixes CLI handling, package scanning, export parsing, and scoring logic; consider extracting the core evaluation logic (export discovery, TSDoc analysis, test/import analysis) into smaller reusable modules to keep the entrypoint thin and easier to maintain. - Argument parsing for the evaluator is duplicated between
evaluate-cellix-tdd.ts(customparseArgs) and the sharedcli-utils.tshelpers; wiringevaluate-cellix-tdd.tstoparseEvaluateArgswould reduce drift and keep behavior consistent across the CLI tools.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `evaluate-cellix-tdd.ts` file is very large and mixes CLI handling, package scanning, export parsing, and scoring logic; consider extracting the core evaluation logic (export discovery, TSDoc analysis, test/import analysis) into smaller reusable modules to keep the entrypoint thin and easier to maintain.
- Argument parsing for the evaluator is duplicated between `evaluate-cellix-tdd.ts` (custom `parseArgs`) and the shared `cli-utils.ts` helpers; wiring `evaluate-cellix-tdd.ts` to `parseEvaluateArgs` would reduce drift and keep behavior consistent across the CLI tools.
## Individual Comments
### Comment 1
<location path=".agents/skills/cellix-tdd/evaluator/cli-utils.ts" line_range="18-23" />
<code_context>
+ json: false,
+ };
+
+ for (let index = 0; index < argv.length; index += 1) {
+ const arg = argv[index];
+ const next = argv[index + 1];
+
+ switch (arg) {
+ case "--":
+ break;
+ case "--fixture":
</code_context>
<issue_to_address>
**issue (bug_risk):** The "--" sentinel is ignored rather than terminating option parsing.
In `parseEvaluateArgs` and `parseCheckArgs`, `case "--": break;` only exits the `switch`, so arguments after `--` are still parsed as options and can cause `Unknown argument` errors. If `--` is intended to terminate option parsing, you should break out of the loop at that point (e.g. by advancing `index` to `argv.length` or returning early).
</issue_to_address>
### Comment 2
<location path=".agents/skills/cellix-tdd/evaluator/check-cellix-tdd.ts" line_range="38-39" />
<code_context>
+
+ const packageRoot = resolve(args.packageRoot);
+ const outputPath = args.outputPath ? resolve(args.outputPath) : getDefaultSummaryPath(packageRoot);
+ const initScriptPath = new URL("./init-cellix-tdd-summary.ts", import.meta.url);
+ const evaluateScriptPath = new URL("./evaluate-cellix-tdd.ts", import.meta.url);
+
+ if (!fileExists(outputPath) || args.forceInit) {
</code_context>
<issue_to_address>
**issue:** Using URL.pathname directly for script paths may be brittle on Windows.
Here `initScriptPath.pathname` and `evaluateScriptPath.pathname` are passed directly to `spawnSync`. On Windows, `file:` URL pathnames can include a leading `/` (e.g. `/C:/...`), breaking path resolution. Use `fileURLToPath(initScriptPath)` / `fileURLToPath(evaluateScriptPath)` from `node:url` instead of `.pathname` for cross‑platform safety.
</issue_to_address>
### Comment 3
<location path="packages/cellix/ui-core/tests/index.test.tsx" line_range="119-128" />
<code_context>
+ describe('RequireAuth', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add RequireAuth tests for forceLogin side effects and auth-parameter edge cases
The current tests only cover the visible states of `RequireAuth`. To fully exercise the documented contract, please also add tests for:
- `forceLogin` side effects: when unauthenticated with `forceLogin=true`, verify `sessionStorage.redirectTo` is set to the current route (including query) before `signinRedirect()` and that nothing is rendered.
- In-progress redirects: when `hasAuthParamsMock` is `true` or `activeNavigator` indicates a redirect in progress, assert that `signinRedirect` is not called again and that the UI shows the appropriate loading/blank state.
- Error precedence: when both `error` is set and `isAuthenticated=false`, confirm the error-redirect branch takes precedence.
These will align the tests with the documented auth-gating behavior and protect the routing side effects your consumers rely on.
Suggested implementation:
```typescript
describe('RequireAuth', () => {
it('renders a loading placeholder while auth state is resolving', () => {
useAuthMock.mockReturnValue(createAuthState({ isLoading: true }));
const html = renderToStaticMarkup(
<RequireAuth forceLogin={false}>
<div>Private content</div>
</RequireAuth>,
);
expect(html).toContain('Please wait...');
});
it('stores redirect target and triggers signinRedirect when unauthenticated with forceLogin=true', () => {
const signinRedirectSpy = jest.fn();
useAuthMock.mockReturnValue(
createAuthState({
isAuthenticated: false,
signinRedirect: signinRedirectSpy,
}),
);
const originalLocation = window.location;
// @ts-expect-error overwrite for test
delete (window as any).location;
(window as any).location = {
...originalLocation,
pathname: '/private',
search: '?foo=bar',
};
sessionStorage.clear();
const html = renderToStaticMarkup(
<RequireAuth forceLogin={true}>
<div>Private content</div>
</RequireAuth>,
);
expect(sessionStorage.getItem('redirectTo')).toBe('/private?foo=bar');
expect(signinRedirectSpy).toHaveBeenCalledTimes(1);
// component should not render children while redirecting
expect(html).not.toContain('Private content');
(window as any).location = originalLocation;
});
it('does not trigger signinRedirect again when auth parameters are already present', () => {
const signinRedirectSpy = jest.fn();
hasAuthParamsMock.mockReturnValue(true);
useAuthMock.mockReturnValue(
createAuthState({
isAuthenticated: false,
signinRedirect: signinRedirectSpy,
}),
);
const html = renderToStaticMarkup(
<RequireAuth forceLogin={true}>
<div>Private content</div>
</RequireAuth>,
);
expect(signinRedirectSpy).not.toHaveBeenCalled();
// should render a loading/blank state instead of children
expect(html).toContain('Please wait');
expect(html).not.toContain('Private content');
});
it('does not trigger signinRedirect again when a redirect is already in progress', () => {
const signinRedirectSpy = jest.fn();
useAuthMock.mockReturnValue(
createAuthState({
isAuthenticated: false,
activeNavigator: 'signinRedirect',
signinRedirect: signinRedirectSpy,
}),
);
const html = renderToStaticMarkup(
<RequireAuth forceLogin={true}>
<div>Private content</div>
</RequireAuth>,
);
expect(signinRedirectSpy).not.toHaveBeenCalled();
expect(html).toContain('Please wait');
expect(html).not.toContain('Private content');
});
it('prefers the error branch over unauthenticated forceLogin behaviour', () => {
const signinRedirectSpy = jest.fn();
useAuthMock.mockReturnValue(
createAuthState({
isAuthenticated: false,
error: new Error('Boom'),
signinRedirect: signinRedirectSpy,
}),
);
const html = renderToStaticMarkup(
<RequireAuth forceLogin={true}>
<div>Private content</div>
</RequireAuth>,
);
// When an error is present, the error-handling branch should win
expect(signinRedirectSpy).not.toHaveBeenCalled();
expect(html).not.toContain('Private content');
// Adjust this assertion to match the actual error UI / redirect marker
expect(html.toLowerCase()).toContain('error');
});
```
These edits assume the following about the existing test harness and implementation:
1. `createAuthState` accepts `signinRedirect` and `activeNavigator` properties and passes them through to the `RequireAuth` implementation. If it does not currently accept those properties, you should extend it accordingly in the test utilities.
2. `hasAuthParamsMock` is a jest mock used by `RequireAuth` to detect existing auth parameters. If it has a different name or is imported from another module, adjust the `hasAuthParamsMock.mockReturnValue(true);` line to match.
3. The loading state text is asserted using `'Please wait'`. If your loading UI uses a different copy or a dedicated `data-*` attribute, update the `toContain` assertions to align with the actual markup.
4. The error precedence test currently checks `html.toLowerCase()).toContain('error')` as a generic assertion. If `RequireAuth` instead redirects (e.g. via a mocked `navigate` function) or renders a specific error component, replace that assertion with whatever signal your existing tests/assertions use for the error branch (for example, an error route pathname, a `data-kind="error"` marker, or a specific message).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
|
Sorry @nnoce14, your pull request is larger than the review limit of 150000 diff characters |
1 similar comment
|
Sorry @nnoce14, your pull request is larger than the review limit of 150000 diff characters |
…vitest - Added @ts-expect-error comment for react-dom/server import since react-dom v19 doesn't include bundled type definitions for server module - Added skipLibCheck: true to base vitest tsconfig to suppress lib type errors during typecheck phase - This is the proper approach for handling third-party libraries without complete type definitions All 38 tests pass with 100% statement coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Simplified auth params test: verify signinRedirect is called exactly once from the else branch (not from useEffect due to SSR limitations with renderToStaticMarkup) - Removed sessionStorage assertions that can't work in SSR context - Added @ts-ignore for react-dom/server import since react-dom v19 doesn't include type definitions for its server exports All 38 tests pass with 100% statement coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
@sourcery-ai review |
Contributor
|
Sorry @nnoce14, your pull request is larger than the review limit of 150000 diff characters |
1 similar comment
|
Sorry @nnoce14, your pull request is larger than the review limit of 150000 diff characters |
…ording to copilot review suggestions
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new cellix-tdd agent skill (with rubric, fixtures, and evaluator tooling) while tightening @cellix/ui-core’s public contract to root-only exports and refining shared Vitest/Storybook configuration to better separate unit vs browser/storybook testing.
Changes:
- Introduce
.agents/skills/cellix-tdd(workflow docs, rubric, fixtures, evaluator CLI + tests) and wire new root scripts to run checks/tests. - Narrow
@cellix/ui-coreexports to the root entrypoint, add a manifest + consumer-focused README, and add root-import contract tests. - Refactor
@cellix/config-vitestconfigs (default typecheck config, split unit vs storybook projects) and related TS config tweaks for Storybook/tests.
Reviewed changes
Copilot reviewed 95 out of 97 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates lockfile for dependency graph changes (storybook/vitest/react types, etc.). |
| packages/cellix/ui-core/vitest.config.ts | Merges Storybook Vitest config with an alias so tests import from @cellix/ui-core root entrypoint. |
| packages/cellix/ui-core/tsconfig.vitest.json | Adds TS path mapping for @cellix/ui-core during tests. |
| packages/cellix/ui-core/tsconfig.json | Excludes Storybook story files from compilation. |
| packages/cellix/ui-core/tests/require-auth.effect.test.tsx | Adds effect-focused contract test for RequireAuth redirect storage behavior. |
| packages/cellix/ui-core/tests/index.test.tsx | Adds root-entrypoint contract tests for ComponentQueryLoader and RequireAuth. |
| packages/cellix/ui-core/src/components/molecules/require-auth/index.tsx | Adds richer TSDoc for RequireAuth and its props. |
| packages/cellix/ui-core/src/components/molecules/index.ts | Adds molecules barrel export for root-only public contract. |
| packages/cellix/ui-core/src/components/molecules/component-query-loader/index.tsx | Adds richer TSDoc for ComponentQueryLoader and its props. |
| packages/cellix/ui-core/package.json | Removes wildcard deep export path and adds missing React DOM/testing deps. |
| packages/cellix/ui-core/manifest.md | Adds a manifest documenting scope, boundaries, and release readiness standards. |
| packages/cellix/ui-core/README.md | Rewrites README to be consumer-facing and root-import oriented. |
| packages/cellix/config-vitest/src/configs/storybook.config.ts | Splits config into unit + storybook projects and disables typecheck in browser/storybook project. |
| packages/cellix/config-vitest/src/configs/node.config.ts | Aligns node config include patterns and enables default typecheck config. |
| packages/cellix/config-vitest/src/configs/base.config.ts | Extracts default include patterns and a reusable typecheck config factory. |
| packages/cellix/config-vitest/package.json | Reclassifies tooling packages (including vitest) into devDependencies. |
| packages/cellix/config-typescript/tsconfig.vitest.json | Enables skipLibCheck for Vitest TS config. |
| package.json | Adds scripts to run cellix-tdd evaluator/checks and adjusts formatting in scripts block. |
| knip.json | Expands global ignores to include .agents/** and .github/** and removes obsolete ignoreIssues entries. |
| .github/workflows/copilot-setup-steps.yml | Bumps pnpm version for CI setup. |
| .agents/skills/cellix-tdd/templates/summary-template.md | Adds a scaffold template for required workflow summary sections. |
| .agents/skills/cellix-tdd/runs/.gitignore | Ignores generated run outputs while keeping .gitignore tracked. |
| .agents/skills/cellix-tdd/rubric.md | Defines scoring rubric and required checks for evaluator. |
| .agents/skills/cellix-tdd/references/package-manifest-template.md | Adds manifest template reference. |
| .agents/skills/cellix-tdd/references/package-docs-model.md | Adds docs layering reference (manifest/README/TSDoc alignment). |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/prompt.md | Adds fixture scenario description. |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/package/src/internal/build-route-key.ts | Fixture internal helper implementation. |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/package/src/index.ts | Fixture public API that uses internal helper. |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/package/src/index.test.ts | Fixture test intentionally violating “no internal imports” rule. |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/package/package.json | Fixture package metadata/exports. |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/package/manifest.md | Fixture manifest. |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/package/README.md | Fixture consumer README. |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/expected-report.json | Fixture expected evaluator outcome. |
| .agents/skills/cellix-tdd/fixtures/tempting-internal-helper/agent-output.md | Fixture sample agent output. |
| .agents/skills/cellix-tdd/fixtures/new-package-greenfield/prompt.md | Adds fixture scenario description. |
| .agents/skills/cellix-tdd/fixtures/new-package-greenfield/package/src/index.ts | Fixture public API + TSDoc. |
| .agents/skills/cellix-tdd/fixtures/new-package-greenfield/package/src/index.test.ts | Fixture contract tests. |
| .agents/skills/cellix-tdd/fixtures/new-package-greenfield/package/package.json | Fixture package metadata/exports. |
| .agents/skills/cellix-tdd/fixtures/new-package-greenfield/package/manifest.md | Fixture manifest. |
| .agents/skills/cellix-tdd/fixtures/new-package-greenfield/package/README.md | Fixture consumer README. |
| .agents/skills/cellix-tdd/fixtures/new-package-greenfield/expected-report.json | Fixture expected evaluator outcome. |
| .agents/skills/cellix-tdd/fixtures/new-package-greenfield/agent-output.md | Fixture sample agent output. |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/prompt.md | Adds fixture scenario description. |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/package/src/internal/normalize-header-name.ts | Fixture internal helper implementation. |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/package/src/index.ts | Fixture public API that imports internal helper. |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/package/src/index.test.ts | Fixture contract tests. |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/package/package.json | Fixture package exports include an internal subpath (intentional violation). |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/package/manifest.md | Fixture manifest. |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/package/README.md | Fixture consumer README. |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/expected-report.json | Fixture expected evaluator outcome. |
| .agents/skills/cellix-tdd/fixtures/leaky-overbroad-api/agent-output.md | Fixture sample agent output. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/prompt.md | Adds fixture scenario description. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/package/src/internal/backoff.ts | Fixture internal refactor target. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/package/src/index.ts | Fixture public API using internal helper. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/package/src/index.test.ts | Fixture contract tests. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/package/package.json | Fixture package metadata/exports. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/package/manifest.md | Fixture manifest. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/package/README.md | Fixture consumer README. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/expected-report.json | Fixture expected evaluator outcome. |
| .agents/skills/cellix-tdd/fixtures/existing-package-internal-refactor/agent-output.md | Fixture sample agent output. |
| .agents/skills/cellix-tdd/fixtures/existing-package-add-feature/prompt.md | Adds fixture scenario description. |
| .agents/skills/cellix-tdd/fixtures/existing-package-add-feature/package/src/index.ts | Fixture public API for additive feature scenario. |
| .agents/skills/cellix-tdd/fixtures/existing-package-add-feature/package/src/index.test.ts | Fixture contract tests. |
| .agents/skills/cellix-tdd/fixtures/existing-package-add-feature/package/package.json | Fixture package metadata/exports. |
| .agents/skills/cellix-tdd/fixtures/existing-package-add-feature/package/manifest.md | Fixture manifest. |
| .agents/skills/cellix-tdd/fixtures/existing-package-add-feature/package/README.md | Fixture consumer README. |
| .agents/skills/cellix-tdd/fixtures/existing-package-add-feature/expected-report.json | Fixture expected evaluator outcome. |
| .agents/skills/cellix-tdd/fixtures/existing-package-add-feature/agent-output.md | Fixture sample agent output. |
| .agents/skills/cellix-tdd/fixtures/docs-lagging-implementation/prompt.md | Adds fixture scenario description. |
| .agents/skills/cellix-tdd/fixtures/docs-lagging-implementation/package/src/index.ts | Fixture public API with docs drift. |
| .agents/skills/cellix-tdd/fixtures/docs-lagging-implementation/package/src/index.test.ts | Fixture contract tests. |
| .agents/skills/cellix-tdd/fixtures/docs-lagging-implementation/package/package.json | Fixture package metadata/exports. |
| .agents/skills/cellix-tdd/fixtures/docs-lagging-implementation/package/manifest.md | Fixture manifest. |
| .agents/skills/cellix-tdd/fixtures/docs-lagging-implementation/package/README.md | Fixture README intentionally not consumer-facing (violation). |
| .agents/skills/cellix-tdd/fixtures/docs-lagging-implementation/expected-report.json | Fixture expected evaluator outcome. |
| .agents/skills/cellix-tdd/fixtures/docs-lagging-implementation/agent-output.md | Fixture sample agent output. |
| .agents/skills/cellix-tdd/fixtures/ambiguous-flat-tests/prompt.md | Adds fixture scenario description. |
| .agents/skills/cellix-tdd/fixtures/ambiguous-flat-tests/package/src/index.ts | Fixture public API for test-structure scenario. |
| .agents/skills/cellix-tdd/fixtures/ambiguous-flat-tests/package/src/index.test.ts | Fixture intentionally ambiguous tests (violation). |
| .agents/skills/cellix-tdd/fixtures/ambiguous-flat-tests/package/package.json | Fixture package metadata/exports. |
| .agents/skills/cellix-tdd/fixtures/ambiguous-flat-tests/package/manifest.md | Fixture manifest. |
| .agents/skills/cellix-tdd/fixtures/ambiguous-flat-tests/package/README.md | Fixture consumer README. |
| .agents/skills/cellix-tdd/fixtures/ambiguous-flat-tests/expected-report.json | Fixture expected evaluator outcome. |
| .agents/skills/cellix-tdd/fixtures/ambiguous-flat-tests/agent-output.md | Fixture sample agent output. |
| .agents/skills/cellix-tdd/fixtures/README.md | Documents fixture structure and how to run the suite. |
| .agents/skills/cellix-tdd/evaluator/vitest.config.ts | Adds evaluator unit test config. |
| .agents/skills/cellix-tdd/evaluator/utils.ts | Adds file/path utilities (existence checks + default output path). |
| .agents/skills/cellix-tdd/evaluator/tests/markdown-utils.test.ts | Adds unit tests for markdown parsing helpers. |
| .agents/skills/cellix-tdd/evaluator/run-integration.js | Adds integration runner to verify fixtures match expected reports. |
| .agents/skills/cellix-tdd/evaluator/markdown-utils.ts | Adds markdown parsing/template detection helpers. |
| .agents/skills/cellix-tdd/evaluator/init-cellix-tdd-summary.ts | Adds CLI to scaffold a summary file from the template. |
| .agents/skills/cellix-tdd/evaluator/evaluate-cellix-tdd.ts | Implements the rubric evaluator and fixture verification logic. |
| .agents/skills/cellix-tdd/evaluator/cli-utils.ts | Adds shared CLI argument parsing for evaluator/check commands. |
| .agents/skills/cellix-tdd/evaluator/check-cellix-tdd.ts | Adds top-level “check” command that scaffolds then evaluates. |
| .agents/skills/cellix-tdd/SKILL.md | Adds the skill workflow specification and required output structure. |
| .agents/skills/README.md | Documents the new skill and discovery/verification guidance. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
…ng in cellix-tdd evaluator according to copilot review suggestions
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 101 out of 103 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/cellix/config-vitest/src/configs/storybook.config.ts:65
- In the multi-project setup, the
storybookproject doesn't settest.include. Withextends: true, Vitest will fall back to its default include patterns, which can cause the storybook project to either (a) rerun unit tests in browser mode or (b) run nothing story-related depending on defaults/plugin behavior. Consider setting an explicitinclude/excludefor thestorybookproject so only story-derived tests run there, and unit tests run only in theunitproject.
Member
Author
|
@copilot review |
Contributor
Here is the updated review. CI is pending (CellixJs build in progress), CodeQL and Analyze are both green. Overall AssessmentAll previous review feedback has been addressed cleanly. The PR is in good shape. Addressed Feedback — Confirmed
New Changes Since Last Review
|
nnoce14
force-pushed
the
nnoce14/issue183
branch
from
10f5032 to
c67b8cd
Compare
gidich
approved these changes
Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce a Cellix TDD agent skill with evaluator and fixtures, tighten @cellix/ui-core’s public contract and documentation, and refine shared Vitest/test configuration for package and Storybook testing.
New Features:
Enhancements:
Build:
Documentation:
Tests: