Skip to content

H1-H3: add deterministic issue prioritization backend - #39

Merged
mchwang merged 7 commits into
mainfrom
codex/lane-h1-prioritization
Sep 24, 2026
Merged

mchwang merged 7 commits into
mainfrom
codex/lane-h1-prioritization

Conversation

@mchwang

@mchwang mchwang commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Lane assignment

  • Owner: Lane H implementation task
  • Steps: H1 ranking policy; H2 issue retrieval and normalization; H3 deterministic ranking, reasons, and fresh/stale/unavailable states
  • Baseline: 0ae71a503592de90926063fa563c1f7c715db22b
  • Validated head: 5f73c0b058a5413425be68ef5557fd8ed737fdc4
  • Dependency: H4 shared-web integration remains blocked on G4

Owned files

  • docs/implementation/issue-prioritization.md
  • github/issues.ts
  • core/issue-ranking.ts
  • test/issues.test.ts
  • test/issue-ranking.test.ts
  • AGENTS.md (required review-lesson extraction)

Interface contract

A read-only GitHub gateway returns one validated, repository-bound issue snapshot. The prioritizer ranks only normalized fields using the recorded deterministic policy and returns explicit fresh, stale, or unavailable state. Issue text remains data and never becomes shell input or ranking instructions. Concurrent refreshes use a generation guard; superseded attempts cannot publish or replace the cached fresh snapshot.

Validation

  • npm test: 15 files, 322 tests passed at 5f73c0b058a5413425be68ef5557fd8ed737fdc4
  • npm run typecheck: passed at 5f73c0b058a5413425be68ef5557fd8ed737fdc4
  • git diff --check origin/main...HEAD: passed
  • Live read-only adapter probe: normalized 12 open codeabovelab/codeboost issues without exposing issue content

Review rounds

  1. Fixed all 3 findings: post-await cancellation in the gateway and prioritizer, plus process-output/schema-bound alignment. Added cancellation and payload-bound regressions. Declined: none.
  2. Fixed the 1 finding: concurrent refresh generations now prevent late success or failure from publishing over newer state. Added controlled late-success and late-failure regressions. Declined: none.
  3. No inline findings; prior generation finding confirmed resolved. Summary-only collaborator-validation and buffer-sizing wording supplied no failure case, so Reproduce summary-only Lane H review concerns #41 was opened under the repository rule.
  4. No inline findings, but the summary supplied a concrete escaped-character buffer failure case. Added an intermediate serialized-byte fixture proving 32 MiB was insufficient and raised the transport budget to 64 MiB. Declined: none.
  5. No inline findings, but the summary supplied a concrete canonical-casing failure case. Issue and pull-marker URLs now accept GitHub canonical repository casing while preserving exact host, URL shape, and record number; added a mixed-case fixture. Declined: none.
  6. Converged with Findings: None, no new thread, and no new failure case. The summary broadly restated author-association, URL, and buffer concerns; URL and buffer have dedicated passing regressions, while Reproduce summary-only Lane H review concerns #41 tracks association reproduction. No speculative change made.

Review lessons

  • Post-await cancellation findings: covered by existing AGENTS.md rules to preserve cancellation reasons and guard background responses by generation.
  • Concurrent refresh finding: covered by the existing generation/attempt rule and required race-regression section.
  • Process output versus accepted payload findings: captured by the new AGENTS.md guarded-external-action rule in this branch.
  • Canonical repository casing: one-off GitHub adapter behavior covered by its dedicated regression.
  • Remaining summary-only collaborator concern: tracked in Reproduce summary-only Lane H review concerns #41 for intermediate-state reproduction; not treated as a confirmed defect.

Deferred scope

H4 is explicitly out of this PR until G4 releases shared web files. Issue #41 is a non-blocking reproduction follow-up. No shared runner, store, web, package, lockfile, or CI files are changed.

@mchwang
mchwang marked this pull request as ready for review September 24, 2026 15:24
Copilot AI lite review requested due to automatic review settings September 24, 2026 15:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved moderate findings affect cancellation handling and gateway output limits.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 Medium severity

Open (3)
What changed in this PR

Adds a deterministic, repository-bound GitHub issue retrieval and prioritization backend with explicit freshness states.

Changes:

  • Adds bounded issue retrieval and normalization.
  • Implements deterministic scoring, explanations, and tie-breaking.
  • Adds documentation and gateway/ranking tests.
File Summary Review notes
test/​issues.test.ts Gateway validation and failure-handling tests. No findings.
test/​issue-ranking.test.ts Ranking and refresh-state tests. No findings.
github/​issues.ts GitHub issue retrieval and normalization. Moderate findings: recheck cancellation after awaited operations and align output limits with payload bounds.
docs/​implementation/​issue-prioritization.md Documents ranking policy and access contract. No findings.
core/​issue-ranking.ts Deterministic ranking and refresh-state management. Moderate finding: recheck cancellation before committing a fetched snapshot.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread core/issue-ranking.ts
Comment thread github/issues.ts Outdated
Comment thread github/issues.ts
Copilot AI review requested due to automatic review settings September 24, 2026 15:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Concurrent refreshes can commit out of order and overwrite newer snapshots with stale data.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (3)

Comment thread core/issue-ranking.ts
Copilot AI review requested due to automatic review settings September 24, 2026 15:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Two moderate issues remain in github/issues.ts involving collaborator validation and response-buffer sizing.

Review effort: Lite
Findings: None

Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 24, 2026 15:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

MAX_PAGE_BYTES can reject valid large issue payloads; increase the bound or tighten accepted field limits.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity MAX_PAGE_BYTES is too low for valid escaped-character payloads

github/​issues.ts:10

MAX_PAGE_BYTES still does not cover every payload accepted by boundedString: a valid 100-issue page whose 65,536-character bodies contain JSON-escaped control characters can exceed 32 MiB (the bodies alone are about 39 MiB), so execFile rejects it and the refresh becomes unavailable. Increase the transport bound or tighten the accepted character set/field limits; for example, a 64 MiB bound covers the current schema.

Copilot AI review requested due to automatic review settings September 24, 2026 15:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Four unresolved moderate findings remain in github/issues.ts.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Compare repository paths case-insensitively for pull requests

github/​issues.ts:87

Repository names are case-insensitive on GitHub, and repositoryName explicitly accepts uppercase characters, but this exact comparison rejects a valid pull-request marker when GitHub returns the repository's canonical casing differently from the configured value. Compare the repository path case-insensitively (while retaining the exact host, issue number, and URL shape).

This issue also appears on line 91 of the same file.

Copilot AI review requested due to automatic review settings September 24, 2026 15:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Three moderate unresolved findings remain in github/issues.ts regarding author associations, URL validation, and payload buffer bounds.

Review effort: Lite
Findings: None

@mchwang
mchwang merged commit 1e59b7d into main Sep 24, 2026
3 checks passed
@mchwang
mchwang deleted the codex/lane-h1-prioritization branch September 24, 2026 16:27
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.

2 participants