Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
049efc2
Add import review signal mining skill
rohitkumarbhagat Aug 14, 2026
82d2679
Add Data Commons import review skill
rohitkumarbhagat Aug 14, 2026
0123785
Clarify import validation guidance and review contracts
rohitkumarbhagat Aug 14, 2026
9d11e9c
Document import review signal merge workflow
rohitkumarbhagat Aug 14, 2026
db44eaf
Strengthen data import code review guidelines
rohitkumarbhagat Aug 14, 2026
d8bcc2e
Require isolated Python environments in import code reviews
rohitkumarbhagat Aug 14, 2026
1bfc2af
Document agent skill authoring conventions
rohitkumarbhagat Aug 15, 2026
ad56c55
Clarify import diagnostics authoring
rohitkumarbhagat Aug 15, 2026
576a5d4
Discover skill link entrypoints from installed skills
rohitkumarbhagat Aug 15, 2026
c11cd8b
Merge branch 'master' into code-review-agent
rohitkumarbhagat Aug 15, 2026
0a0b0f6
lint fix
rohitkumarbhagat Aug 15, 2026
6511158
Allow explicitly authorized review publishing
rohitkumarbhagat Aug 15, 2026
7e56eea
Document shared download utility guidance
rohitkumarbhagat Aug 15, 2026
7e4941f
Clarify review target resolution in import code review skill
rohitkumarbhagat Aug 16, 2026
295fc0c
Refine import review skill guidance
rohitkumarbhagat Aug 17, 2026
afa479e
Document golden path review guidance
rohitkumarbhagat Aug 17, 2026
1ffa03a
docs: update import guidelines to include GCS artifact retention, sta…
rohitkumarbhagat Aug 17, 2026
a8aa0fd
docs: add requirement for node_mcf in manifest.json import_inputs
rohitkumarbhagat Aug 17, 2026
807f8e9
docs: add guidelines for mandatory date consistency and freshness val…
rohitkumarbhagat Aug 17, 2026
e52c8b3
docs: update import review guidelines to allow bounded, read-only GCS…
rohitkumarbhagat Aug 17, 2026
f17dc95
docs: add critical enforcement requirements for golden summaries and …
rohitkumarbhagat Aug 17, 2026
8f6693a
Merge branch 'master' into code-review-agent
rohitkumarbhagat Aug 18, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .agents/skills.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
{
"path": "agents/skills/dc-import-diagnostics"
},
{
"path": "agents/skills/dc-import-code-review"
},
Comment thread
rohitkumarbhagat marked this conversation as resolved.
{
"path": "agents/skills/dc-import-postmortem-doc"
}
Expand Down
39 changes: 39 additions & 0 deletions agents/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,42 @@ For read-only ET import information or diagnosis, use the
Copy the prompt into the agent conversation and append the specific import
question. The prompt routes the request through the repository-owned
`dc-import-diagnostics` skill and its bounded operational references.

## Review import changes

For a read-only review of staged, unstaged, branch-comparison, or pull request
changes under `scripts/**` and `statvar_imports/**`, use the
[`dc-import-code-review` starter prompt](prompts/dc-import-code-review-starter.md).
The skill returns P0-P3 findings, meaningful positive findings, coverage, and
verification.

## Mine import review signals

To collect positive and corrective review signals from merged import pull
requests, use the `dc-import-review-signal-miner` skill through its
[`starter prompt`](prompts/dc-import-review-signal-miner-starter.md). The skill
produces a complete comment audit and a projection containing only strong,
unambiguous signals. It does not update guidelines or create a pull request.

Before running the prompt, ensure the agent has network access to GitHub and
can write to a temporary directory and the selected output directory. Then run
the skill's
[`prerequisite checker`](skills/dc-import-review-signal-miner/scripts/check_prerequisites.sh):

```bash
bash agents/skills/dc-import-review-signal-miner/scripts/check_prerequisites.sh
```

The checker validates `git`, GitHub CLI, authentication, and the exact `gh`
capabilities used by the skill. The mining skill does not require standalone
`jq`, Python, Google Cloud CLI, GCS access, or write access to the Data Commons
repository. Its GitHub commands were tested with GitHub CLI 2.74.2; the checker
reports the installed version without treating 2.74.2 as a minimum.

