Skip to content

fix: show linked commits for hot topics - #1507

Merged
paulinevos merged 1 commit into
productionfrom
fix/headline-linked-commits
Nov 6, 2026
Merged

paulinevos merged 1 commit into
productionfrom
fix/headline-linked-commits

Conversation

@paulinevos

@paulinevos paulinevos commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Hot-topic pages used semantic search on the headline name, which could include unlinked posts and omit linked posts without embeddings. They now show commits explicitly linked to the selected headline, posted within the past week, newest first.

Changes

  • Add a dedicated HeadlineCommitsQuery using headline associations and an inclusive rolling seven-day posting-time cutoff, with descending date and ID ordering.
  • Preserve viewer visibility, mutes, blocks, and standard commit-card data.
  • Remove hot-topic browsing from the AI search quota and remove the headline-only time filter from semantic search. Ordinary semantic search remains rate-limited.
  • Update the knowledge base and regression tests for linked-only results, the week boundary, ordering, no AI search calls, and independent search budgets.

Validation

All checks ran locally on the reviewed implementation tree:

  • php artisan test --compact tests/Feature/Controllers/HeadlineControllerTest.php tests/Unit/Queries/CommitSearchQueryTest.php tests/Feature/DatabaseSeederTest.php — 45 passed, 425 assertions.
  • vendor/bin/pint --dirty --format agent — passed.
  • composer test:lint — passed; existing JavaScript lint warnings remain in unchanged files.
  • composer test:types — passed.
  • composer test:type-coverage — passed, 100%.
  • bun run test:unit — 301 passed.
  • XDEBUG_MODE=coverage php -d memory_limit=-1 vendor/bin/pest --parallel --ci --coverage --exactly=100.0 --passthru-php="'-d' 'memory_limit=-1'" — 5,832 passed, 31,509 assertions, 100.0% coverage.
  • git diff --check — passed.

No frontend markup, styling, component contracts, or mobile interactions changed. Visual and device verification are inapplicable; HTTP feature tests verify result selection and rate limiting. Hosted CI results are pending.

Independent review

Separate read-only review agent: PASS, with no actionable findings or publication blockers, covering correctness, regressions, architecture, privacy/visibility filtering, and meaningful tests.

Limitations

Production-scale query performance was not benchmarked. No deployment or production data changes are included.

Summary by CodeRabbit

  • New Features
    • Hot-topic headline pages now show linked commits from the past week, ordered newest first. These results follow the existing visibility rules and do not use semantic search.
  • Bug Fixes
    • Browsing headline pages no longer consumes or gets blocked by the semantic-search request limit, including during partial page updates.
    • Search requests are consistently subject to the configured limit of 15 per minute; limited requests receive a JSON response.

Copilot AI balanced review requested due to automatic review settings November 6, 2026 21:00
@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Headline pages now list visible, non-pending linked commits from the past week in newest-first order, without semantic search. The page query uses a dedicated query for that list. Headline routes no longer use the search throttle, and the search limiter applies to every request it handles. Recap behavior remains unchanged.

Changes: Headline Commit Listing

Layer / File(s) Summary
Linked commit retrieval
app/Queries/HeadlineCommitsQuery.php, app/Queries/HeadlinePageQuery.php, app/Queries/CommitSearchQuery.php, tests/Unit/Queries/CommitSearchQueryTest.php, KNOWLEDGE_BASE.md, tests/Feature/Controllers/HeadlineControllerTest.php
A dedicated query returns visible, linked headline commits from the past week, ordered newest first. HeadlinePageQuery uses it instead of semantic search. The commit search query no longer accepts or applies a since filter. Feature tests cover linked-commit selection and partial reloads; the unit test checks results without a time cutoff.
Separate headline requests from search throttling
routes/web.php, app/Providers/AppServiceProvider.php, tests/Feature/Controllers/HeadlineControllerTest.php
The headline.show route retains authentication but no longer uses throttle:search. The search limiter applies its per-user limit and JSON 429 response to requests it handles without the former partial-request exemption. Tests cover headline requests and the semantic-search budget.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant HeadlinePageQuery
  participant HeadlineCommitsQuery
  participant Database
  HeadlinePageQuery->>HeadlineCommitsQuery: get(headline, viewer)
  HeadlineCommitsQuery->>Database: select visible linked commits from the past week
  Database-->>HeadlineCommitsQuery: matching commits
  HeadlineCommitsQuery-->>HeadlinePageQuery: newest-first commit collection
Loading

Possibly related PRs

