dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix: avoid code-search quota for new requirements (#3883)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-09-23 17:30:09 +01:00
committed by GitHub
parent 25633de627
commit a9e069ada4
2 changed files with 118 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/scripts/pr-docs.cjs
+72 -24
View File
@@ -62,7 +62,8 @@ const WORK_ORDER_REQUIRED_FIELDS = [
'system_design',
];
const MAX_DOCUMENT_BYTES = 256 * 1024;
const MAX_DOCUMENTS = 100;
const MAX_DOCUMENTS = 200;
const MAX_CHANGED_WORK_ORDERS = 100;
const MAX_TOTAL_DOCUMENT_BYTES = 4 * 1024 * 1024;
const MAX_RESPONSE_BYTES = 8 * 1024 * 1024;
const MAX_CHANGED_FILES = 3000;
@@ -844,8 +845,8 @@ function validateCoverage({ changedFiles = [], fileContents = {} } = {}) {
}
result.workOrders = selectChangedWorkOrders(changedFiles);
if (result.workOrders.length > MAX_DOCUMENTS) {
result.errors.push(`More than ${MAX_DOCUMENTS} changed work orders were supplied`);
if (result.workOrders.length > MAX_CHANGED_WORK_ORDERS) {
result.errors.push(`More than ${MAX_CHANGED_WORK_ORDERS} changed work orders were supplied`);
}
if (result.workOrders.length === 0) {
result.status = 'missing';
@@ -1506,7 +1507,8 @@ function changedRequirementSources(changedFiles, requirementDirectory) {
}
const status = typeof change === 'string' ? undefined : change?.status;
const headPath = status === 'removed' ? undefined : normalizedCurrent;
let basePath = normalizedCurrent;
const isAdded = status === 'added';
let basePath = isAdded ? undefined : normalizedCurrent;
if (status === 'renamed' && typeof change.previous_filename === 'string') {
try {
basePath = normalizeRepoPath(change.previous_filename);
@@ -1525,12 +1527,18 @@ function changedRequirementSources(changedFiles, requirementDirectory) {
}
const key = normalizedHead ?? `base:${normalizedBase}`;
if (!sources.has(key)) {
sources.set(key, { basePath: normalizedBase, headPath: normalizedHead });
sources.set(key, { basePath: normalizedBase, headPath: normalizedHead, isAdded });
}
}
return [...sources.values()];
}
function candidateRequirementPath(requirementDirectory, requirementId) {
const parts = requirementId.split('-');
const filename = `${parts.slice(2).join('-').replace(/-\d+$/, '').toLowerCase()}.md`;
return `${requirementDirectory}/${filename}`;
}
async function loadCoverageContents({ client, changedFiles, headSha, baseSha }) {
if (!classifyChangedFiles(changedFiles).requiresCoverage) {
return {};
@@ -1581,6 +1589,22 @@ async function loadCoverageContents({ client, changedFiles, headSha, baseSha })
return content;
}
async function listRequirementDirectory(requirementDirectory, ref) {
const key = JSON.stringify([requirementDirectory, ref]);
if (!requirementDirectories.has(key)) {
let entries = [];
try {
entries = await client.listDirectory(requirementDirectory, ref);
} catch (error) {
if (!isMissingResourceError(error)) {
throw error;
}
}
requirementDirectories.set(key, entries);
}
return requirementDirectories.get(key);
}
for (const workOrderPath of selectChangedWorkOrders(changedFiles)) {
const workOrderContent = await load(workOrderPath, headSha, contents);
if (workOrderContent === undefined) {
@@ -1643,9 +1667,13 @@ async function loadCoverageContents({ client, changedFiles, headSha, baseSha })
const requirementDirectory = `docs/specs/${system}/requirements`;
const requirementPaths = new Set();
const baseContentByHeadPath = new Map();
const addedRequirementPaths = new Set();
for (const source of changedRequirementSources(changedFiles, requirementDirectory)) {
if (source.headPath) {
requirementPaths.add(source.headPath);
if (source.isAdded) {
addedRequirementPaths.add(source.headPath);
}
await load(source.headPath, headSha, contents);
}
if (source.basePath) {
@@ -1655,13 +1683,37 @@ async function loadCoverageContents({ client, changedFiles, headSha, baseSha })
}
}
}
const checkedAddedRequirementPaths = new Set();
if (addedRequirementPaths.size > 0 && typeof client.listDirectory === 'function') {
const entries = await listRequirementDirectory(requirementDirectory, headSha);
for (const entry of entries) {
if (
entry.type !== 'file'
|| typeof entry.path !== 'string'
|| !entry.path.startsWith(`${requirementDirectory}/`)
|| !entry.path.endsWith('.md')
) {
continue;
}
const pathname = normalizeRepoPath(entry.path);
await load(pathname, headSha, contents);
if (addedRequirementPaths.has(pathname)) {
checkedAddedRequirementPaths.add(pathname);
}
}
}
const workOrderRequirements = new Set(workOrder.requirements ?? []);
const referencedRequirementIds = designRequirements.filter(requirementId =>
workOrderRequirements.has(requirementId)
);
const verifiedRequirementIds = new Set();
const ambiguousRequirementIds = new Set();
for (const requirementId of referencedRequirementIds) {
const definitions = requirementDefinitions(contents, requirementId, system);
if (definitions.length > 1) {
ambiguousRequirementIds.add(requirementId);
continue;
}
if (definitions.length !== 1) {
continue;
}
@@ -1669,7 +1721,13 @@ async function loadCoverageContents({ client, changedFiles, headSha, baseSha })
const baseContent = baseContentByHeadPath.get(definition.pathname);
if (
hasRequirementHeading(definition.content, requirementId)
&& hasRequirementHeading(baseContent, requirementId)
&& (
hasRequirementHeading(baseContent, requirementId)
|| (
checkedAddedRequirementPaths.has(definition.pathname)
&& definition.pathname === candidateRequirementPath(requirementDirectory, requirementId)
)
)
) {
verifiedRequirementIds.add(requirementId);
}
@@ -1677,7 +1735,7 @@ async function loadCoverageContents({ client, changedFiles, headSha, baseSha })
const unresolvedRequirementIds = [];
if (typeof client.searchCode === 'function') {
for (const requirementId of referencedRequirementIds) {
if (verifiedRequirementIds.has(requirementId)) {
if (verifiedRequirementIds.has(requirementId) || ambiguousRequirementIds.has(requirementId)) {
continue;
}
const searchKey = JSON.stringify([requirementDirectory, requirementId]);
@@ -1697,26 +1755,16 @@ async function loadCoverageContents({ client, changedFiles, headSha, baseSha })
}
} else {
unresolvedRequirementIds.push(
...referencedRequirementIds.filter(requirementId => !verifiedRequirementIds.has(requirementId)),
...referencedRequirementIds.filter(requirementId =>
!verifiedRequirementIds.has(requirementId) && !ambiguousRequirementIds.has(requirementId)
),
);
}
if (unresolvedRequirementIds.length > 0 && typeof client.listDirectory === 'function') {
if (!requirementDirectories.has(requirementDirectory)) {
let entries = [];
try {
entries = await client.listDirectory(requirementDirectory, headSha);
} catch (error) {
if (!isMissingResourceError(error)) {
throw error;
}
}
requirementDirectories.set(requirementDirectory, entries);
}
const entries = requirementDirectories.get(requirementDirectory);
const candidateNames = new Set(unresolvedRequirementIds.map(requirementId => {
const parts = requirementId.split('-');
return `${parts.slice(2).join('-').replace(/-\d+$/, '').toLowerCase()}.md`;
}));
const entries = await listRequirementDirectory(requirementDirectory, headSha);
const candidateNames = new Set(unresolvedRequirementIds.map(requirementId =>
POSIX_PATH.basename(candidateRequirementPath(requirementDirectory, requirementId))
));
for (const entry of entries) {
if (entry.type === 'file' && candidateNames.has(POSIX_PATH.basename(entry.path))) {
requirementPaths.add(entry.path);
.github/scripts/pr-docs.test.cjs
+46 -10
View File
@@ -1689,23 +1689,23 @@ test('six work orders sharing three requirements stay within the search quota',
});
// @covers AC-CI-PR-DOCS-001.4, AC-CI-PR-DOCS-003.2
test('PR-only requirements and empty searches reuse a bounded directory lookup', async () => {
const { contents, changed, requirementPath, requirementIds } = repeatedCoverageFixture();
test('newly added requirements resolve from the head diff without code search', async () => {
const { contents, changed, requirementPath: originalPath } = repeatedCoverageFixture();
const requirementPath = 'docs/specs/ui/requirements/coverage.md';
contents[requirementPath] = contents[originalPath];
delete contents[originalPath];
changed.push({ filename: requirementPath, status: 'added' });
let searches = 0;
let listings = 0;
const client = coverageClient(contents, changed, {
async searchCode() {
searches += 1;
return [];
throw new Error('code search should not run for a newly added requirement');
},
async listDirectory(directory, ref) {
assert.equal(directory, 'docs/specs/ui/requirements');
assert.equal(ref, SHA_B);
listings += 1;
if (listings > 1) {
throw new Error('GitHub API request failed with HTTP 429: Too Many Requests');
}
return [{ path: requirementPath, type: 'file' }];
},
});
@@ -1713,7 +1713,42 @@ test('PR-only requirements and empty searches reuse a bounded directory lookup',
const result = await validator.evaluatePullRequest({ client, pullNumber: 42 });
assert.equal(result.status, 'covered', result.errors.join('; '));
assert.equal(searches, requirementIds.length);
assert.equal(searches, 0);
assert.equal(listings, 1);
});
// @covers AC-CI-PR-DOCS-001.5
test('newly added canonical requirements still reject duplicate IDs', async () => {
const { contents, changed, requirementPath: originalPath } = repeatedCoverageFixture();
const requirementPath = 'docs/specs/ui/requirements/coverage.md';
const duplicatePath = 'docs/specs/ui/requirements/duplicate.md';
contents[requirementPath] = contents[originalPath];
delete contents[originalPath];
contents[duplicatePath] = contents[requirementPath];
changed.push({ filename: requirementPath, status: 'added' });
let searches = 0;
let listings = 0;
const client = coverageClient(contents, changed, {
async searchCode() {
searches += 1;
return [requirementPath];
},
async listDirectory(directory, ref) {
assert.equal(directory, 'docs/specs/ui/requirements');
assert.equal(ref, SHA_B);
listings += 1;
return [
{ path: requirementPath, type: 'file' },
{ path: duplicatePath, type: 'file' },
];
},
});
const result = await validator.evaluatePullRequest({ client, pullNumber: 42 });
assert.equal(result.status, 'invalid', result.errors.join('; '));
assert.match(result.errors.join('; '), /ambiguous definitions/);
assert.equal(searches, 0);
assert.equal(listings, 1);
});
@@ -1786,20 +1821,21 @@ test('lookup errors fail closed without poisoning later evaluations', async t =>
for (const boundary of ['searchCode', 'listDirectory']) {
await t.test(boundary, async () => {
const { contents, changed, requirementPath } = repeatedCoverageFixture();
changed.push({ filename: requirementPath, status: 'added' });
const directoryFallbackPath = 'docs/specs/ui/requirements/coverage.md';
contents[directoryFallbackPath] = contents[requirementPath];
let fail = true;
const client = coverageClient(contents, changed, {
async searchCode() {
if (fail && boundary === 'searchCode') {
throw new Error('GitHub API request failed with HTTP 403: API rate limit exceeded');
}
return [];
return boundary === 'listDirectory' ? [] : [requirementPath];
},
async listDirectory() {
if (fail && boundary === 'listDirectory') {
throw new Error('GitHub API request failed with HTTP 503: Service Unavailable');
}
return [];
return [{ path: directoryFallbackPath, type: 'file' }];
},
});