## Merge import review signals

To merge considered signals into the import code-review guidelines, use the
[`import review signal merge prompt`](prompts/import-review-signal-merge.md).
Provide the miner output directory and the data repository path. The prompt
requires a clean current branch, leaves guideline edits uncommitted, and writes
a decision report beside the miner output.
71 changes: 71 additions & 0 deletions agents/common/scripts/skill_contract_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,5 +428,76 @@ def test_runtime_environment_registry_is_minimal_and_complete(self):
self.assertTrue(environment[section][field])


class ImportCodeReviewSkillContractTest(unittest.TestCase):

def setUp(self):
self._repo_root = Path(__file__).parents[3]
self._agents_root = self._repo_root / 'agents'
self._skill_root = (self._agents_root / 'skills/dc-import-code-review')
self._skill_path = self._skill_root / 'SKILL.md'
self._prompt_path = (self._agents_root /
'prompts/dc-import-code-review-starter.md')

def _read(self, relative_path: str) -> str:
return (self._repo_root / relative_path).read_text(encoding='utf-8')

def test_review_skill_is_registered_and_discoverable(self):
registry = json.loads(self._read('.agents/skills.json'))
paths = [entry['path'] for entry in registry['entries']]
readme = self._read('agents/README.md')
prompt = self._prompt_path.read_text(encoding='utf-8')

self.assertIn('agents/skills/dc-import-code-review', paths)
self.assertIn('prompts/dc-import-code-review-starter.md', readme)
self.assertIn('`dc-import-code-review` skill', prompt)

def test_review_skill_keeps_core_contract(self):
skill = self._skill_path.read_text(encoding='utf-8')

for marker in ('scripts/**', 'statvar_imports/**', 'read-only', 'P0',
'P1', 'P2', 'P3', 'Finding', 'Impact', 'Recommendation'):
with self.subTest(marker=marker):
self.assertIn(marker, skill)

links = {path for path, _, _ in _local_markdown_links(skill)}
self.assertTrue({
'references/guidelines.md',
'../../common/references/import-automation/manifest.md',
}.issubset(links))

def test_review_guidance_stays_single_and_lightweight(self):
references = sorted(path.name for path in (self._skill_root /
'references').glob('*.md'))
guidelines = (self._skill_root /
'references/guidelines.md').read_text(encoding='utf-8')

self.assertEqual(['guidelines.md'], references)
self.assertFalse((self._skill_root / 'README.md').exists())
self.assertFalse((self._skill_root / 'scripts').exists())

for stale_content in ('DCIR-', 'Evidence:', 'Last verified:',
'["support@datacommons.org"]', 'logging.fatal()',
'exit(1)'):
with self.subTest(stale_content=stale_content):
self.assertNotIn(stale_content, guidelines)

for validation_reference in (
'../../../../tools/import_validation/README.md',
'../../../../tools/import_validation/Validations.md'):
with self.subTest(validation_reference=validation_reference):
self.assertIn(validation_reference, guidelines)

def test_review_guidance_uses_only_relative_paths(self):
sources = [self._skill_path, self._prompt_path]
sources.extend((self._skill_root / 'references').glob('*.md'))

for source in sources:
text = source.read_text(encoding='utf-8')
with self.subTest(source=source.name):
self.assertNotIn('/Users/', text)
self.assertNotIn('file://', text)
self.assertNotIn('<REPO_ROOT>/', text)


if __name__ == '__main__':
unittest.main()
Comment thread
rohitkumarbhagat marked this conversation as resolved.
12 changes: 12 additions & 0 deletions agents/prompts/dc-import-code-review-starter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Start a Data Commons import code review

Use the `dc-import-code-review` skill to review the request below.

- Resolve the exact staged, unstaged, all-local, branch-comparison, or pull
request target before reviewing.
- Review only changed files under `scripts/**` and `statvar_imports/**`.
- Treat the repository and GitHub as read-only.

## Request

<IMPORT_REVIEW_REQUEST>
13 changes: 13 additions & 0 deletions agents/prompts/dc-import-review-signal-miner-starter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Start Data Commons import review-signal mining

Use the `dc-import-review-signal-miner` skill with these inputs:

