Skip to content

Comments

Improve /check and preserve free-text report statuses#63

Open
WZ wants to merge 5 commits intomainfrom
codex/fix-ticket-prefix-dedupe
Open

Improve /check and preserve free-text report statuses#63
WZ wants to merge 5 commits intomainfrom
codex/fix-ticket-prefix-dedupe

Conversation

@WZ
Copy link
Owner

@WZ WZ commented Feb 21, 2026

What changed

  • /check now counts all weekly work items (including fetched GitLab/GitHub items), not only source=slack rows
  • A member is considered reported if either:
    • their Slack user ID appears on a weekly item, or
    • their Slack profile name fuzzily matches a weekly item author
  • /list Edit modal category default label changed from (no change) to Auto
  • Generated report rendering now preserves free-text statuses instead of coercing them to canonical statuses
    • Example kept as-is: (resolved in session; root cause analysis in progress)

Tests

  • Added unit test coverage for memberReportedThisWeek helper
  • Added regression test for free-text status preservation in report builder
  • go test ./...

Copilot AI review requested due to automatic review settings February 21, 2026 03:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Slack /check (list-missing) flow so members with imported GitLab/GitHub MR/PR activity are not incorrectly flagged as missing, by considering both Slack author_id signals and fuzzy name matches against fetched item authors.

Changes:

  • Load all weekly work items for /check and derive a unique set of reported author names from them.
  • Treat a member as “reported” if their Slack user ID appears on a Slack-sourced item or their Slack profile name fuzzily matches a fetched item author.
  • Add unit test coverage for the new memberReportedThisWeek helper.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
internal/integrations/slack/slack.go Enhances /check missing-member detection to include fetched MR/PR activity via author-name matching and adds helper utilities.
internal/integrations/slack/slack_logic_test.go Adds focused unit test for memberReportedThisWeek.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 891 to +894
for _, uid := range memberIDs {
if reportedAuthorIDs[uid] {
user, err := api.GetUserInfo(uid)
nameCandidates := []string{uid}
if err == nil {
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

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

handleListMissing now calls api.GetUserInfo(uid) for every team member before checking whether they’re already reported. This adds N Slack API calls per /check and increases latency/rate-limit risk. Consider deriving nameCandidates from the existing cached users list (getCachedUsers/api.GetUsers) or only fetching user info for members that are still potentially missing after the author_id check.

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

Comment on lines +901 to +903
if user.Name != "" {
nameCandidates = append(nameCandidates, user.Name)
}
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

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

Including user.Name (Slack handle) in nameCandidates makes the fuzzy token-subset match very permissive (e.g., handle "alex" will match fetched author "Alex Rivera"), which can incorrectly mark unrelated members as reported and hide genuinely missing reports. Suggest restricting fuzzy matching to DisplayName/RealName (or requiring a minimum token count / exact-match-only for handles) to avoid false positives.

Suggested change
if user.Name != "" {
nameCandidates = append(nameCandidates, user.Name)
}

Copilot uses AI. Check for mistakes.
@WZ WZ changed the title Fix /check to include fetched MR/PR activity Improve /check and preserve free-text report statuses Feb 21, 2026
Copy link
Contributor

Copilot AI commented Feb 21, 2026

@WZ I've opened a new pull request, #64, to work on those changes. Once the pull request is ready, I'll request review from you.

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