Suggested reviewers: mnapoli

Merge Risk: Minimal · up to 1d507

Headline browsing uses linked commits without consuming the search budget. No issue requiring a pre-merge fix was established.

Security architecture risk: Moderate · up to 1d507

Viewer visibility rules remain in place, and headline browsing no longer invokes semantic-search providers. However, headline requests now load all matching commits without the previous result limit or request throttle. This creates an availability risk for busy topics; actual impact depends on traffic and topic size.

Retained concerns

  • Medium · security · inferred: The new headline path removes both the previous result bound and the search request throttle. An authenticated user can repeatedly request all visible, linked commits from the past week, including their card data. Busy topics can therefore amplify database work, application memory use, and response serialization without an application-level bound in the inspected path. The time window and controlled association producer reduce exposure, but production cardinality and upstream traffic controls are unknown; an actual denial of service was not demonstrated.
Security review details

Security Blast Radius

  • inferred — The identified risk requires a signed-in user requesting an existing headline; it does not require posting or controlling associations. Repeated reads of a high-cardinality topic could contend for application and database resources used by other requests. The affected deployment capacity and resulting service impact are not established.

Security Findings and Attack Paths

  • inferred — The resource-abuse path is repeated authenticated headline GET requests, followed by uncapped seven-day linked-commit retrieval and card loading. Unlike the base behavior, this path has neither the default-20 semantic result bound nor throttle:search. This supports an availability concern, not a demonstrated outage or authorization bypass.

Trust Boundaries and Controls

  • observed — The authenticated viewer is preserved through the controller and query boundary. Root visibility, blocks, and mutes use shared scopes; continuation IDs and quoted commits also retain viewer filtering. The inspected replacement does not introduce a privileged or anonymous viewer substitution.

Resilience and Maintainability Implications

  • observed — The strongest resource-risk counterevidence is the seven-day association predicate and the producer's approved, top-level commit checks. Producer batches of 20 limit individual classification jobs, not accumulated commits per headline; the inspected persistence action adds links without a per-headline count cap.

Hardening Proposals

  • proposed — Preserve independence from the AI search budget while bounding headline work through pagination and a separate request budget sized to measured topic cardinality and query cost.
Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: showing linked commits for hot topics.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 7 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 82dc3d2e-4266-4811-ac31-01a5072d52da
Commits

Reviewing files that changed from the base of the PR and between 946d574 and 1d507ad.

Files selected for processing: 8

  • KNOWLEDGE_BASE.md
  • app/Providers/AppServiceProvider.php
  • app/Queries/CommitSearchQuery.php
  • app/Queries/HeadlineCommitsQuery.php
  • app/Queries/HeadlinePageQuery.php
  • routes/web.php
  • tests/Feature/Controllers/HeadlineControllerTest.php
  • tests/Unit/Queries/CommitSearchQueryTest.php

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The query loads and serializes all visible linked commits for the week instead of capping results at 20, allowing request memory and response size to grow unchecked for busy topics.

1 open issue
What changed in this PR

Hot-topic pages now show explicitly linked commits from the past week instead of semantic matches, making results reflect headline associations without requiring embeddings.

Changes:

  • Add a dedicated linked-commit query with newest-first ordering and existing visibility and card-data handling.
  • Separate headline browsing from the AI search budget and remove the headline-specific time filter from semantic search.
  • Update documentation and regression tests for linked results, time boundaries, and independent search budgets.
File Description
tests/​Unit/​Queries/​CommitSearchQueryTest.php Verify ordinary search remains unrestricted by posting time.
tests/​Feature/​Controllers/​HeadlineControllerTest.php Cover linked results, visibility, ordering, and search-budget independence.
routes/​web.php Remove search throttling from authenticated headline browsing.
KNOWLEDGE_BASE.md Document linked-only hot-topic results and quota behavior.
app/​Queries/​HeadlinePageQuery.php Use the dedicated headline-commit query.
app/​Queries/​HeadlineCommitsQuery.php Retrieve visible linked commits within the rolling seven-day window.
app/​Queries/​CommitSearchQuery.php Remove the headline-only posting-time filter.
app/​Providers/​AppServiceProvider.php Simplify search throttling by removing headline exceptions.

Review effort: Balanced

->withContinuationId($viewer)
->withCardDataFor($viewer)
->newestFirst()
->get()
@paulinevos
paulinevos merged commit 72d03a5 into production Nov 6, 2026
17 checks passed
@paulinevos
paulinevos deleted the fix/headline-linked-commits branch November 6, 2026 21:13
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