- Start time, inclusive: `<START_TIME>` in ISO 8601 UTC.
- End time, exclusive: `<END_TIME>` in ISO 8601 UTC.
- Output directory: `<OUTPUT_DIRECTORY>`.
- Reviewer identities, optional: `<REVIEWERS>` as comma-separated GitHub
logins, numeric user IDs, or both.

Follow the skill's import-path boundary, conservative signal criteria, output
contracts, and read-only safety rules. Produce only the complete comments
report and the considered-signals projection.
99 changes: 99 additions & 0 deletions agents/prompts/import-review-signal-merge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# Merge mined import review signals

Apply strong recommendations produced by `dc-import-review-signal-miner` to
the Data Commons import code-review guidelines.

## Inputs

- Signal output directory: `<SIGNAL_OUTPUT_DIRECTORY>`
- Data repository path: `<DATA_REPOSITORY_PATH>`

Ask for either input if it is missing. Do not guess it.

## Check the repository

Resolve the repository root from `<DATA_REPOSITORY_PATH>`. Require a named
current branch and a clean checkout, including staged, unstaged, and untracked
files. If it is not clean, stop and list the dirty paths. Never stash, reset,
discard, or overwrite existing work.

Apply changes to the current branch. Do not fetch, switch, create, or delete a
branch. Do not stage, commit, push, or create a pull request.

Modify only
`agents/skills/dc-import-code-review/references/guidelines.md` in the data
repository.

## Select the signals

Require exactly one `import-review-signals-*.md` file in
`<SIGNAL_OUTPUT_DIRECTORY>`. If none or more than one exists, stop and ask the
user to disambiguate.

Treat each recommendation section in that projection as one signal. Use the
matching `import-review-comments-*.md` report only when more source context is
needed. Do not process comments marked `Not considered`, reclassify comments,
or modify either miner report.

## Merge the signals

Compare each signal semantically with the existing guidelines and current
repository implementation. Assign one disposition:

- `Added`: The recommendation is strong, general, new, and non-conflicting.
Add one concise, neutral recommendation bullet under the best existing
heading. Add a heading only when no existing heading fits.
- `Already covered`: An existing guideline expresses the same desired
behavior. Do not replace, rewrite, or duplicate it.
- `Conflict`: The recommendation contradicts existing guidance or current
implementation. Do not change the guidelines.
- `Skipped`: The recommendation is ambiguous, too specific, unsupported, or
cannot be merged safely. Do not change the guidelines.

If several signals support the same new guideline, add it once and report a
decision for every signal. If a signal contains a clearly separable new point,
add only that point. Otherwise prefer no change.

Keep the guidelines simple:

- Phrase positive and corrective signals as neutral recommendations.
- Preserve existing recommendations and organization.
- Do not add guideline IDs, severity, confidence, evidence metadata, dates,
reviewer identities, or source links.
- Do not duplicate generic repository language or style guidance.

## Write the merge report

Write
`<SIGNAL_OUTPUT_DIRECTORY>/import-review-signal-merge-<YYYYMMDD-HHMMSS>.md`
using the current UTC time. Do not overwrite an existing report.

Include a summary with the projection path, repository root, current branch,
HEAD commit, and disposition counts. Then include every signal using:

```markdown
### <SIGNAL RECOMMENDATION>

- Disposition: Added | Already covered | Conflict | Skipped
- Source comments: <URLS FROM THE PROJECTION>
- Existing guidance: <MATCHING OR CONFLICTING GUIDELINE> | None
- Change: <ADDED GUIDELINE BULLET> | None
- Rationale: <SPECIFIC REASON FOR THE DECISION>
```

End the report with checks run, checks not run, changed repository paths, and
explicit statements that changes were not staged, committed, pushed, or used
to create a pull request.

## Verify and finish

- Inspect the final diff and confirm no existing guideline was replaced.
- Confirm the only intended repository edit is `guidelines.md`. If the signal
output directory is inside the repository, treat the merge report as an
output artifact and never stage it.
- Run `git diff --check`.
- Run `python3 -m unittest agents.common.scripts.skill_contract_test` with the
repository's configured Python environment when available. Do not install
dependencies solely for this run; record unavailable checks in the report.
- Leave all changes unstaged and uncommitted on the current branch.
- Return the merge-report path, disposition counts, and verification results.
Loading
Loading